Re: [PATCH v1.1 04/18] btrfs-progs: add global verbose and quiet options and helper functions

[Anand Jain <anand.jain@oracle.com>]

On 11/20/19 12:51 AM, David Sterba wrote:
> On Tue, Nov 19, 2019 at 11:36:59AM +0800, Anand Jain wrote:
>>> As this is a simple fixup
>>> s/HELPINFO_GLOBAL_OPTIONS_HEADER/HELPINFO_INSERT_GLOBALS/, hold on with
>>> resending I might find more things or fix it myself.
>>>
>>
>> But there is one problem,  HELPINFO_INSERT_GLOBALS is already defined
>> as..
>>        Global options:
>>       --format TYPE      where TYPE is: text
>>
>> (ref: common/help.c   do_usage_one_command())
>>
>> Albeit all commands support --format text by default.
>>
>> But no sub-command is using the HELPINFO_INSERT_GLOBALS in its usage.
>>
>> Looks like its a good idea to separate title and the options, like
>> #define HELPINFO_INSERT_GLOBALS		"Global options:"
>> #define HELPINGO_INSERT_FORMAT		"--format TYPE"
> 
> Yeah, makes sense to split it, right now the format does not bring
> anything so that will be better done in a major release some time in the
> future when more commands have json output.

  Ok. In v2 I have split
   HELPINFO_GLOBAL_OPTIONS_HEADER into HELPINFO_INSERT_GLOBALS and
   HELPINGO_INSERT_FORMAT as above.

Thanks, Anand


>> As at the moment I am not sure if its safe to declare that all
>> sub-commands will support --format json (whenever we do that).
> 
> No, right now json output should not be declared anywhere.
> 
>> So with the defines split as above, in each sub-command-usage
>> we shall do..
>> 
>> -----------------------------------------
>> diff --git a/cmds/filesystem.c b/cmds/filesystem.c
>> index 4f22089abeaa..f4dba38b4c17 100644
>> --- a/cmds/filesystem.c
>> +++ b/cmds/filesystem.c
>> @@ -631,6 +631,10 @@ static const char * const
>> cmd_filesystem_show_usage[] = {
>>           "-m|--mounted       show only mounted btrfs",
>>           HELPINFO_UNITS_LONG,
>>           "If no argument is given, structure of all present filesystems
>> is shown.",
>> +       HELPINFO_INSERT_GLOBALS,
>> +       HELPINFO_INSERT_FORMAT,
>> +       HELPINFO_INSERT_VERBOSE,
>> +       HELPINFO_INSERT_QUIET,
> 
> Sounds ok.
>