Re: [PATCH] Menu control for NPAGE and PPAGE

[Carles Pina i Estany]

Hello,

On Sep/24/2008, Robert Millan wrote:
> On Sat, Sep 20, 2008 at 10:37:45PM +0200, Carles Pina i Estany wrote:
> > 
> > Hello,
> > 
> > (This patch includes the changes for GRUB_TERM_NPAGE/PPAGE macros, that
> > I sent in a previous mail)
> > 
> > ChangeLog:
> > --------
> > 2008-09-20  Carles Pina i Estany  <address@hidden>
> >     * normal/menu.c (run_menu): Add Previous and Next Page keys in
> >     grub-menu.
> > 
> >         * include/grub/powerpc/ieee1275/console.h (GRUB_TERM_NPAGE):
> >         Changed to 0x5100.
> >         (GRUB_TERM_PPAGE): Changed to 0x4900.
> > 
> >         * include/grub/sparc64/ieee1275/console.h: Likewise.
> > 
> >         * include/grub/i386/pc/console.h: Likewise.
> > 
> >         * include/grub/efi/console.h: Likewise.
> > --------
> 
> The scancode part is merged already, please update/resync.

done

New ChangeLog entry:
2008-09-24  Carles Pina i Estany  <address@hidden>
        * normal/menu.c (run_menu): Add Previous and Next Page keys in
        grub-menu.

> > +         else
> > +           {
> > +             first = first - GRUB_TERM_NUM_ENTRIES;
> > +
> > +             if (first < 0)
> > +               {
> > +                 offset = offset + first;
> 
> I'd suggest using '+=' and '-=' on these to make it more readable and
> avoid redundancy.

done!

(I'm not a bit fan of it when there is three operators like in:
offset += GRUB_TERM_NUM_ENTRIES - 1;, but yes, it's shorter :-) )

> > +       case GRUB_TERM_NPAGE:
> > +         if (offset==0)
> > +           {
> > +             offset = offset + GRUB_TERM_NUM_ENTRIES -1 ;
> > +             if (first+offset>menu->size)
> > +               {
> > +                 offset=menu->size-first-1;
> > +               }
> > +           }
> 
> Please add spaces around '==', '>', '+', '-', etc.

done!

> > +                 if (offset > menu->size - 1 || offset > 
> > GRUB_TERM_NUM_ENTRIES - 1)
> 
> Does this generate a compiler warning?

no. At least not in my gcc:
address@hidden:~$ gcc --version
gcc (Debian 4.3.1-2) 4.3.1
....

New patch is attached. I also improved some other thing.

Feedback is welcomed :-)

-- 
Carles Pina i Estany            GPG id: 0x17756391
        http://pinux.info

ppage_npage_control04.patch
Description: Text Data