[PATCH] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[PATCH] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[Carles Pina i Estany]

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


Hello,

I send a patch that corrects the GRUB_CONSOLE_KEY_NPAGE/PPAGE constants.

ChangeLog:
--------
2008-08-24  Carles Pina i Estany  <carles@pina.cat>

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

(this new values are the same used in Grub and also the same that one
can find googling for PGUP and PGDN key codes).

If everything is correct, could someone please commit it?

Cheers,

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

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

Index: include/grub/powerpc/ieee1275/console.h
===================================================================
--- include/grub/powerpc/ieee1275/console.h	(revision 1828)
+++ 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 1828)
+++ 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 1828)
+++ 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 1828)
+++ 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>

Re: [PATCH] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[Carles Pina i Estany]

Hi,

I was just thinking now...

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

> 	* 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 was just thinking... is there any reason that we have the same macros
in 4 different files?

Why not having just a grub/console_scancodes.h: 

#ifndef GRUB_CONSOLE_SCAN_CODES

#define GRUB_CONSOLE_KEY_LEFT           0x4B00
#define GRUB_CONSOLE_KEY_RIGHT          0x4D00
#define GRUB_CONSOLE_KEY_UP             0x4800
#define GRUB_CONSOLE_KEY_DOWN           0x5000
#define GRUB_CONSOLE_KEY_IC             0x5200
#define GRUB_CONSOLE_KEY_DC             0x5300
#define GRUB_CONSOLE_KEY_BACKSPACE      0x0008
#define GRUB_CONSOLE_KEY_HOME           0x4700
#define GRUB_CONSOLE_KEY_END            0x4F00
#define GRUB_CONSOLE_KEY_NPAGE          0x5100
#define GRUB_CONSOLE_KEY_PPAGE          0x4900

#endif

and include this file from the other 4 files?

Thanks,

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

Re: [PATCH] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[Vesa Jääskeläinen]

Carles Pina i Estany wrote:
> I was just thinking... is there any reason that we have the same macros
> in 4 different files?
> 
> Why not having just a grub/console_scancodes.h: 
> 
> #ifndef GRUB_CONSOLE_SCAN_CODES
> 
> #define GRUB_CONSOLE_KEY_LEFT           0x4B00
> #define GRUB_CONSOLE_KEY_RIGHT          0x4D00
> #define GRUB_CONSOLE_KEY_UP             0x4800
> #define GRUB_CONSOLE_KEY_DOWN           0x5000
> #define GRUB_CONSOLE_KEY_IC             0x5200
> #define GRUB_CONSOLE_KEY_DC             0x5300
> #define GRUB_CONSOLE_KEY_BACKSPACE      0x0008
> #define GRUB_CONSOLE_KEY_HOME           0x4700
> #define GRUB_CONSOLE_KEY_END            0x4F00
> #define GRUB_CONSOLE_KEY_NPAGE          0x5100
> #define GRUB_CONSOLE_KEY_PPAGE          0x4900
> 
> #endif
> 
> and include this file from the other 4 files?

Sounds fine. Why do we have same thing with TERM and CONSOLE?

Thanks,
Vesa Jääskeläinen

Re: [PATCH] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[Carles Pina i Estany]

Hi,

On Aug/24/2008, Vesa Jääskeläinen wrote:
> Carles Pina i Estany wrote:
> > I was just thinking... is there any reason that we have the same macros
> > in 4 different files?
> > 
> > Why not having just a grub/console_scancodes.h: 
> > 
> > #ifndef GRUB_CONSOLE_SCAN_CODES
> > 
> > #define GRUB_CONSOLE_KEY_LEFT           0x4B00

[...]

> > #define GRUB_CONSOLE_KEY_PPAGE          0x4900
> > 
> > #endif
> > 
> > and include this file from the other 4 files?
> 
> Sounds fine. Why do we have same thing with TERM and CONSOLE?

Right now, these macros are in these files:
include/grub/powerpc/ieee1275/console.h
include/grub/sparc64/ieee1275/console.h
include/grub/i386/pc/console.h
include/grub/efi/console.h

In sparc64 there is a FIXME above the macros:
/* FIXME (sparc64).  */

I would merge these macros in a file and include this file if all
architectures uses the same codes. But maybe next architecture (I mean,
right know it's powerpc, sparc64, i386 and efi) will use a different
codes. 

What are your opinions, merging to a single file
(grub/console_scancodes.h?) or just leave in different files because one
of the files has a FIXME and maybe our next architecture will not share
these codes?

Sorry for so many questions. I guess that you have had the same question
in other areas in Grub.

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

Re: [PATCH] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[Robert Millan]

On Sun, Aug 24, 2008 at 07:09:24PM +0300, Vesa Jääskeläinen wrote:
> 
> Sounds fine. Why do we have same thing with TERM and CONSOLE?

TERM is for GRUB's internal, arch-independant, key codes.  CONSOLE is for
key codes as provided by i386-pc BIOS.

-- 
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] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[Robert Millan]

