Re: Argument parser

[Marco Gerards]

"Yoshinori K. Okuji" <address@hidden> writes:

> Some random ideas about your code:
> 
> * What is ARG_TYPE_CHAR? In what case would it be useful?

It's just like all other ARG_TYPE_'s.  It checks if the used argument
is of the right type.  So the argument parser can check if the
arguments are of the right type.  In this case it checks for a single
char.  Does this answer your question or did I misunderstand you?

> * ARG_TYPE_INT uses strtoul... It sounds odd, because I myself think
>   of _signed_ int when seeing just "int".

Right.  I have used strtoul out of laziness for now.  I will write
pupa_strtol or change this into ARG_TYPE_UINT.  To me it seems
ARG_TYPE_UINT is ok, what do you think?

> * It seems not to support concatenating multiple single-character
>   options (e.g. "-zxvf").

Right!  I forgot this.

> * Does it support normal arguments, like the "foobar" in "ls foobar"?

It does.

> * shortarg should have the type int instead of char.

I can do this.  Why do you want this?

> Also, I'd like to suggest an additional idea. Basically, my goal is to 
> reduce code which must be written for each command. So your parser 
> should be integrated with the command-line engine fully, so that your 
> parser is invoked automatically by the engine, instead of calling your 
> parser explicitly from a function of each command.

I agree with you about reducing the code.

> For example, you can extend the pupa_register_command to accept 
> information on options of a registered command. Then, probably it is 
> also necessary to change the arguments to a callback function for a 
> command.

I'm not sure what you mean with this callback function.
 
> Please see this psuedo code:
> 
> static struct pupa_option_information = 
> {
>   // The default is 1.
>   { 'c', "count", "1", "Specify how many times ...", PUPA_TYPE_UINT },
>   { 0 }
> }
> options;
> 
> pupa_cmd_hello(opts, args)
> {
>   // 1 is the default value.
>   count = pupa_atoi(pupa_parser_get_option (opts, 'c'));
>   // Say it now!
>   while (count-- > 0)
>     pupa_printf ("Hello World\n");
> }

To be honest, I have my doubts about using something like
"pupa_parser_get_option".  There is not much code required for this
parser callback function.  And this pupa_parser_get_options will be
much harder to write.  To me it seems the best that the command is
told which options there are, it shouldn't ask for them.  Perhaps I
just worked too often with argp and I got brainwashed by it. :)

What about adding the struct hello_arguments (for example) to the
options structure and using this to tell the command which options are
used (see my example in my previous mail now this works).  If this
doesn't make sense to you I can show you some code to demonstrate
this.

It would make sense to call the command (pupa_cmd_hello for example)
with a filled argument structure in this case.  However, is it
required to call it with a list of remaining arguments (like 'foo' in
'ls foo')?  This is only a good idea if this is used because it is a
lot of work to implement.  Even if this isn't implemented it is
possible to parse those options with "option_parser".

> init()
> {
>   pupa_register_command ("hello", pupa_cmd_hello, &options);
> }

Right.

> I'm sorry that I didn't reply very soon. In my town, we had a big 
> festival, so I was enjoying with another thing (I was about dying 
> actually, though.)


Oh,  don't  worry.  I'm  really  happy you  replied  and  it was  this
detailed. :)

I have started working on normal mode today.  Can you explain me even
a bit more how you'd like to see it?

I expect the main job is just to implement all commands GRUB has.
Should the user load all these commands by just loading them by adding
them to the core image or by loading them using pupa.conf?  Do you
want to make a subdirectory for every command in the PUPA sourcetree
like it was done for "hello"?

Is it ok to make the commands a bit more feature rich than the GRUB
commands?  For example the ls command can have the following
arguments:

ls [OPTIONS...] [DIR]
List devices and files

-l, --long              Show a long list with more information
-h, --human-readable            Print sizes in a human readable format
-a, --all               List all files
    --more              Wait for a keypress after each displayed screen
-?, --help              Display help
-u, --usage             Show how to use this command

As you can see it has more options than the rescue ls version.  I like
having much (not too much) functionality, I hope you don't think this
is bloated.

Are there some GRUB commands that shouldn't be implemented?  I had a
look at the GRUB commands and some were a bit confusing, for example:
fstest, testload, ioprobe, etc.  I'd appreciate any information about
the commands that should be added and how you'd like to see this.

I hope this mail makes sense to you.  I'm really tired and I shouldn't
send mails when I am tired.

Thanks,
Marco