Re: [PATCH] allow user-configurable menucolor

[Vesa Jääskeläinen]

Robert Millan wrote:
> This patch allows the following to work:
> 
>   set color_normal=cyan/blue
>   set color_highlight=white/blue
> 
> which is equivalent to this command in GRUB Legacy:
> 
>   color cyan/blue white/blue
> 
> I haven't written a ChangeLog entry yet, because I'd like to receive comments
> on the function names.  I don't really like `parse_single_color_name' and
> `parse_color_name' but I don't know what to call them :-(.  What conventions
> are there for referring to single colors (i.e. those that describe only FG or
> only BG, not both) and complete colors (FG + BG) ?

First some questions ;)

- What happens when you forgot to provide / in this string ;) ?

- What happens when all memory is used?

- What happens when you alter list of colors... remove one entry of it
or add one. (eg. hard coding is bad... sizeof...)

Now to your naming issue...

This code is quite specific to be only local so I would propose following:

parse_single_color_name -> parse_color_name

parse_color_name -> parse_color_tuple, parse_color_pair,
parse_text_color or parse_menu_color

Ideally these would be superseded by theming but should work as
temporary code until this feature is implemented. That being said... I
would be too concerned about its naming :)