Re: [PATCH] Fix grub-emu curses KEY_* mapping

[Marco Gerards]

Christian Franke <address@hidden> writes:

> Curses function keys do not work in grub-emu, this patch fixes this in
> grub_ncurses_getkey().

Ha!  I once noticed this and forgot to document this bug :(

> Another approach would be to remove the scancode translation in
> grub_console_getkey(), and check for GRUB_CONSOLE_KEY_* in
> grub_cmdline_get() instead. The GRUB_CONSOLE_KEY_* definitions are
> already platform specific.

Yes, this is really weird!  All archs define these macros and expect
for i386-pc none of them are used.  I actually wonder if any of those
work :-)

> Christian
>
> 2007-11-02  Christian Franke  <address@hidden>
>
>       * util/console.c (grub_ncurses_getkey): Change curses KEY_* mapping,
>       now return control chars instead of GRUB_CONSOLE_KEY_* constants.
>       This fixes the problem that function keys did not work in grub-emu.
>
>
> --- grub2.orig/util/console.c 2007-07-22 01:32:31.000000000 +0200
> +++ grub2/util/console.c      2007-10-13 16:13:46.000000000 +0200
> @@ -161,53 +161,59 @@ grub_ncurses_getkey (void)
>        c = getch ();
>      }
>  
> +  /* XXX: return GRUB_CONSOLE_KEY_* does not work here,
> +     grub_cmdline_get() does not check for these constants.
> +     At least on i386-pc, GRUB_CONSOLE_KEY_* are in fact keyboard
> +     scancodes which are converted into control chars by
> +     grub_console_getkey(). */
> +

I do not think this comment is needed.  You explained it in the
email.  I do not like comments that explain why we aren't doing some
things, it is usually better to explain why we do something if it is a
hack or not obvious.  In this case I prefer no comment.

>    switch (c)
>      {
>      case KEY_LEFT:
> -      c = GRUB_CONSOLE_KEY_LEFT;
> +      c = 2; /*GRUB_CONSOLE_KEY_LEFT*/
>        break;

Please just remove the comment.  As this appears to be wrong anyways
:-)

Thanks,
Marco