Revamp hidden timeout handling

Add a new timeout_style environment variable and a corresponding
GRUB_TIMEOUT_STYLE configuration key for grub-mkconfig.  This
controls hidden-timeout handling more simply than the previous
arrangements, and pressing any hotkeys associated with menu entries
during the hidden timeout will now boot the corresponding menu entry
immediately.

GRUB_HIDDEN_TIMEOUT=<non-empty> + GRUB_TIMEOUT=<non-zero> now
generates a warning, and if it shows the menu it will do so as if
the second timeout were not present.  Other combinations are
translated into reasonable equivalents.

ChangeLog

@@ -1,3 +1,17 @@
+2013-11-28  Colin Watson  <cjwatson@ubuntu.com>
+
+	Add a new timeout_style environment variable and a corresponding
+	GRUB_TIMEOUT_STYLE configuration key for grub-mkconfig.  This
+	controls hidden-timeout handling more simply than the previous
+	arrangements, and pressing any hotkeys associated with menu entries
+	during the hidden timeout will now boot the corresponding menu entry
+	immediately.
+
+	GRUB_HIDDEN_TIMEOUT=<non-empty> + GRUB_TIMEOUT=<non-zero> now
+	generates a warning, and if it shows the menu it will do so as if
+	the second timeout were not present.  Other combinations are
+	translated into reasonable equivalents.
+
 2013-11-27  Vladimir Serbinenko  <phcoder@gmail.com>
 
 	Eliminate variable length arrays in grub_vsnprintf_real.

docs/grub.texi

@@ -1298,19 +1298,46 @@ a key is pressed.  The default is @samp{5}.  Set to @samp{0} to boot
 immediately without displaying the menu, or to @samp{-1} to wait
 indefinitely.
 
+If @samp{GRUB_TIMEOUT_STYLE} is set to @samp{countdown} or @samp{hidden},
+the timeout is instead counted before the menu is displayed.
+
+@item GRUB_TIMEOUT_STYLE
+If this option is unset or set to @samp{menu}, then GRUB will display the
+menu and then wait for the timeout set by @samp{GRUB_TIMEOUT} to expire
+before booting the default entry.  Pressing a key interrupts the timeout.
+
+If this option is set to @samp{countdown} or @samp{hidden}, then, before
+displaying the menu, GRUB will wait for the timeout set by
+@samp{GRUB_TIMEOUT} to expire.  If @key{ESC} is pressed during that time, it
+will display the menu and wait for input according to @samp{GRUB_TIMEOUT}.
+If a hotkey associated with a menu entry is pressed, it will boot the
+associated menu entry immediately.  If the timeout expires before either of
+these happens, it will display the menu.  In the @samp{countdown} case, it
+will show a one-line indication of the remaining time.
+
 @item GRUB_HIDDEN_TIMEOUT
-Wait this many seconds for @key{ESC} to be pressed before displaying the menu.
+Wait this many seconds before displaying the menu.  If @key{ESC} is pressed
-If no @key{ESC} is pressed during that time, display the menu for the number of
+during that time, display the menu and wait for input according to
-seconds specified in GRUB_TIMEOUT before booting the default entry. We expect
+@samp{GRUB_TIMEOUT}.  If a hotkey associated with a menu entry is pressed,
-that most people who use GRUB_HIDDEN_TIMEOUT will want to have GRUB_TIMEOUT set 
+boot the associated menu entry immediately.  If the timeout expires before
-to @samp{0} so that the menu is not displayed at all unless @key{ESC} is
+either of these happens, display the menu for the number of seconds
-pressed.
+specified in @samp{GRUB_TIMEOUT} before booting the default entry.
-Unset by default.
+
+If you set @samp{GRUB_HIDDEN_TIMEOUT}, you should also set
+@samp{GRUB_TIMEOUT=0} so that the menu is not displayed at all unless
+@key{ESC} is pressed.
+
+This option is unset by default, and is deprecated in favour of the less
+confusing @samp{GRUB_TIMEOUT_STYLE=countdown} or
+@samp{GRUB_TIMEOUT_STYLE=hidden}.
 
 @item GRUB_HIDDEN_TIMEOUT_QUIET
 In conjunction with @samp{GRUB_HIDDEN_TIMEOUT}, set this to @samp{true} to
 suppress the verbose countdown while waiting for a key to be pressed before
