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

[Colin Watson]

On Mon, Sep 16, 2013 at 06:46:47PM +0800, Yang Bai wrote:
> 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.

That's not quite how C works.  The storage for the struct has to live in
the kernel (probably unacceptable) or in some module.

(Or it could be a #define expanded in a couple of places to instantiate
the storage in multiple locations, I suppose; but that's awfully
inelegant and should be a last resort.)

> > 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?

This still involves cumbersome duplicate configuration.  I don't think
it's the right answer.  I prefer the option discussed in a previous
thread whereby "sleep --interruptible" could introspect menu entries for
hotkeys; it is clearly much less effort to use even if it's a bit more
work to implement.

> > 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.

This proposal doesn't scale beyond a single hotkey, and I'm not
convinced by moving the complexity out to grub.cfg.  In particular
having to put all the menu entries inside an if block would massively
complicate the existing configuration structure.

Franz's approach to this is better.

-- 
Colin Watson                                       address@hidden