Re: [RFC] [PATCH] Allow user defined key to interupt sleep command

[Yang Bai]

Hi Colin,

Thanks for your review. Please see my inline comments.

On Mon, Sep 16, 2013 at 5:26 PM, Colin Watson <address@hidden> wrote:
> On Mon, Sep 16, 2013 at 04:49:46PM +0800, Yang Bai wrote:
>> At now, sleep --interruptible 3 can only be interupted by ESC key.
>> With this patch, we can special a key such as sleep --interruptible
>> f10 3 and we can type F10 to interrupt the sleep. This can work as a
>> hotkey handler.
>
> This patch still duplicates key aliases from
> grub-core/commands/menuentry.c, only it's slightly out of sync and has
> its table in a different order for no discernible reason.  This is an
> excellent illustration of why that table should be in only one place in
> the source code.

What about moving this table out of the commands/menuentry.c and put
it into include/grub/term.h as a map from key's name to GRUB_KEY_CODE?
And every code uses this struct just include this header.

>
> Changing "sleep --interruptible" to require a string argument breaks a
> user-visible interface.  Please do not do this.

What about add an repeatable option maybe called "--key" so this will
not break a user-visible interface and handle multi-key?

>
> Requiring the hotkey to be configured in two locations (once in the
> menuentry command, once in "sleep --interruptible") is cumbersome.  It
> also does not support recognising multiple hotkeys (i.e. any of those
> configured for any menuentry command) at the hiddenmenu stage.
>
> This patch does not pass hotkeys on to the menu.  As a result, you will
> in practice end up pressing the hotkey twice to actually boot the
> hotkeyed menu entry.

Maybe we can use this as:

if sleep --interruptible --key f10 3; then
  menuentry blahblahblah...
else
  set normal_boot=true
fi

if normal_boot; then
  blahblahblah
fi

So that we can map a special hotkey to a special menuentry.

>
> I think you have misunderstood the UI requirement here, and as a result
> I don't think this patch is the right approach.  Sorry.
>
> --
> Colin Watson                                       address@hidden
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel