On Nov 28, 2013 3:31 AM, "Colin Watson" <cjwat...@ubuntu.com> wrote:
>
> Following discussion with Vladimir on IRC, here's another attempt. The
> preferred user-facing configuration mechanism is now simpler, although
> some unfortunate compatibility code is required to make all this work.
>
> 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 | 14 ++++
> docs/grub.texi | 57 ++++++++++++---
> grub-core/normal/main.c | 2 +-
> grub-core/normal/menu.c | 175
+++++++++++++++++++++++++++++++++++++++++------
> util/grub-mkconfig.in | 1 +
> util/grub.d/00_header.in | 35 +++++++++-
> 6 files changed, 251 insertions(+), 33 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index d24f533..4cc4562 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,17 @@
> +2013-11-28 Colin Watson <cjwat...@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 <phco...@gmail.com>
>
> Eliminate variable length arrays in grub_vsnprintf_real.
> diff --git a/docs/grub.texi b/docs/grub.texi
> index 6aee292..f494a3d 100644
> --- a/docs/grub.texi
> +++ b/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.
What you describe here doesn‘t serm what code is doing. Copypaste error?
> 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.
> -If no @key{ESC} is pressed during that time, display the menu for the
number of
> -seconds specified in GRUB_TIMEOUT before booting the default entry. We
expect
> -that most people who use GRUB_HIDDEN_TIMEOUT will want to have
GRUB_TIMEOUT set
> -to @samp{0} so that the menu is not displayed at all unless @key{ESC} is
> -pressed.
> -Unset by default.
> +Wait this many seconds before displaying the menu. If @key{ESC} is
pressed
> +during that time, display the menu and wait for input according to
> +@samp{GRUB_TIMEOUT}. If a hotkey associated with a menu entry is
pressed,
> +boot the associated menu entry immediately. If the timeout expires
before
> +either of these happens, display the menu for the number of seconds
> +specified in @samp{GRUB_TIMEOUT} before booting the default entry.
> +
> +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
>
> diff --git a/grub-core/normal/main.c b/grub-core/normal/main.c
> index ad36273..778de61 100644
> --- a/grub-core/normal/main.c
> +++ b/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)
> diff --git a/grub-core/normal/menu.c b/grub-core/normal/menu.c
> index 9b88290..fa85c35 100644
> --- a/grub-core/normal/menu.c
> +++ b/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,18 +679,11 @@ 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);
> - }
> + timeout--;
> + grub_menu_set_timeout (timeout);
> + menu_print_timeout (timeout);
> }
>
> if (timeout == 0)
> @@ -653,16 +785,15 @@ run_menu (grub_menu_t menu, int nested, int
*auto_boot)
>
> 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;
> +
> + entry = get_entry_index_by_hotkey (menu, c);
> + if (entry >= 0)
> + {
> + menu_fini ();
> + *auto_boot = 0;
> + return entry;
> + }
> }
> break;
> }
> diff --git a/util/grub-mkconfig.in b/util/grub-mkconfig.in
> index ba1d4ef..50f73aa 100644
> --- a/util/grub-mkconfig.in
> +++ b/util/grub-mkconfig.in
> @@ -186,6 +186,7 @@ export GRUB_DEFAULT \
> GRUB_HIDDEN_TIMEOUT \
> GRUB_HIDDEN_TIMEOUT_QUIET \
> GRUB_TIMEOUT \
> + GRUB_TIMEOUT_STYLE \
you need button variant as well
> GRUB_DEFAULT_BUTTON \
> GRUB_HIDDEN_TIMEOUT_BUTTON \
> GRUB_TIMEOUT_BUTTON \
> diff --git a/util/grub.d/00_header.in b/util/grub.d/00_header.in
> index 9838720..d5cf342 100644
> --- a/util/grub.d/00_header.in
> +++ b/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
> + 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
> +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 sleep$verbose --interruptible ${1} ; then
> +if [ x\$feature_timeout_style = xy ] ; then
> + set timeout_style=$style
> + set timeout=${1}
> +elif sleep$verbose --interruptible ${1} ; then
> set timeout=${2}
Is behaviour mismatch between both versions intentional?
I see 2 ways of handling double timeout: either not supporting at all
anymore or generate old code for it. This one seems to be mix of both
> fi
> EOF
> --
> 1.8.4.4
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
