[RFC][PATCH] Allow hotkeys to interrupt hidden menu

* [RFC][PATCH] Allow hotkeys to interrupt hidden menu
@ 2013-09-11 13:18 Colin Watson
  2013-09-11 13:31 ` Colin Watson
  2013-09-12  2:44 ` Andrey Borzenkov
  0 siblings, 2 replies; 29+ messages in thread
From: Colin Watson @ 2013-09-11 13:18 UTC (permalink / raw)
  To: grub-devel; +Cc: Franz Hsieh

One of my colleagues, Franz Hsieh (CCed) sent this patch to
https://bugs.launchpad.net/ubuntu/+source/grub2/+bug/1178618.  It has a
few minor implementation issues, and I'll follow up with my own review
in a few minutes (it certainly also requires rebasing to apply properly
to trunk), but I wanted to ask what people thought of the design,
particularly since it adds new configuration options.  The basic idea is
to allow a hidden menu (sleep --interruptible) to be interrupted by a
hotkey which is then passed on to the menu code, so that you can use
menuentry --hotkey and still hotkey-select an item from the hidden menu.

I suggested adding an "unput"-type feature to the terminal layer to
support this, but the approach used here of stuffing the hotkey into an
environment variable should work just as well and avoids the need to add
more code to the kernel.

Index: grub2-1.99/grub-core/commands/sleep.c
===================================================================

--- grub2-1.99.orig/grub-core/commands/sleep.c	2013-06-13 10:15:08.574977456 +0800
+++ grub2-1.99/grub-core/commands/sleep.c	2013-06-13 10:15:09.366977489 +0800
@@ -24,6 +24,7 @@
 #include <grub/misc.h>
 #include <grub/extcmd.h>
 #include <grub/i18n.h>
+#include <grub/env.h>
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
@@ -31,9 +32,31 @@
   {
     {"verbose", 'v', 0, N_("Verbose countdown."), 0, 0},
     {"interruptible", 'i', 0, N_("Interruptible with ESC."), 0, 0},
+    {"function-key", 'f', 0,
+     N_("Interruptible with function key."), "KEY", ARG_TYPE_STRING},
     {0, 0, 0, 0, 0, 0}
   };
 
+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},
+  };
+
 static grub_uint16_t *pos;
 
 static void
@@ -46,7 +69,21 @@
 }
 
 static int
-grub_check_keyboard (void)
+grub_parse_function_key(const char* name)
+{
+  unsigned i;
+  if (!name)
+    return 0;
+
+  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;
+
+  return 0;
+}
+
+static int
+grub_check_keyboard (int fnkey)
 {
   int mods = 0;
   grub_term_input_t term;
@@ -64,22 +101,34 @@
       (mods & (GRUB_TERM_STATUS_LSHIFT | GRUB_TERM_STATUS_RSHIFT)) != 0)
     return 1;
 
-  if (grub_checkkey () >= 0 && grub_getkey () == GRUB_TERM_ESC)
-    return 1;
+  if (grub_checkkey () >= 0)
+    {
+      int key = grub_getkey();
+      if (key == GRUB_TERM_ESC)
+	return 1;
+
+      if (key == fnkey)
+	{
+	  char hotkey[32] = {0};
+	  grub_snprintf(hotkey, 32, "%d", key);
+	  grub_env_set("hotkey", (char*)&hotkey);
+	  return 1;
+	}
+    }
 
   return 0;
 }
 
 /* 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_check_keyboard ())
+    if (grub_check_keyboard (key))
       return 1;
 
   return 0;
@@ -90,6 +139,7 @@
 {
   struct grub_arg_list *state = ctxt->state;
   int n;
+  int key = 0;
 
   if (argc != 1)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "missing operand");
@@ -104,14 +154,21 @@
 
   pos = grub_term_save_pos ();
 
+  if (state[2].set)
+    {
+      key = grub_parse_function_key(state[2].arg);
+      if (key == 0)
+	return grub_error (GRUB_ERR_BAD_ARGUMENT, "invalid function key");
+    }
+
   for (; n; n--)
     {
       if (state[0].set)
 	do_print (n);
 
-      if (state[1].set)
+      if (state[1].set || state[2].set)
 	{
-	  if (grub_interruptible_millisleep (1000))
+	  if (grub_interruptible_millisleep (1000, key))
 	    return 1;
 	}
       else
Index: grub2-1.99/grub-core/normal/menu.c
===================================================================
--- grub2-1.99.orig/grub-core/normal/menu.c	2013-06-13 10:15:08.618977458 +0800
+++ grub2-1.99/grub-core/normal/menu.c	2013-06-13 10:15:09.370977489 +0800
@@ -103,6 +103,33 @@
   return timeout;
 }
 
+int
+grub_menu_get_hotkey (void)
+{
+  char *val;
+  int hotkey;
+
+  val = grub_env_get ("hotkey");
+  if (! val)
+    return -1;
+
+  grub_error_push();
+
+  hotkey = (int) grub_strtoul (val, 0, 10);
+
+  /* If the value is invalid, unset the variable.  */
+  if (grub_errno != GRUB_ERR_NONE)
+    {
+      grub_env_unset ("hotkey");
+      grub_errno = GRUB_ERR_NONE;
+      hotkey = -1;
+    }
+
+  grub_error_pop ();
+
+  return hotkey;
+}
+
 /* Set current timeout in the variable "timeout".  */
 void
 grub_menu_set_timeout (int timeout)