On Sun, Aug 24, 2008 at 04:29:44PM +0200, Carles Pina i Estany wrote:
> 
> Hi,
> 
> I was just thinking now...
> 
> On Aug/24/2008, Carles Pina i Estany wrote:
> 
> > 	* 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 was just thinking... is there any reason that we have the same macros
> in 4 different files?

This would need to be checked carefully, but a quick peek at
GRUB_CONSOLE_KEY_LEFT:

./kern/i386/pc/startup.S:       .word   GRUB_CONSOLE_KEY_LEFT, GRUB_TERM_LEFT
./include/grub/i386/pc/console.h:#define GRUB_CONSOLE_KEY_LEFT          0x4B00
./include/grub/efi/console.h:#define GRUB_CONSOLE_KEY_LEFT              0x4B00
./include/grub/powerpc/ieee1275/console.h:#define GRUB_CONSOLE_KEY_LEFT         0x4B00
./include/grub/sparc64/ieee1275/console.h:#define GRUB_CONSOLE_KEY_LEFT         0x4B00

seems to indicate these macros are part of the BIOS console interface,
and therefore only used on i386-pc.

If you can confirm this, I'd recommend just removing them from the other
console.h files.

-- 
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] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[Carles Pina i Estany]

Hi,

On Aug/30/2008, Robert Millan wrote:
> On Sun, Aug 24, 2008 at 04:29:44PM +0200, Carles Pina i Estany wrote:
> > 
> > Hi,
> > 
> > I was just thinking now...
> > 
> > On Aug/24/2008, Carles Pina i Estany wrote:
> > 
> > > 	* 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 was just thinking... is there any reason that we have the same macros
> > in 4 different files?
> 
> This would need to be checked carefully, but a quick peek at
> GRUB_CONSOLE_KEY_LEFT:
> 
> ./kern/i386/pc/startup.S:       .word   GRUB_CONSOLE_KEY_LEFT, GRUB_TERM_LEFT
> ./include/grub/i386/pc/console.h:#define GRUB_CONSOLE_KEY_LEFT          0x4B00
> ./include/grub/efi/console.h:#define GRUB_CONSOLE_KEY_LEFT              0x4B00
> ./include/grub/powerpc/ieee1275/console.h:#define GRUB_CONSOLE_KEY_LEFT         0x4B00
> ./include/grub/sparc64/ieee1275/console.h:#define GRUB_CONSOLE_KEY_LEFT         0x4B00
> 
> seems to indicate these macros are part of the BIOS console interface,
> and therefore only used on i386-pc.
> 
> If you can confirm this, I'd recommend just removing them from the other
> console.h files.

I will also check it and confirm (or not confirm). I will do it on next
days.

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

Re: [PATCH] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE (summary)

[Carles Pina i Estany]

Hello,

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

> ChangeLog:
> --------
> 2008-08-24  Carles Pina i Estany  <carles@pina.cat>
> 
> 	* 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.
> --------
> 
> (this new values are the same used in Grub and also the same that one
> can find googling for PGUP and PGDN key codes).
> 
> If everything is correct, could someone please commit it?

summarizing all discussions about this topic (there is three different
issues):

a) This patch fix the incorrect values of GRUB_TERM_PPAGE/NPAGE. In my
opinion, can be committed (so, feel free to commit if you think so).

b) The name of PPAGE/NPAGE/IC/... are ncurses based and not very
intuitive. Discussing if we should change these names in the same
thread.

c) If it's save merging all places where these macros appears (including
a new file with the values). Discussing in the same thread.

I would go patching or not patching these issues one by one or we will
discuss it forever :-)

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

Re: [PATCH] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE (summary)

[Robert Millan]

On Sun, Aug 24, 2008 at 08:34:10PM +0200, Carles Pina i Estany wrote:
> 
> summarizing all discussions about this topic (there is three different
> issues):
> 
> a) This patch fix the incorrect values of GRUB_TERM_PPAGE/NPAGE. In my
> opinion, can be committed (so, feel free to commit if you think so).

Done.

> b) The name of PPAGE/NPAGE/IC/... are ncurses based and not very
> intuitive. Discussing if we should change these names in the same
> thread.

I'm not concerned about this.  But if Vesa wants to change them, I have no
objection either.

> c) If it's save merging all places where these macros appears (including
> a new file with the values). Discussing in the same thread.

The non-i386 instances of these macros have been removed.

-- 
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] Correcting GRUB_CONSOLE_KEY_NPAGE/PPAGE

[Robert Millan]

On Sun, Aug 24, 2008 at 03:02:16PM +0200, Carles Pina i Estany wrote:
> 
> Hello,
> 
> I send a patch that corrects the GRUB_CONSOLE_KEY_NPAGE/PPAGE constants.
> 
> ChangeLog:
> --------
> 2008-08-24  Carles Pina i Estany  <carles@pina.cat>
> 
> 	* 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.
> --------
> 
> (this new values are the same used in Grub and also the same that one
> can find googling for PGUP and PGDN key codes).
> 
> If everything is correct, could someone please commit it?

Committed (with some adjustments).

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