[PATCH] Menu control for NPAGE and PPAGE

[PATCH] Menu control for NPAGE and PPAGE

[Carles Pina i Estany]

[-- Attachment #1: Type: text/plain, Size: 852 bytes --]


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  <carles@pina.cat>
	* 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.
--------

I've tested using several grub.cfg files.

Feel free to commit or test (I would fix any error, please send a step by step
user-case that fails -number of elements in the menu, description of what you
do)

Thanks,

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

[-- Attachment #2: ppage_npage_control01.patch --]
[-- Type: text/x-diff, Size: 3503 bytes --]

Index: include/grub/powerpc/ieee1275/console.h
===================================================================
--- include/grub/powerpc/ieee1275/console.h	(revision 1864)
+++ include/grub/powerpc/ieee1275/console.h	(working copy)
@@ -29,8 +29,8 @@
 #define GRUB_CONSOLE_KEY_BACKSPACE	0x0008
 #define GRUB_CONSOLE_KEY_HOME		0x4700
 #define GRUB_CONSOLE_KEY_END		0x4F00
-#define GRUB_CONSOLE_KEY_NPAGE		0x4900
-#define GRUB_CONSOLE_KEY_PPAGE		0x5100
+#define GRUB_CONSOLE_KEY_NPAGE		0x5100
+#define GRUB_CONSOLE_KEY_PPAGE		0x4900
 
 /* Initialize the console system.  */
 void grub_console_init (void);
Index: include/grub/sparc64/ieee1275/console.h
===================================================================
--- include/grub/sparc64/ieee1275/console.h	(revision 1864)
+++ include/grub/sparc64/ieee1275/console.h	(working copy)
@@ -30,8 +30,8 @@
 #define GRUB_CONSOLE_KEY_BACKSPACE	0x0008
 #define GRUB_CONSOLE_KEY_HOME		0x4700
 #define GRUB_CONSOLE_KEY_END		0x4F00
-#define GRUB_CONSOLE_KEY_NPAGE		0x4900
-#define GRUB_CONSOLE_KEY_PPAGE		0x5100
+#define GRUB_CONSOLE_KEY_NPAGE		0x5100
+#define GRUB_CONSOLE_KEY_PPAGE		0x4900
 
 /* Initialize the console system.  */
 void grub_console_init (void);
Index: include/grub/i386/pc/console.h
===================================================================
--- include/grub/i386/pc/console.h	(revision 1864)
+++ include/grub/i386/pc/console.h	(working copy)
@@ -29,8 +29,8 @@
 #define GRUB_CONSOLE_KEY_BACKSPACE	0x0008
 #define GRUB_CONSOLE_KEY_HOME		0x4700
 #define GRUB_CONSOLE_KEY_END		0x4F00
-#define GRUB_CONSOLE_KEY_NPAGE		0x4900
-#define GRUB_CONSOLE_KEY_PPAGE		0x5100
+#define GRUB_CONSOLE_KEY_NPAGE		0x5100
+#define GRUB_CONSOLE_KEY_PPAGE		0x4900
 
 #ifndef ASM_FILE
 
Index: include/grub/efi/console.h
===================================================================
--- include/grub/efi/console.h	(revision 1864)
+++ include/grub/efi/console.h	(working copy)
@@ -29,8 +29,8 @@
 #define GRUB_CONSOLE_KEY_BACKSPACE	0x0008
 #define GRUB_CONSOLE_KEY_HOME		0x4700
 #define GRUB_CONSOLE_KEY_END		0x4F00
-#define GRUB_CONSOLE_KEY_NPAGE		0x4900
-#define GRUB_CONSOLE_KEY_PPAGE		0x5100
+#define GRUB_CONSOLE_KEY_NPAGE		0x5100
+#define GRUB_CONSOLE_KEY_PPAGE		0x4900
 
 #include <grub/types.h>
 #include <grub/symbol.h>
Index: normal/menu.c
===================================================================
--- normal/menu.c	(revision 1864)
+++ normal/menu.c	(working copy)
@@ -457,6 +457,52 @@
 		    }
 		}
 	      break;
+	    
+	    case GRUB_TERM_PPAGE:
+	      if (first == 0)
+		{
+		  offset = 0;
+		}
+	      else
+		{
+		  first = first - GRUB_TERM_NUM_ENTRIES;
+
+		  if (first < 0)
+		    {
+		      offset = offset + first;
+		      first = 0;
+		    }
+		}
+	      print_entries (menu, first, offset);
+	      break;
+
+	    case GRUB_TERM_NPAGE:
+	      if (offset==0)
+		{
+		  offset = offset + GRUB_TERM_NUM_ENTRIES -1 ;
+		  if (first+offset>menu->size)
+		    {
+		      offset=menu->size-first-1;
+		    }
+		}
+	      else
+		{
+		  first = first + GRUB_TERM_NUM_ENTRIES;
+			
+		  if (first + offset >= menu->size)
+		    {
+		      first = first - GRUB_TERM_NUM_ENTRIES;
+		      offset = menu->size - 1;
+		      
+		      if (offset > menu->size - 1 || offset > GRUB_TERM_NUM_ENTRIES - 1)
+			{
+			  first = offset - (GRUB_TERM_NUM_ENTRIES - 1);
+			  offset = GRUB_TERM_NUM_ENTRIES - 1;
+			}
+		    }
+		}
+	      print_entries (menu, first, offset);
+	      break;
 	      
 	    case '\n':
 	    case '\r':

Re: [PATCH] Menu control for NPAGE and PPAGE

[Robert Millan]

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  <carles@pina.cat>
> 	* 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.

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

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

> +		      if (offset > menu->size - 1 || offset > GRUB_TERM_NUM_ENTRIES - 1)

Does this generate a compiler warning?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] Menu control for NPAGE and PPAGE

[Carles Pina i Estany]

[-- Attachment #1: Type: text/plain, Size: 2052 bytes --]


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  <carles@pina.cat>
> > 	* 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  <carles@pina.cat>
	* 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:
carles@pinux:~$ 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

[-- Attachment #2: ppage_npage_control04.patch --]
[-- Type: text/x-diff, Size: 1327 bytes --]

Index: normal/menu.c
===================================================================
--- normal/menu.c	(revision 1872)
+++ normal/menu.c	(working copy)
@@ -457,6 +457,57 @@
 		    }
 		}
 	      break;
+	    
+	    case GRUB_TERM_PPAGE:
+	      if (first == 0)
+		{
+		  offset = 0;
+		}
+	      else
+		{
+		  first -= GRUB_TERM_NUM_ENTRIES;
+
+		  if (first < 0)
+		    {
+		      offset += first;
+		      first = 0;
+		    }
+		}
+	      print_entries (menu, first, offset);
+	      break;
+
+	    case GRUB_TERM_NPAGE:
+	      if (offset == 0)
+		{
+		  offset += GRUB_TERM_NUM_ENTRIES - 1;
+		  if (first+offset > menu->size)
+		    {
+		      offset = menu -> size-first - 1;
+		    }
+		}
+	      else
+		{
+		  first += GRUB_TERM_NUM_ENTRIES;
+			
+		  if (first + offset >= menu->size)
+		    {
+		      first -= GRUB_TERM_NUM_ENTRIES;
+		      offset += GRUB_TERM_NUM_ENTRIES;
+		      
+		      if (offset > menu->size - 1 ||
+		                     offset > GRUB_TERM_NUM_ENTRIES - 1)
+			{
+			  offset = menu->size - first - 1;
+			}
+		      if (offset > GRUB_TERM_NUM_ENTRIES)
+		        {
+			  first += offset - GRUB_TERM_NUM_ENTRIES + 1;
+			  offset = GRUB_TERM_NUM_ENTRIES - 1;
+			}
+		    }
+		}
+	      print_entries (menu, first, offset);
+	      break;
 	      
 	    case '\n':
 	    case '\r':

