Re: [PATCH] 1/5 Multiple fallback entries

[Colin D Bennett]

Here is an updated patch based on Vesa's feedback.  I will commit it in
a few days if no one has any objections.

This patch is against GRUB trunk revision 1973.

On Sat, 31 Jan 2009
23:08:23 +0200 Vesa Jääskeläinen <address@hidden> wrote:

> What do you think if all text menu related would be moved to own file,
> like menu_text.c or so ? I think it will cleanup code nicely...

Agreed.  It has been done.

> Usually pointer to userdata is last argument in list. Please use same
> term for it as in grub_menu_execute_with_fallback (you are free to pick
> new name if you see that it fits better).

The ‘userdata’ argument has been moved to the end of the argument list.
  
> > +/* Get the first entry number from the variable NAME, which is a
> > +   space-separated list of nonnegative integers.  The entry number which
> > +   is returned is stripped from the value of NAME.  If no entry number can
> > +   be found, returns -1.  */
> 
> We use term environment variable...

Fixed.

> > +
> > +/* Callbacks for grub_menu_execute_with_fallback() for the text menu.  */
> > +
> 
> If you move these to new file, please replace this comment with new
> comments for every function.

Done.

> > +static void
> > +notify_booting (void *userdata __attribute__((unused)),
> > +           grub_menu_entry_t entry)
> > +{
> > +  grub_printf ("  Booting \'%s\'\n\n", entry->title);
> > +}
> > +
> > +static void
> > +notify_fallback (void *userdata __attribute__((unused)),
> > +            grub_menu_entry_t entry)
> > +{
> > +  grub_printf ("\n  Falling back to \'%s\'\n\n", entry->title);
> > +  grub_millisleep (2000);
> 
> I think this delay should be generic part of the implementation and not
> specific to text menu. Or alternatively it needs to be documented that
> fallback callback function is supposed to wait for 2 secs...

I just added a comment documenting the fact that the implementation of
the callback should delay at least 2 sec before returning so that the
user can see the message.  I didn't just put the delay in the generic
code because I thought the graphical menu, for instance, might want to
do something during the delay like doing animation or something, or
providing an OK button to dismiss the message before the delay has
elapsed.

Regards,
Colin

01_multiple-fallback.patch
Description: Text Data

signature.asc
Description: PGP signature