* [RFC] [PATCH] Allow user defined key to interupt sleep command
@ 2013-09-16 8:49 Yang Bai
2013-09-16 9:26 ` Colin Watson
0 siblings, 1 reply; 4+ messages in thread
From: Yang Bai @ 2013-09-16 8:49 UTC (permalink / raw)
To: grub-devel
Hi all,
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.
Modified some code borrowed from [PATCH] Allow hotkeys to interrupt hidden menu.
=== modified file 'grub-core/commands/sleep.c'
--- grub-core/commands/sleep.c 2013-07-11 14:02:22 +0000
+++ grub-core/commands/sleep.c 2013-09-16 08:29:33 +0000
@@ -30,7 +30,7 @@
static const struct grub_arg_option options[] =
{
{"verbose", 'v', 0, N_("Verbose countdown."), 0, 0},
- {"interruptible", 'i', 0, N_("Allow to interrupt with ESC."), 0, 0},
+ {"interruptible", 'i', 0, N_("Allow to interrupt with one key."),
0, ARG_TYPE_STRING},
{0, 0, 0, 0, 0, 0}
};
@@ -46,16 +46,61 @@
grub_refresh ();
}
+static struct
+{
+ char *name;
+ int key;
+} function_key_aliases[] =
+ {
+ {"f1", GRUB_TERM_KEY_F1},
+ {"f2", GRUB_TERM_KEY_F2},
+ {"f3", GRUB_TERM_KEY_F3},
+ {"f4", GRUB_TERM_KEY_F4},
+ {"f5", GRUB_TERM_KEY_F5},
+ {"f6", GRUB_TERM_KEY_F6},
+ {"f7", GRUB_TERM_KEY_F7},
+ {"f8", GRUB_TERM_KEY_F8},
+ {"f9", GRUB_TERM_KEY_F9},
+ {"f10", GRUB_TERM_KEY_F10},
+ {"f11", GRUB_TERM_KEY_F11},
+ {"f12", GRUB_TERM_KEY_F12},
+ {"tab", GRUB_TERM_TAB},
+ {"backspace", GRUB_TERM_BACKSPACE},
+ {"esc", GRUB_TERM_ESC},
+ };
+
+static int
+grub_get_keycode_from_str (const char *name)
+{
+ unsigned i;
+ if (!name)
+ return 0;
+
+ int n = grub_strlen (name);
+
+ /* User gives a char */
+ if (n == 1)
+ return name[0];
+
+ /* check for special keys. */
+ for (i = 0; i < ARRAY_SIZE (function_key_aliases); i++)
+ if (grub_strcmp (name, function_key_aliases[i].name) == 0)
+ return function_key_aliases[i].key;
+
+ /* fallback to ESC */
+ return GRUB_TERM_ESC;
+}
+
/* Based on grub_millisleep() from kern/generic/millisleep.c. */
static int
-grub_interruptible_millisleep (grub_uint32_t ms)
+grub_interruptible_millisleep (grub_uint32_t ms, int key)
{
grub_uint64_t start;
start = grub_get_time_ms ();
while (grub_get_time_ms () - start < ms)
- if (grub_getkey_noblock () == GRUB_TERM_ESC)
+ if (grub_getkey_noblock () == key)
return 1;
return 0;
@@ -65,7 +110,7 @@
grub_cmd_sleep (grub_extcmd_context_t ctxt, int argc, char **args)
{
struct grub_arg_list *state = ctxt->state;
- int n;
+ int n, key;
if (argc != 1)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
@@ -82,6 +127,9 @@
pos = grub_term_save_pos ();
+ if (state[1].set)
+ key = grub_get_keycode_from_str (state[1].arg);
+
for (; n; n--)
{
if (state[0].set)
@@ -89,7 +137,7 @@
if (state[1].set)
{
- if (grub_interruptible_millisleep (1000))
+ if (grub_interruptible_millisleep (1000, key))
return 1;
}
else
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] [PATCH] Allow user defined key to interupt sleep command
2013-09-16 8:49 [RFC] [PATCH] Allow user defined key to interupt sleep command Yang Bai
@ 2013-09-16 9:26 ` Colin Watson
2013-09-16 10:46 ` Yang Bai
0 siblings, 1 reply; 4+ messages in thread
From: Colin Watson @ 2013-09-16 9:26 UTC (permalink / raw)
To: The development of GNU GRUB
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.
Changing "sleep --interruptible" to require a string argument breaks a
user-visible interface. Please do not do this.
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.
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 [cjwatson@ubuntu.com]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] [PATCH] Allow user defined key to interupt sleep command
2013-09-16 9:26 ` Colin Watson
@ 2013-09-16 10:46 ` Yang Bai
2013-09-16 10:57 ` Colin Watson
0 siblings, 1 reply; 4+ messages in thread
From: Yang Bai @ 2013-09-16 10:46 UTC (permalink / raw)
To: The development of GNU GRUB, cjwatson
Hi Colin,
Thanks for your review. Please see my inline comments.
On Mon, Sep 16, 2013 at 5:26 PM, Colin Watson <cjwatson@ubuntu.com> 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 [cjwatson@ubuntu.com]
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC] [PATCH] Allow user defined key to interupt sleep command
2013-09-16 10:46 ` Yang Bai
@ 2013-09-16 10:57 ` Colin Watson
0 siblings, 0 replies; 4+ messages in thread
From: Colin Watson @ 2013-09-16 10:57 UTC (permalink / raw)
To: Yang Bai; +Cc: The development of GNU GRUB
On Mon, Sep 16, 2013 at 06:46:47PM +0800, Yang Bai wrote:
> On Mon, Sep 16, 2013 at 5:26 PM, Colin Watson <cjwatson@ubuntu.com> 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 [cjwatson@ubuntu.com]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-16 10:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-16 8:49 [RFC] [PATCH] Allow user defined key to interupt sleep command Yang Bai
2013-09-16 9:26 ` Colin Watson
2013-09-16 10:46 ` Yang Bai
2013-09-16 10:57 ` Colin Watson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).