Re: [PATCH] Menu control for NPAGE and PPAGE

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 474 bytes --]

On Wed, Sep 24, 2008 at 06:07:24PM +0200, Carles Pina i Estany wrote:
> 
> New patch is attached. I also improved some other thing.
> 
> Feedback is welcomed :-)

There seems to be an off-by-one problem.  Try with the attached grub.cfg.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[-- Attachment #2: grub.cfg --]
[-- Type: text/plain, Size: 244 bytes --]

menuentry 1 {
	blah
}
menuentry 2 {
	blah
}
menuentry 3 {
	blah
}
menuentry 4 {
	blah
}
menuentry 5 {
	blah
}
menuentry 6 {
	blah
}
menuentry 7 {
	blah
}
menuentry 8 {
	blah
}
menuentry 9 {
	blah
}
menuentry 10 {
	blah
}
menuentry 11 {
	blah
}

Re: [PATCH] Menu control for NPAGE and PPAGE

[Carles Pina i Estany]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]


Hello,

On Sep/24/2008, Robert Millan wrote:
> On Wed, Sep 24, 2008 at 06:07:24PM +0200, Carles Pina i Estany wrote:
> > 
> > New patch is attached. I also improved some other thing.
> > 
> > Feedback is welcomed :-)
> 
> There seems to be an off-by-one problem.  Try with the attached grub.cfg.

should be fine now, sorry.

There is two things that I don't like in this world:
a) program menus and menus movements
b) testing my code

:-)

(of course I tested but not this case)

Regards,

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

[-- Attachment #2: ppage_npage_control06.patch --]
[-- Type: text/x-diff, Size: 1330 bytes --]

Index: normal/menu.c
===================================================================
--- normal/menu.c	(revision 1872)
+++ normal/menu.c	(working copy)
@@ -457,6 +457,57 @@
 		    }
 		}
 	      break;
+	    
+	    case GRUB_TERM_PPAGE:
+	      if (first == 0)
+		{
+		  offset = 0;
+		}
+	      else
+		{
+		  first -= GRUB_TERM_NUM_ENTRIES;
+
+		  if (first < 0)
+		    {
+		      offset += first;
+		      first = 0;
+		    }
+		}
+	      print_entries (menu, first, offset);
+	      break;
+
+	    case GRUB_TERM_NPAGE:
+	      if (offset == 0)
+		{
+		  offset += GRUB_TERM_NUM_ENTRIES - 1;
+		  if (first + offset >= menu->size)
+		    {
+		      offset = menu->size - first - 1;
+		    }
+		}
+	      else
+		{
+		  first += GRUB_TERM_NUM_ENTRIES;
+			
+		  if (first + offset >= menu->size)
+		    {
+		      first -= GRUB_TERM_NUM_ENTRIES;
+		      offset += GRUB_TERM_NUM_ENTRIES;
+		      
+		      if (offset > menu->size - 1 ||
+		                     offset > GRUB_TERM_NUM_ENTRIES - 1)
+			{
+			  offset = menu->size - first - 1;
+			}
+		      if (offset > GRUB_TERM_NUM_ENTRIES)
+		        {
+			  first += offset - GRUB_TERM_NUM_ENTRIES + 1;
+			  offset = GRUB_TERM_NUM_ENTRIES - 1;
+			}
+		    }
+		}
+	      print_entries (menu, first, offset);
+	      break;
 	      
 	    case '\n':
 	    case '\r':

Re: [PATCH] Menu control for NPAGE and PPAGE

[Carles Pina i Estany]

Hi,

On Sep/24/2008, Carles Pina i Estany wrote:

> There is two things that I don't like in this world:
> a) program menus and menus movements
> b) testing my code

[...]

> (of course I tested but not this case)

well, and want more thing that I don't like: Grub without next/prev page
support :-D


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

