Re: New commands (reboot, halt, help)

[Marco Gerards]

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

> On Friday 28 January 2005 13:36, Marco Gerards wrote:
>> +static int
>> +print_command_info (grub_command_t cmd)
>> +{
>> +  if (cmd->flags & GRUB_COMMAND_FLAG_CMDLINE
>> +      || cmd->flags & GRUB_COMMAND_FLAG_BOTH)
>> +    grub_printf ("%s\t\t%s\n", cmd->name, cmd->description);
>> +  return 0;
>> +}
>
> Why do you test the flags with GRUB_COMMAND_FLAG_BOTH?

Not all commands, for example `title', should be printed.

> BTW, I prefer a two-column format like GRUB Legacy and BASH. Otherwise, 
> it can emit too many lines.

This is a two column format.  The output is, for example:

boot            Boot an operating system
cat             Show the contents of a file
cmp             Compare two files
help            Shows a help message
insmod          Insert a module.
loopback        Makes a device of a file
ls              List devices and files
lsmod           Show loaded modules.
rescue          Enter into the rescue mode.
rmmod           Remove a module.
set             Set an environment variable.
terminal        Select a terminal.
unset           Remove an environment variable.


>> +static grub_err_t
>> +grub_cmd_help (struct grub_arg_list *state __attribute__ ((unused)),
>> +           int argc __attribute__ ((unused)),
>> +           char **args __attribute__ ((unused)))
>> +
>> +{
>> +  grub_printf ("All GRUB commands accept `--help'.\n\n");
>> +
>> +  grub_iterate_commands (print_command_info);
>> +  return 0;
>> +}
>
> Please make it possible to specify arguments (e.g help ls).

Ok.

>> +  grub_register_command ("help", grub_cmd_help,
>> GRUB_COMMAND_FLAG_BOTH, +                     "help", "Shows a help 
>> message", 0);
>> +}
>
> I think GRUB_COMMAND_FLAG_CMDLINE should be used for this command. I 
> cannot think of any case where help is useful in the menu interface.

Ok.

>> EXPORT_FUNC(grub_register_command)
>>  void EXPORT_FUNC(grub_unregister_command) (const char *name);
>>  grub_command_t grub_command_find (char *cmdline);
>>  grub_err_t grub_set_history (int newsize);
>> -int grub_iterate_commands (int (*iterate) (grub_command_t));
>> +int EXPORT_FUNC(grub_iterate_commands) (int (*iterate)
>> (grub_command_t)); int grub_command_execute (char *cmdline);
>>  void grub_command_init (void);
>>  void grub_normal_init_page (void);
>
> This is strange. You don't have to specify EXPORT_FUNC in normal.h, 
> because the function is defined in a module but not in a kernel. 
> EXPORT_* are required only by the kernel (because the kernel may not be 
> an ELF object).

Ok.  Perhaps I was confused because the other functions were exported.

Thanks,
Marco