[PATCH] Replace magic numbers by constants

[PATCH] Replace magic numbers by constants

[Carles Pina i Estany]

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


Hello,

When I did the Home/End keys patch I noticied that in normal/menu.c
there was some magic numbers (and some constants was already defined, in
include/grub/term.h (actually, reading the ChangeLog was committed by
Robert Millan on 2008-02-05)

I'm sending a patch to change the magic numbers by constants.

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

	* menu/normal.c (run_menu): Change magic numbers by constants.
--------

Thanks,

PS: thanks Felix for committing the previous patch

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

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

Index: normal/menu.c
===================================================================
--- normal/menu.c	(revision 1815)
+++ normal/menu.c	(working copy)
@@ -421,7 +421,7 @@
 		print_entries (menu, first, offset);
 	      break;
 
-	    case 16:
+	    case GRUB_TERM_UP:
 	    case '^':
 	      if (offset > 0)
 		{
@@ -438,7 +438,7 @@
 		}
 	      break;
 	      
-	    case 14:
+	    case GRUB_TERM_DOWN:
 	    case 'v':
 	      if (menu->size > first + offset + 1)
 		{

Re: [PATCH] Replace magic numbers by constants

[Marco Gerards]

Hi,

Carles Pina i Estany <carles@pina.cat> writes:

> When I did the Home/End keys patch I noticied that in normal/menu.c
> there was some magic numbers (and some constants was already defined, in
> include/grub/term.h (actually, reading the ChangeLog was committed by
> Robert Millan on 2008-02-05)
>
> I'm sending a patch to change the magic numbers by constants.
>
> ChangeLog:
> --------
> 2008-08-17  Carles Pina i Estany  <carles@pina.cat>
>
> 	* menu/normal.c (run_menu): Change magic numbers by constants.

Can you be more specific about what you replace?

Thanks,
Marco

Re: [PATCH] Replace magic numbers by constants

[Carles Pina i Estany]

Hi,

On Aug/17/2008, Marco Gerards wrote:
> Hi,
> 
> Carles Pina i Estany <carles@pina.cat> writes:
> 
> > When I did the Home/End keys patch I noticied that in normal/menu.c
> > there was some magic numbers (and some constants was already defined, in
> > include/grub/term.h (actually, reading the ChangeLog was committed by
> > Robert Millan on 2008-02-05)
> >
> > I'm sending a patch to change the magic numbers by constants.
> >
> > ChangeLog:
> > --------
> > 2008-08-17  Carles Pina i Estany  <carles@pina.cat>
> >
> > 	* menu/normal.c (run_menu): Change magic numbers by constants.
> 
> Can you be more specific about what you replace?

sure:

--------
2008-08-17  Carles Pina i Estany  <carles@pina.cat>

        * menu/normal.c (run_menu): Change magic numbers by constants
	(16 by GRUB_TERM_UP and 14 by GRUB_TERM_DOWN)
--------

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

Re: [PATCH] Replace magic numbers by constants

[Robert Millan]

On Sun, Aug 17, 2008 at 04:29:16PM +0200, Carles Pina i Estany wrote:
> 2008-08-17  Carles Pina i Estany  <carles@pina.cat>
> 
>         * menu/normal.c (run_menu): Change magic numbers by constants
> 	(16 by GRUB_TERM_UP and 14 by GRUB_TERM_DOWN)

When reading the subject I thought it was the other way around.  Would
something like:

  "replace hardcoded numbers with macros"

or

  "macroify numbers"

etc, make it more clear?

-- 
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] Replace magic numbers by constants

[Carles Pina i Estany]

Hello,

On Aug/17/2008, Robert Millan wrote:
> On Sun, Aug 17, 2008 at 04:29:16PM +0200, Carles Pina i Estany wrote:
> > 2008-08-17  Carles Pina i Estany  <carles@pina.cat>
> > 
> >         * menu/normal.c (run_menu): Change magic numbers by constants
> > 	(16 by GRUB_TERM_UP and 14 by GRUB_TERM_DOWN)
> 
> When reading the subject I thought it was the other way around.  Would
> something like:
> 
>   "replace hardcoded numbers with macros"
> 
> or
> 
>   "macroify numbers"

Ok:

--------
2008-08-17  Carles Pina i Estany  <carles@pina.cat>

        * menu/normal.c (run_menu): Replace hardcoded numbers with macros
	(16 for GRUB_TERM_UP and 14 for GRUB_TERM_DOWN)
--------

> etc, make it more clear?

An English note: In my previous mail I've confused "change ... by" 
with "change ... for" :-)

Thanks,

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

Re: [PATCH] Replace magic numbers by constants

[Carles Pina i Estany]

Hola,

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


> > When reading the subject I thought it was the other way around.  Would
> > something like:
> > 
> >   "replace hardcoded numbers with macros"
> > 
> > or
> > 
> >   "macroify numbers"
> 
> Ok:
> 
> --------
> 2008-08-17  Carles Pina i Estany  <carles@pina.cat>
> 
>         * menu/normal.c (run_menu): Replace hardcoded numbers with macros
> 	(16 for GRUB_TERM_UP and 14 for GRUB_TERM_DOWN)
> --------
> 
> > etc, make it more clear?
> 
> An English note: In my previous mail I've confused "change ... by" 
> with "change ... for" :-)

any other change is needed? (patch or changelog) or patch is not needed
and then i will forget about it :-)

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

Re: [PATCH] Replace magic numbers by constants

[Robert Millan]

On Sun, Aug 17, 2008 at 09:11:29PM +0200, Carles Pina i Estany wrote:
> Ok:
> 
> --------
> 2008-08-17  Carles Pina i Estany  <carles@pina.cat>
> 
>         * menu/normal.c (run_menu): Replace hardcoded numbers with macros
> 	(16 for GRUB_TERM_UP and 14 for GRUB_TERM_DOWN)
> --------

Thanks, I checked this in.

Btw, were you going to add support for PageUp / PageDown as well? :-)

-- 
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] Replace magic numbers by constants

[Carles Pina i Estany]

Hi,

On Aug/20/2008, Robert Millan wrote:
> On Sun, Aug 17, 2008 at 09:11:29PM +0200, Carles Pina i Estany wrote:
> > Ok:
> > 
> > --------
> > 2008-08-17  Carles Pina i Estany  <carles@pina.cat>
> > 
> >         * menu/normal.c (run_menu): Replace hardcoded numbers with macros
> > 	(16 for GRUB_TERM_UP and 14 for GRUB_TERM_DOWN)
> > --------
> 
> Thanks, I checked this in.
> 
> Btw, were you going to add support for PageUp / PageDown as well? :-)

Yes, that was my plan.

Today (probably) I will write a proposal of how would behave (how much
lines would scroll, which physical line number would be the new selected
-last one of the screen, first one-). When we agree (if we do :-) ) I
will write the patch (hopefully on weekend).

I will also start a new mailing thread.

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