Re: [PATCH] Menu control for NPAGE and PPAGE

[Carles Pina i Estany]

Hello,

On Sep/24/2008, Carles Pina i Estany wrote:

> On Sep/24/2008, Robert Millan wrote:
> > On Wed, Sep 24, 2008 at 06:07:24PM +0200, Carles Pina i Estany wrote:
> > > 
> > > New patch is attached. I also improved some other thing.
> > > 
> > > Feedback is welcomed :-)
> > 
> > There seems to be an off-by-one problem.  Try with the attached grub.cfg.
> 
> should be fine now, sorry.

pinging... somebody has had time to check it?


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

Re: [PATCH] Menu control for NPAGE and PPAGE

[Pavel Roskin]

On Tue, 2008-10-21 at 20:39 +0200, Carles Pina i Estany wrote:
> Hello,
> 
> On Sep/24/2008, Carles Pina i Estany wrote:
> 
> > On Sep/24/2008, Robert Millan wrote:
> > > On Wed, Sep 24, 2008 at 06:07:24PM +0200, Carles Pina i Estany wrote:
> > > > 
> > > > New patch is attached. I also improved some other thing.
> > > > 
> > > > Feedback is welcomed :-)
> > > 
> > > There seems to be an off-by-one problem.  Try with the attached grub.cfg.
> > 
> > should be fine now, sorry.
> 
> pinging... somebody has had time to check it?

I can commit it if there are no objections and my testing finds no
problems.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Menu control for NPAGE and PPAGE

[Carles Pina i Estany]

Hi,

On Oct/21/2008, Pavel Roskin wrote:
> On Tue, 2008-10-21 at 20:39 +0200, Carles Pina i Estany wrote:
> > Hello,
> > 
> > On Sep/24/2008, Carles Pina i Estany wrote:
> > 
> > > On Sep/24/2008, Robert Millan wrote:
> > > > On Wed, Sep 24, 2008 at 06:07:24PM +0200, Carles Pina i Estany wrote:
> > > > > 
> > > > > New patch is attached. I also improved some other thing.
> > > > > 
> > > > > Feedback is welcomed :-)
> > > > 
> > > > There seems to be an off-by-one problem.  Try with the attached grub.cfg.
> > > 
> > > should be fine now, sorry.
> > 
> > pinging... somebody has had time to check it?
> 
> I can commit it if there are no objections and my testing finds no
> problems.

I don't think that will be objections, we talked about it some time ago.

I did some testing and Robert did some too (I think), if you test it and
you find some problems report it to me, I will fix and we can commit
soon or later.

Thanks,

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

Re: [PATCH] Menu control for NPAGE and PPAGE

[Robert Millan]

On Sat, Oct 25, 2008 at 09:02:27PM +0200, Carles Pina i Estany wrote:
> 
> Hi,
> 
> On Oct/21/2008, Pavel Roskin wrote:
> > On Tue, 2008-10-21 at 20:39 +0200, Carles Pina i Estany wrote:
> > > Hello,
> > > 
> > > On Sep/24/2008, Carles Pina i Estany wrote:
> > > 
> > > > On Sep/24/2008, Robert Millan wrote:
> > > > > On Wed, Sep 24, 2008 at 06:07:24PM +0200, Carles Pina i Estany wrote:
> > > > > > 
> > > > > > New patch is attached. I also improved some other thing.
> > > > > > 
> > > > > > Feedback is welcomed :-)
> > > > > 
> > > > > There seems to be an off-by-one problem.  Try with the attached grub.cfg.
> > > > 
> > > > should be fine now, sorry.
> > > 
> > > pinging... somebody has had time to check it?
> > 
> > I can commit it if there are no objections and my testing finds no
> > problems.
> 
> I don't think that will be objections, we talked about it some time ago.
> 
> I did some testing and Robert did some too (I think), if you test it and
> you find some problems report it to me, I will fix and we can commit
> soon or later.

Hi,

Sorry for the delay, I checked this in.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."