-displaying the menu.  Unset by default.
+displaying the menu.
+
+This option is unset by default, and is deprecated in favour of the less
+confusing @samp{GRUB_TIMEOUT_STYLE=countdown}.
 
 @item GRUB_DEFAULT_BUTTON
 @itemx GRUB_TIMEOUT_BUTTON
@@ -3030,6 +3057,7 @@ These variables have special meaning to GRUB.
 * superusers::
 * theme::
 * timeout::
+* timeout_style::
 @end menu
 
 
@@ -3462,10 +3490,23 @@ keyboard input before booting the default menu entry.  A timeout of @samp{0}
 means to boot the default entry immediately without displaying the menu; a
 timeout of @samp{-1} (or unset) means to wait indefinitely.
 
+If @samp{timeout_style} (@pxref{timeout_style}) is set to @samp{countdown}
+or @samp{hidden}, the timeout is instead counted before the menu is
+displayed.
+
 This variable is often set by @samp{GRUB_TIMEOUT} or
 @samp{GRUB_HIDDEN_TIMEOUT} (@pxref{Simple configuration}).
 
 
+@node timeout_style
+@subsection timeout_style
+
+This variable may be set to @samp{menu}, @samp{countdown}, or @samp{hidden}
+to control the way in which the timeout (@pxref{timeout}) interacts with
+displaying the menu.  See the documentation of @samp{GRUB_TIMEOUT_STYLE}
+(@pxref{Simple configuration}) for details.
+
+
 @node Environment block
 @section The GRUB environment block
 

grub-core/normal/main.c

@@ -523,7 +523,7 @@ static const char *features[] = {
   "feature_chainloader_bpb", "feature_ntldr", "feature_platform_search_hint",
   "feature_default_font_path", "feature_all_video_module",
   "feature_menuentry_id", "feature_menuentry_options", "feature_200_final",
-  "feature_nativedisk_cmd"
+  "feature_nativedisk_cmd", "feature_timeout_style"
 };
 
 GRUB_MOD_INIT(normal)

grub-core/normal/menu.c

@@ -40,6 +40,22 @@
 grub_err_t (*grub_gfxmenu_try_hook) (int entry, grub_menu_t menu,
 				     int nested) = NULL;
 
+enum timeout_style {
+  TIMEOUT_STYLE_MENU,
+  TIMEOUT_STYLE_COUNTDOWN,
+  TIMEOUT_STYLE_HIDDEN
+};
+
+struct timeout_style_name {
+  const char *name;
+  enum timeout_style style;
+} timeout_style_names[] = {
+  {"menu", TIMEOUT_STYLE_MENU},
+  {"countdown", TIMEOUT_STYLE_COUNTDOWN},
+  {"hidden", TIMEOUT_STYLE_HIDDEN},
+  {NULL, 0}
+};
+
 /* Wait until the user pushes any key so that the user
    can see what happened.  */
 void
@@ -70,6 +86,38 @@ grub_menu_get_entry (grub_menu_t menu, int no)
   return e;
 }
 
+/* Get the index of a menu entry associated with a given hotkey, or -1.  */
+static int
+get_entry_index_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)
+      return i;
+
+  return -1;
+}
+
+/* Return the timeout style.  If the variable "timeout_style" is not set or
+   invalid, default to TIMEOUT_STYLE_MENU.  */
+static enum timeout_style
+get_timeout_style (void)
+{
+  const char *val;
+  struct timeout_style_name *style_name;
+
+  val = grub_env_get ("timeout_style");
+  if (val)
+    for (style_name = timeout_style_names; style_name->name; style_name++)
+      if (grub_strcmp (style_name->name, val) == 0)
+	return style_name->style;
+
+  return TIMEOUT_STYLE_MENU;
+}
+
 /* Return the current timeout. If the variable "timeout" is not set or
    invalid, return -1.  */
 int
