[SLOF] [PATCH 4/4] bootmenu: Wire up the new boot menu in the Forth code

[Thomas Huth]

On 07.06.2017 05:45, Alexey Kardashevskiy wrote:
> On 07/06/17 02:21, Thomas Huth wrote:
[...]
>>
>> Honestly, every time I use the current boot menu after a while, I am
>> confused by its current behavior (I never press RETURN in that case
>> since I expect that one key press is enough to start an entry).
> 
> When would you use it at all? You can control the boot sequence via QEMU
> command line, it is easier imho.

Sure, but apparently there are at least some people who try to use the
boot menu ... otherwise I would not get those Bugzilla tickets where
they complain about the crappy behavior ;-)

>> I think
>> I hardly have ever seen another boot menu in the past where you have to
>> enter a multi-digit number and press RETURN afterwards ...
>> And if you've got more than 30 boot devices, they likely do not fit on
>> your screen anymore anyway, so using the boot menu does not make much
>> sense anymore in that case, too, I think.
> 
> Well, then the next bugreport will be "I have 31 device and only 30 show up
> in the list" ;) You could at least print a line saying "no more than 23 can
> be shown" (80x25 minus one line for boot menu header minus one line for the
> user input).

I promise that I'll close that bug report as WONTFIX ... if you try to
use a boot menu for handling more than 30 devices, you're doing
something wrong anyway.

>> Then I wonder why you apparently prefer now to keep the Forth
>> implementation of the boot menu?? IMHO the current code is quite hard to
>> maintain/understand/extend with all this "evaluate" + "parse-word" magic
>> in there. Last time you said "Either forth or c is ok" for an update
>> (see https://lists.ozlabs.org/pipermail/slof/2017-April/001511.html),
> 
> I remember :) I just looked at the patches and wondered if it would be
> easier to stick to Forth. I do not understand parse-word magic though so if
> even you struggle here, then moving to C is a good thing.
> 
>> so
>> I took the chance now to rewrite it in C, but if that's now suddenly not
>> OK for you anymore, please let me now, then I'll try to disimprove the
>> Forth code instead...
> 
> C is still ok. The patches splitting is confusing. I had to do little
> debugging to find out where is that "\r" which you got rid of and where you
> switched from qemu,boot-list/device to aliases, and commit logs did not
> help with that. If the first patch moved forth bits to C (preserving the
> old functionality) and the second one switched to aliases and removed "\r"
> - that would make things easier to review imho, and also bisectable.

It's a complete rewrite, so first trying to implement the old behavior
in C and then rewrite most of the code again to implement the new
behavior does not make much sense either, I think. So I'll squash the
current patches together, let's see how that looks like...

 Thomas