@@ -481,6 +508,21 @@
   return entry;
 }
 
+static int
+get_menuentry_by_hotkey(grub_menu_t menu, int hotkey)
+{
+  grub_menu_entry_t entry;
+  int i;
+  for (i = 0, entry = menu->entry_list; i < menu->size;
+       i++, entry = entry->next)
+    if (entry->hotkey == hotkey)
+      {
+	menu_fini ();
+	return i;
+      }
+  return 0;
+}
+
 #define GRUB_MENU_PAGE_SIZE 10
 
 /* Show the menu and handle menu entry selection.  Returns the menu entry
@@ -495,14 +537,23 @@
   grub_uint64_t saved_time;
   int default_entry, current_entry;
   int timeout;
+  int hotkey = 0;
 
   default_entry = get_entry_number (menu, "default");
+  hotkey = grub_menu_get_hotkey ();
 
   /* If DEFAULT_ENTRY is not within the menu entries, fall back to
      the first entry.  */
   if (default_entry < 0 || default_entry >= menu->size)
     default_entry = 0;
 
+  /* check if hotkey is set */
+  if (hotkey > 0 && (current_entry = get_menuentry_by_hotkey(menu, hotkey)) > 0)
+    {
+      *auto_boot = 1;
+      return current_entry;
+    }
+
   /* If timeout is 0, drawing is pointless (and ugly).  */
   if (grub_menu_get_timeout () == 0)
     {
@@ -646,16 +697,12 @@
 
 	    default:
 	      {
-		grub_menu_entry_t entry;
-		int i;
-		for (i = 0, entry = menu->entry_list; i < menu->size;
-		     i++, entry = entry->next)
-		  if (entry->hotkey == c)
-		    {
-		      menu_fini ();
-		      *auto_boot = 0;
-		      return i;
-		    }
+		int entry = get_menuentry_by_hotkey(menu, c);
+		if (entry > 0)
+		  {
+		    *auto_boot = 0;
+		    return entry;
+		  }
 	      }
 	      break;
 	    }
Index: grub2-1.99/util/grub-mkconfig.in
===================================================================
--- grub2-1.99.orig/util/grub-mkconfig.in	2013-06-13 10:15:09.322977487 +0800
+++ grub2-1.99/util/grub-mkconfig.in	2013-06-13 10:16:33.954980977 +0800
@@ -251,7 +251,8 @@
   GRUB_INIT_TUNE \
   GRUB_SAVEDEFAULT \
   GRUB_BADRAM \
-  GRUB_RECORDFAIL_TIMEOUT
+  GRUB_RECORDFAIL_TIMEOUT \
+  GRUB_HIDDEN_TIMEOUT_HOTKEY
 
 if test "x${grub_cfg}" != "x"; then
   rm -f ${grub_cfg}.new
Index: grub2-1.99/util/grub.d/30_os-prober.in
===================================================================
--- grub2-1.99.orig/util/grub.d/30_os-prober.in	2013-06-13 10:15:09.010977474 +0800
+++ grub2-1.99/util/grub.d/30_os-prober.in	2013-06-13 10:22:12.534994943 +0800
@@ -34,6 +34,12 @@
 	verbose=" --verbose"
       fi
 
+      if [ "x${GRUB_HIDDEN_TIMEOUT_HOTKEY}" = "x" ] ; then
+        hotkey=
+      else
+        hotkey=" --function-key ${GRUB_HIDDEN_TIMEOUT_HOTKEY}"
+      fi
+
       if [ "x${1}" = "x0" ] ; then
 	cat <<EOF
 if [ "x\${timeout}" != "x-1" ]; then
@@ -44,7 +50,7 @@
       set timeout=0
     fi
   else
-    if sleep$verbose --interruptible 3 ; then
+    if sleep$verbose --interruptible 3$hotkey ; then
       set timeout=0
     fi
   fi
@@ -53,7 +59,7 @@
       else
 	cat << EOF
 if [ "x\${timeout}" != "x-1" ]; then
-  if sleep$verbose --interruptible ${GRUB_HIDDEN_TIMEOUT} ; then
+  if sleep$verbose --interruptible ${GRUB_HIDDEN_TIMEOUT}$hotkey ; then
     set timeout=0
   fi
 fi

Thanks,

-- 
Colin Watson                                       [cjwatson@ubuntu.com]


^ permalink raw reply	[flat|nested] 29+ messages in thread