@@ -488,6 +536,33 @@ get_entry_number (grub_menu_t menu, const char *name)
   return entry;
 }
 
+/* Check whether a second has elapsed since the last tick.  If so, adjust
+   the timer and return 1; otherwise, return 0.  */
+static int
+has_second_elapsed (grub_uint64_t *saved_time)
+{
+  grub_uint64_t current_time;
+
+  current_time = grub_get_time_ms ();
+  if (current_time - *saved_time >= 1000)
+    {
+      *saved_time = current_time;
+      return 1;
+    }
+  else
+    return 0;
+}
+
+static void
+print_countdown (struct grub_term_coordinate *pos, int n)
+{
+  grub_term_restore_pos (pos);
+  /* NOTE: Do not remove the trailing space characters.
+     They are required to clear the line.  */
+  grub_printf ("%d    ", n);
+  grub_refresh ();
+}
+
 #define GRUB_MENU_PAGE_SIZE 10
 
 /* Show the menu and handle menu entry selection.  Returns the menu entry
@@ -502,6 +577,7 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot)
   grub_uint64_t saved_time;
   int default_entry, current_entry;
   int timeout;
+  enum timeout_style timeout_style;
 
   default_entry = get_entry_number (menu, "default");
 
@@ -510,8 +586,71 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot)
   if (default_entry < 0 || default_entry >= menu->size)
     default_entry = 0;
 
+  timeout = grub_menu_get_timeout ();
+  if (timeout < 0)
+    /* If there is no timeout, the "countdown" and "hidden" styles result in
+       the system doing nothing and providing no or very little indication
+       why.  Technically this is what the user asked for, but it's not very
+       useful and likely to be a source of confusion, so we disallow this.  */
+    grub_env_unset ("timeout_style");
+
+  timeout_style = get_timeout_style ();
+
+  if (timeout_style == TIMEOUT_STYLE_COUNTDOWN
+      || timeout_style == TIMEOUT_STYLE_HIDDEN)
+    {
+      static struct grub_term_coordinate *pos;
+      int entry = -1;
+
+      if (timeout_style == TIMEOUT_STYLE_COUNTDOWN && timeout)
+	{
+	  pos = grub_term_save_pos ();
+	  print_countdown (pos, timeout);
+	}
+
+      /* Enter interruptible sleep until Escape or a menu hotkey is pressed,
+         or the timeout expires.  */
+      saved_time = grub_get_time_ms ();
+      while (1)
+	{
+	  int key;
+
+	  key = grub_getkey_noblock ();
+	  if (key != GRUB_TERM_NO_KEY)
+	    {
+	      entry = get_entry_index_by_hotkey (menu, key);
+	      if (entry >= 0)
+		break;
+	    }
+	  if (key == GRUB_TERM_ESC)
+	    {
+	      timeout = -1;
+	      break;
+	    }
+
+	  if (timeout > 0 && has_second_elapsed (&saved_time))
+	    {
+	      timeout--;
+	      if (timeout_style == TIMEOUT_STYLE_COUNTDOWN)
+		print_countdown (pos, timeout);
+	    }
+
+	  if (timeout == 0)
+	    /* We will fall through to auto-booting the default entry.  */
+	    break;
+	}
+
+      grub_env_unset ("timeout");
+      grub_env_unset ("timeout_style");
+      if (entry >= 0)
+	{
+	  *auto_boot = 0;
+	  return entry;
+	}
+    }
+
   /* If timeout is 0, drawing is pointless (and ugly).  */
