[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC] [PATCH] Allow user defined key to interupt sleep command
From: |
Yang Bai |
Subject: |
Re: [RFC] [PATCH] Allow user defined key to interupt sleep command |
Date: |
Mon, 16 Sep 2013 18:46:47 +0800 |
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