read_config_file() is missleading

read_config_file() is missleading

[Robert Millan]

The following function name:

  normal/main.c:read_config_file (const char *config, int nested)

is missleading.  It does actually execute the config file, not just
read it.  Does it seem fine to rename it?

How about `process_config_file' ?

Or maybe I am missing something.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

Re: read_config_file() is missleading

[Yoshinori K. Okuji]

On Friday 01 February 2008 20:58, Robert Millan wrote:
> The following function name:
>
>   normal/main.c:read_config_file (const char *config, int nested)
>
> is missleading.  It does actually execute the config file, not just
> read it.  Does it seem fine to rename it?
>
> How about `process_config_file' ?
>
> Or maybe I am missing something.

I think the initial idea was that read_config_file would only read a file, 
then a menu would be executed by another function. But it is not for some 
reason. I forget why. Maybe it didn't make sense to separate the 
functionality into two functions. I am not sure.

If you can think of any reason that we should separate the functionality, you 
can make process_config or something like that. Otherwise, it is fine for me 
to rename the function.

Okuji

Re: read_config_file() is missleading

[Yoshinori K. Okuji]

On Saturday 02 February 2008 13:14, Yoshinori K. Okuji wrote:
> On Friday 01 February 2008 20:58, Robert Millan wrote:
> > The following function name:
> >
> >   normal/main.c:read_config_file (const char *config, int nested)
> >
> > is missleading.  It does actually execute the config file, not just
> > read it.  Does it seem fine to rename it?
> >
> > How about `process_config_file' ?
> >
> > Or maybe I am missing something.
>
> I think the initial idea was that read_config_file would only read a file,
> then a menu would be executed by another function. But it is not for some
> reason. I forget why. Maybe it didn't make sense to separate the
> functionality into two functions. I am not sure.
>
> If you can think of any reason that we should separate the functionality,
> you can make process_config or something like that. Otherwise, it is fine
> for me to rename the function.

I was mistaken. It really depends on the meaning of "execute". For me, 
executing a menu means analyzing the menu structure, displaying it, 
controlling user inputs, and so on. read_menu_config does not execute a menu 
in this sense.

But, of course, it executes commands in a config file.

Okuji

Re: read_config_file() is missleading

[Robert Millan]

On Sat, Feb 02, 2008 at 01:20:04PM +0100, Yoshinori K. Okuji wrote:
> 
> I was mistaken. It really depends on the meaning of "execute". For me, 
> executing a menu means analyzing the menu structure, displaying it, 
> controlling user inputs, and so on. read_menu_config does not execute a menu 
> in this sense.
> 
> But, of course, it executes commands in a config file.

Still, "read" is missleading, as it does more than reading.  If you add
"echo foo" to grub.cfg, this function will print "foo".

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)