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/15/19 12:08 AM, David Sterba wrote:
> On Mon, Nov 04, 2019 at 02:33:02PM +0800, Anand Jain wrote:
>>   /*
>> + * Global verbose option for the sub-commands
>> + */
>> +#define HELPINFO_GLOBAL_OPTIONS_HEADER						\
>> +	"",									\
>> +	"Global options:"
>> +#define HELPINFO_INSERT_VERBOSE							\
>> +	"-v|--verbose       show verbose output"
>> +#define HELPINFO_INSERT_QUIET							\
>> +	"-q|--quiet         run the command quietly"
>> +
>> +/*
>>    * Special marker in the help strings that will preemptively insert the global
>>    * options and then continue with the following text that possibly follows
>>    * after the regular options
> 
> I've realized there's more magic around the global options, because
> currently the --format option depends on the subcommand definition thus
> it can't be a static text like you do with the definition of
> HELPINFO_GLOBAL_OPTIONS_HEADER.
> 
> There's a special keyword that gets replaced, the verbose/quite options
> don't need that so it's just the plain textual definition/description.
> 
> 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"

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).

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,
         NULL
-----------------------------------------


Thanks, Anand