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

[Colin Watson]

On Thu, Nov 28, 2013 at 07:19:46AM +0100, Vladimir 'phcoder' Serbinenko wrote:
> On Nov 28, 2013 3:31 AM, "Colin Watson" <address@hidden> wrote:
> > +If this option is set to @samp{countdown} or @samp{hidden}, then, before
> > +displaying the menu, GRUB will wait for the timeout set by
> > address@hidden 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?

I must be missing something.  What part of this doesn't agree with the
code?

... oh, right, if the timeout expires then it will boot the default
entry.  I'll fix that, thanks.

> > 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

Can you suggest a use case for that?  I can understand why you might
want different timeouts in the button case, just about, but not why
you'd want an entirely different style of menu.

> > +       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

The code is somewhat inevitably confusing, I'll agree, but I don't see
the mismatch.  Could you please give me an example?

Thanks,

-- 
Colin Watson                                       address@hidden