-  if (grub_menu_get_timeout () == 0)
+  if (timeout == 0)
     {
       *auto_boot = 1;
       return default_entry;
@@ -540,19 +679,12 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot)
       if (grub_normal_exit_level)
 	return -1;
 
-      if (timeout > 0)
+      if (timeout > 0 && has_second_elapsed (&saved_time))
-	{
-	  grub_uint64_t current_time;
-
-	  current_time = grub_get_time_ms ();
-	  if (current_time - saved_time >= 1000)
 	{
 	  timeout--;
 	  grub_menu_set_timeout (timeout);
-	      saved_time = current_time;
 	  menu_print_timeout (timeout);
 	}
-	}
 
       if (timeout == 0)
 	{
@@ -653,15 +785,14 @@ run_menu (grub_menu_t menu, int nested, int *auto_boot)
 
 	    default:
 	      {
-		grub_menu_entry_t entry;
+		int entry;
-		int i;
+
-		for (i = 0, entry = menu->entry_list; i < menu->size;
+		entry = get_entry_index_by_hotkey (menu, c);
-		     i++, entry = entry->next)
+		if (entry >= 0)
-		  if (entry->hotkey == c)
 		  {
 		    menu_fini ();
 		    *auto_boot = 0;
-		      return i;
+		    return entry;
 		  }
 	      }
 	      break;

util/grub-mkconfig.in

@@ -186,6 +186,7 @@ export GRUB_DEFAULT \
   GRUB_HIDDEN_TIMEOUT \
   GRUB_HIDDEN_TIMEOUT_QUIET \
   GRUB_TIMEOUT \
+  GRUB_TIMEOUT_STYLE \
   GRUB_DEFAULT_BUTTON \
   GRUB_HIDDEN_TIMEOUT_BUTTON \
   GRUB_TIMEOUT_BUTTON \

util/grub.d/00_header.in

@@ -282,14 +282,45 @@ fi
 
 make_timeout ()
 {
-    if [ "x${1}" != "x" ] ; then
+    if [ "x${GRUB_TIMEOUT_STYLE}" != "x" ] ; then
-	if [ "x${GRUB_HIDDEN_TIMEOUT_QUIET}" = "xtrue" ] ; then
+	cat << EOF
+if [ x\$feature_timeout_style = xy ] ; then
+  set timeout_style=${GRUB_TIMEOUT_STYLE}
+  set timeout=${2}
+EOF
+	if [ "x${GRUB_TIMEOUT_STYLE}" != "xmenu" ] ; then
+	    # Fallback hidden-timeout code in case the timeout_style feature
+	    # is unavailable.  Note that we now ignore GRUB_HIDDEN_TIMEOUT
+	    # and take the hidden-timeout from GRUB_TIMEOUT instead.
+	    if [ "x${GRUB_TIMEOUT_STYLE}" = "xhidden" ] ; then
 		verbose=
 	    else
 		verbose=" --verbose"
 	    fi
 	    cat << EOF
-if sleep$verbose --interruptible ${1} ; then
+elif sleep$verbose --interruptible ${2} ; then
+  set timeout=0
+EOF
+	fi
+	cat << EOF
+fi
+EOF
+    elif [ "x${1}" != "x" ] ; then
+	if [ "x${2}" != "x0" ] ; then
+	    grub_warn "$(gettext "Setting GRUB_TIMEOUT to a non-zero value when GRUB_HIDDEN_TIMEOUT is set is no longer supported.")"
+	fi
+	if [ "x${GRUB_HIDDEN_TIMEOUT_QUIET}" = "xtrue" ] ; then
+	    verbose=
+	    style="hidden"
+	else
+	    verbose=" --verbose"
+	    style="countdown"
+	fi
+	cat << EOF
+if [ x\$feature_timeout_style = xy ] ; then
+  set timeout_style=$style
+  set timeout=${1}
+elif sleep$verbose --interruptible ${1} ; then
   set timeout=${2}
 fi
 EOF