Attempt at pull request (KDL suggestion)


#1

Hope it’s okay with the Haiku devs; I’ve submitted a proposal for KDL where instead of showing,

Welcome to kernel debugging land

My idea is to show something like:

----------------------------------------------------
If you have opened the KDL shell by choice, welcome.
----------------------------------------------------
IF THIS HAS RANDOMLY APPEARED...
You need to restart your computer. Type reboot to restart.
If you cannot restart, hold down the Power button.
----------------------------------------------------

Of course, this is just an idea, which you guys are free to change or what not, but I did want to try to have the gist of the idea out there. Hopefully, by adding a message that both end users and developers can read here, it’ll make what has happened clearer. (And since the scrolling output may hide the whole message, hopefully the “If you cannot restart” line will still be visible.)

Anyway, hope again that this is okay with the team; thanks for reading…


#2

I think that would break R5 compatibility. Or it at least breaks R5 nostalgia. :joy:


#3

Where did you submitted? It should be visible at gerrit, but it isn’t.

https://dev.haiku-os.org/wiki/CodingGuidelines/SubmittingPatches


#4

That’s most likely because I did it wrong. This is the first time I’ve tried to do something like this. From the master branch, I clicked the pencil (edit) for debug.cpp, to which GitHub mentioned to fork where I could do changes and propose it. So, it created a fork, where I edited the file, then proposed the change. And… I see my error. I forgot to click “Open pull request”. So I did, and received:

Merging is blocked

The base branch restricts merging to authorized users.

So, that’s that. I’ve closed the pull request for now (Closed with unmerged commits; This pull request is closed, but the apgreimann:patch-1 branch has unmerged commits.)

But at any rate, my change is shown here on the forum, so if anyone wants to copy-paste it into an editor or what not, they can…

kprintf("----------------------------------------------------\n");
kprintf("If you have opened the KDL shell by choice, welcome.\n");
kprintf("----------------------------------------------------\n");
kprintf("IF THIS HAS RANDOMLY APPEARED...\n");
kprintf("You need to restart your computer. Type reboot to restart.\n");
kprintf("If you cannot restart, hold down the Power button.\n");
kprintf("----------------------------------------------------\n");

#5

It is written in the description at github:

“The Haiku operating system. (Pull requests will be ignored; patches may be sent to https://review.haiku-os.org).”

Read the docs here: https://dev.haiku-os.org/wiki/CodingGuidelines/SubmittingPatches for more info.


#6

That would definitely explain it! :smiley: I’ve visited Haiku’s GitHub several times, and never read that little tidbit, even though it was written right in front of me above the stats all the time! :smile:


#7

Bonus points if you can rephrase in the form of a haiku.
Welcome kernel debugger land!
How did I get here?
Existential failure now,
Restart computer.


#8

Okay, now that I’ve seen my alarmingly obvious, embarrasing mistake, I’ve signed into Gerrit. I’ve never used it; as a newbie to it, how would I propose the changes?

I’ve looked under the Changes and Your menus in what’s apparently the “new UI”. In the “old UI”, this is the All and My tabs. I can view everything, but I don’t see any links for adding or editing anything.

Thanks, all, for your patience as I’ve made a fool out of myself. This has been quite the learning process! :blush:


#9

Love this. :slight_smile:


#10

Without mistakes made,
Nothing would be learned by us.
Have a beautiful day now.


#11

The basics are explained well in the linked documentation. Read that first please.


#12

Oh the kernel panic. There is nothing more frightening and frustrating that those horrible white screens… :grimacing: I doubt that singing an haiku to the user is enough to alleviate his/her pain.


#13

Too long for my taste. Pressing the power button if nothing else works, is a given, I’d say.
“Welcome to kernel debugging land…” must stay as greeting! :slight_smile:
IMO, one additional line would be enough:

Welcome to kernel debugging land...
Type 'co' to try to leave KDL, 'reboot' to restart, or 'help' for all KDL commands.

#14

Agree. This is a lot shorter, and sweeter. There’s two reasons mine was long though:

  • Backscrolling may prevent seeing the whole message. If every other line was blocked, If you cannot restart, hold down the Power button would still be visible.
  • What if the user cannot type? I have had that happen before; forcing continue also sometimes can leave things unstable, imho, as I’ve done that before as well. But maybe I’m just bad at trying to debug :smile:

But I really like yours though; it says it all in just one extra line, and is a great improvement to just having ‘Welcome to kernel debugging land…’ shown. Would you guys consider adding your version to Haiku then? Because, again, it does add to the original…


#15

I dont get it. @humdinger’s solution is better in this case as his contains more info in a single line.


#16

Um… I already said this in my post, twice.

And I’ll say it again, I think his is better. But can I not leave my own honest observations about it as well? I didn’t know that wasn’t allowed.


#17

I just haven’t noticed your reply. I tought lazyness is allowed here. :slight_smile:


#18

No worries; I originally thought you had read my whole reply and was being rude about it. So just a misunderstanding. :slightly_smiling_face:


#19

IMO Humdinger’s one-liner is good; I don’t really understand your objection about back-scrolling… if the whole message is overwritten because e.g. the backtrace is too long, then it’s overwritten, and not much can be done about that.

I think we should only add this message in release builds though; builds with KDEBUG defined are intended for full-system debugging and so the message would just be redundant.


#20

100% agree; no objections. Yeah, my original worry was visibility, but you’re right.