[PATCH 1/2] add advanced use of --help to help message

[PATCH 1/2] add advanced use of --help to help message

[Hubert Kario]

explain how to use

        btrfs <cmd> --help

command in help message

Signed-off-by: Hubert Kario <kario@wit.edu.pl>
---
 btrfs.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index 46314cf..b84607a 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -151,6 +151,8 @@ static void help(char *np)
 		print_help(np, cp);
 
 	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
+	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command 
or\n\t\t"
+            "subset of commands.\n",np);
 	printf("\n%s\n", BTRFS_BUILD_VERSION);
 }
 
-- 
1.7.3.5

[PATCH 2/2] add detailed help messages to btrfs command

[Hubert Kario]

extend the

        btrfs <cmd> --help

command to print detailed help message if available but fallback to
basic help message if detailed is unavailable

add detailed help message for 'filesystem defragment' command

little tweaks in comments

Signed-off-by: Hubert Kario <kario@wit.edu.pl>
---
 btrfs.c |  101 ++++++++++++++++++++++++++++++++++++++++++--------------------
 1 files changed, 68 insertions(+), 33 deletions(-)

diff --git a/btrfs.c b/btrfs.c
index b84607a..bd6f6f8 100644
--- a/btrfs.c
+++ b/btrfs.c
@@ -23,6 +23,9 @@
 #include "btrfs_cmds.h"
 #include "version.h"
 
+#define BASIC_HELP 0
+#define ADVANCED_HELP 1
+
 typedef int (*CommandFunction)(int argc, char **argv);
 
 struct Command {
@@ -31,8 +34,10 @@ struct Command {
 				   if >= 0, number of arguments,
 				   if < 0, _minimum_ number of arguments */
 	char	*verb;		/* verb */
-	char	*help;		/* help lines; form the 2nd onward they are
-				   indented */
+	char	*help;		/* help lines; from the 2nd line onward they 
+                                   are automatically indented */
+        char    *adv_help;      /* advanced help message; from the 2nd line 
+                                   onward they are automatically indented */
 
 	/* the following fields are run-time filled by the program */
 	char	**cmds;		/* array of subcommands */
@@ -47,73 +52,96 @@ static struct Command commands[] = {
 	{ do_clone, 2,
 	  "subvolume snapshot", "<source> [<dest>/]<name>\n"
 		"Create a writable snapshot of the subvolume <source> with\n"
-		"the name <name> in the <dest> directory."
+		"the name <name> in the <dest> directory.",
+          NULL
 	},
 	{ do_delete_subvolume, 1,
 	  "subvolume delete", "<subvolume>\n"
-		"Delete the subvolume <subvolume>."
+		"Delete the subvolume <subvolume>.",
+          NULL
 	},
 	{ do_create_subvol, 1,
 	  "subvolume create", "[<dest>/]<name>\n"
 		"Create a subvolume in <dest> (or the current directory if\n"
-		"not passed)."
+		"not passed).",
+          NULL
 	},
 	{ do_subvol_list, 1, "subvolume list", "<path>\n"
-		"List the snapshot/subvolume of a filesystem."
+		"List the snapshot/subvolume of a filesystem.",
+          NULL
 	},
 	{ do_find_newer, 2, "subvolume find-new", "<path> <last_gen>\n"
-		"List the recently modified files in a filesystem."
+		"List the recently modified files in a filesystem.",
+          NULL
 	},
 	{ do_defrag, -1,
 	  "filesystem defragment", "[-vcf] [-s start] [-l len] [-t size] <file>|<dir> [<file>|<dir>...]\n"
-		"Defragment a file or a directory."
+		"Defragment a file or a directory.",
+          "[-vcf] [-s start] [-l len] [-t size] <file>|<dir> [<file>|<dir>...]\n"
+          "Defragment file data or directory metadata.\n"
+                "-v         be verbose\n"
+                "-c         compress the file while defragmenting\n"
+                "-f         flush data to disk immediately after defragmenting\n"
+                "-s start   defragment only from byte onward\n"
+                "-l len     defragment only up to len bytes\n"
+                "-t size    minimal size of file to be considered for defragmenting\n"
 	},
 	{ do_set_default_subvol, 2,
 	  "subvolume set-default", "<id> <path>\n"
 		"Set the subvolume of the filesystem <path> which will be mounted\n"
-		"as default."
+		"as default.",
+          NULL
 	},
 	{ do_fssync, 1,
 	  "filesystem sync", "<path>\n"
-		"Force a sync on the filesystem <path>."
+		"Force a sync on the filesystem <path>.",
+          NULL
 	},
 	{ do_resize, 2,
 	  "filesystem resize", "[+/-]<newsize>[gkm]|max <filesystem>\n"
 		"Resize the file system. If 'max' is passed, the filesystem\n"
-		"will occupe all available space on the device."
+		"will occupe all available space on the device.",
+          NULL
 	},
 	{ do_show_filesystem, 999,
 	  "filesystem show", "[<uuid>|<label>]\n"
 		"Show the info of a btrfs filesystem. If no <uuid> or <label>\n"
-		"is passed, info of all the btrfs filesystem are shown."
+		"is passed, info of all the btrfs filesystem are shown.",
+          NULL
 	},
 	{ do_df_filesystem, 1,
 	  "filesystem df", "<path>\n"
-		"Show space usage information for a mount point\n."
+		"Show space usage information for a mount point.",
+          NULL
 	},
 	{ do_balance, 1,
 	  "filesystem balance", "<path>\n"
-		"Balance the chunks across the device."
+		"Balance the chunks across the device.",
+          NULL
 	},
 	{ do_scan,
 	  999, "device scan", "[<device> [<device>..]\n"
 		"Scan all device for or the passed device for a btrfs\n"
-		"filesystem."
+		"filesystem.",
+          NULL
 	},
 	{ do_add_volume, -2,
 	  "device add", "<dev> [<dev>..] <path>\n"
-		"Add a device to a filesystem."
+		"Add a device to a filesystem.",
+          NULL
 	},
 	{ do_remove_volume, -2,
 	  "device delete", "<dev> [<dev>..] <path>\n"
-		"Remove a device from a filesystem."
+		"Remove a device from a filesystem.",
+          NULL
 	},
 	/* coming soon
 	{ 2, "filesystem label", "<label> <path>\n"
-		"Set the label of a filesystem"
+		"Set the label of a filesystem",
+          NULL
 	}
 	*/
-	{ 0, 0 , 0 }
+	{ 0, 0, 0, 0 }
 };
 
 static char *get_prgname(char *programname)
@@ -128,17 +156,25 @@ static char *get_prgname(char *programname)
 	return np;
 }
 
-static void print_help(char *programname, struct Command *cmd)
+static void print_help(char *programname, struct Command *cmd, int helptype)
 {
 	char	*pc;
 
 	printf("\t%s %s ", programname, cmd->verb );
 
-	for(pc = cmd->help; *pc; pc++){
-		putchar(*pc);
-		if(*pc == '\n')
-			printf("\t\t");
-	}
+        if (helptype == ADVANCED_HELP && cmd->adv_help)
+                for(pc = cmd->adv_help; *pc; pc++){
+                        putchar(*pc);
+                        if(*pc == '\n')
+                                printf("\t\t");
+                }
+        else
+	        for(pc = cmd->help; *pc; pc++){
+		        putchar(*pc);
+		        if(*pc == '\n')
+			        printf("\t\t");
+	        }
+
 	putchar('\n');
 }
 
@@ -148,10 +184,10 @@ static void help(char *np)
 
 	printf("Usage:\n");
 	for( cp = commands; cp->verb; cp++ )
-		print_help(np, cp);
+		print_help(np, cp, BASIC_HELP);
 
 	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
-	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command or\n\t\t"
+	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command or"
             "subset of commands.\n",np);
 	printf("\n%s\n", BTRFS_BUILD_VERSION);
 }
@@ -257,15 +293,14 @@ static int prepare_args(int *ac, char ***av, char *prgname, struct Command *cmd
 
 
 /*
-
-	This function perform the following jobs:
+	This function performs the following jobs:
 	- show the help if '--help' or 'help' or '-h' are passed
 	- verify that a command is not ambiguous, otherwise show which
 	  part of the command is ambiguous
-	- if after a (even partial) command there is '--help' show the help
+	- if after a (even partial) command there is '--help' show detailed help
 	  for all the matching commands
-	- if the command doesn't' match show an error
-	- finally, if a command match, they return which command is matched and
+	- if the command doesn't match show an error
+	- finally, if a command matches, they return which command matched and
 	  the arguments
 
 	The function return 0 in case of help is requested; <0 in case
@@ -319,7 +354,7 @@ static int parse_args(int argc, char **argv,
 		if(argc>i+1 && !strcmp(argv[i+1],"--help")){
 			if(!helprequested)
 				printf("Usage:\n");
-			print_help(prgname, cp);
+			print_help(prgname, cp, ADVANCED_HELP);
 			helprequested=1;
 			continue;
 		}
-- 
1.7.3.5

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Goffredo Baroncelli]

Hello Hubert,

please update the man page too.

Regards
G.Baroncelli

On 01/23/2011 01:42 PM, Hubert Kario wrote:
> extend the
> 
>         btrfs <cmd> --help
> 
> command to print detailed help message if available but fallback to
> basic help message if detailed is unavailable
> 
> add detailed help message for 'filesystem defragment' command
> 
> little tweaks in comments
> 
> Signed-off-by: Hubert Kario <kario@wit.edu.pl>
> ---
>  btrfs.c |  101 ++++++++++++++++++++++++++++++++++++++++++--------------------
>  1 files changed, 68 insertions(+), 33 deletions(-)
> 
> diff --git a/btrfs.c b/btrfs.c
> index b84607a..bd6f6f8 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -23,6 +23,9 @@
>  #include "btrfs_cmds.h"
>  #include "version.h"
>  
> +#define BASIC_HELP 0
> +#define ADVANCED_HELP 1
> +
>  typedef int (*CommandFunction)(int argc, char **argv);
>  
>  struct Command {
> @@ -31,8 +34,10 @@ struct Command {
>  				   if >= 0, number of arguments,
>  				   if < 0, _minimum_ number of arguments */
>  	char	*verb;		/* verb */
> -	char	*help;		/* help lines; form the 2nd onward they are
> -				   indented */
> +	char	*help;		/* help lines; from the 2nd line onward they 
> +                                   are automatically indented */
> +        char    *adv_help;      /* advanced help message; from the 2nd line 
> +                                   onward they are automatically indented */
>  
>  	/* the following fields are run-time filled by the program */
>  	char	**cmds;		/* array of subcommands */
> @@ -47,73 +52,96 @@ static struct Command commands[] = {
>  	{ do_clone, 2,
>  	  "subvolume snapshot", "<source> [<dest>/]<name>\n"
>  		"Create a writable snapshot of the subvolume <source> with\n"
> -		"the name <name> in the <dest> directory."
> +		"the name <name> in the <dest> directory.",
> +          NULL
>  	},
>  	{ do_delete_subvolume, 1,
>  	  "subvolume delete", "<subvolume>\n"
> -		"Delete the subvolume <subvolume>."
> +		"Delete the subvolume <subvolume>.",
> +          NULL
>  	},
>  	{ do_create_subvol, 1,
>  	  "subvolume create", "[<dest>/]<name>\n"
>  		"Create a subvolume in <dest> (or the current directory if\n"
> -		"not passed)."
> +		"not passed).",
> +          NULL
>  	},
>  	{ do_subvol_list, 1, "subvolume list", "<path>\n"
> -		"List the snapshot/subvolume of a filesystem."
> +		"List the snapshot/subvolume of a filesystem.",
> +          NULL
>  	},
>  	{ do_find_newer, 2, "subvolume find-new", "<path> <last_gen>\n"
> -		"List the recently modified files in a filesystem."
> +		"List the recently modified files in a filesystem.",
> +          NULL
>  	},
>  	{ do_defrag, -1,
>  	  "filesystem defragment", "[-vcf] [-s start] [-l len] [-t size] <file>|<dir> [<file>|<dir>...]\n"
> -		"Defragment a file or a directory."
> +		"Defragment a file or a directory.",
> +          "[-vcf] [-s start] [-l len] [-t size] <file>|<dir> [<file>|<dir>...]\n"
> +          "Defragment file data or directory metadata.\n"
> +                "-v         be verbose\n"
> +                "-c         compress the file while defragmenting\n"
> +                "-f         flush data to disk immediately after defragmenting\n"
> +                "-s start   defragment only from byte onward\n"
> +                "-l len     defragment only up to len bytes\n"
> +                "-t size    minimal size of file to be considered for defragmenting\n"
>  	},
>  	{ do_set_default_subvol, 2,
>  	  "subvolume set-default", "<id> <path>\n"
>  		"Set the subvolume of the filesystem <path> which will be mounted\n"
> -		"as default."
> +		"as default.",
> +          NULL
>  	},
>  	{ do_fssync, 1,
>  	  "filesystem sync", "<path>\n"
> -		"Force a sync on the filesystem <path>."
> +		"Force a sync on the filesystem <path>.",
> +          NULL
>  	},
>  	{ do_resize, 2,
>  	  "filesystem resize", "[+/-]<newsize>[gkm]|max <filesystem>\n"
>  		"Resize the file system. If 'max' is passed, the filesystem\n"
> -		"will occupe all available space on the device."
> +		"will occupe all available space on the device.",
> +          NULL
>  	},
>  	{ do_show_filesystem, 999,
>  	  "filesystem show", "[<uuid>|<label>]\n"
>  		"Show the info of a btrfs filesystem. If no <uuid> or <label>\n"
> -		"is passed, info of all the btrfs filesystem are shown."
> +		"is passed, info of all the btrfs filesystem are shown.",
> +          NULL
>  	},
>  	{ do_df_filesystem, 1,
>  	  "filesystem df", "<path>\n"
> -		"Show space usage information for a mount point\n."
> +		"Show space usage information for a mount point.",
> +          NULL
>  	},
>  	{ do_balance, 1,
>  	  "filesystem balance", "<path>\n"
> -		"Balance the chunks across the device."
> +		"Balance the chunks across the device.",
> +          NULL
>  	},
>  	{ do_scan,
>  	  999, "device scan", "[<device> [<device>..]\n"
>  		"Scan all device for or the passed device for a btrfs\n"
> -		"filesystem."
> +		"filesystem.",
> +          NULL
>  	},
>  	{ do_add_volume, -2,
>  	  "device add", "<dev> [<dev>..] <path>\n"
> -		"Add a device to a filesystem."
> +		"Add a device to a filesystem.",
> +          NULL
>  	},
>  	{ do_remove_volume, -2,
>  	  "device delete", "<dev> [<dev>..] <path>\n"
> -		"Remove a device from a filesystem."
> +		"Remove a device from a filesystem.",
> +          NULL
>  	},
>  	/* coming soon
>  	{ 2, "filesystem label", "<label> <path>\n"
> -		"Set the label of a filesystem"
> +		"Set the label of a filesystem",
> +          NULL
>  	}
>  	*/
> -	{ 0, 0 , 0 }
> +	{ 0, 0, 0, 0 }
>  };
>  
>  static char *get_prgname(char *programname)
> @@ -128,17 +156,25 @@ static char *get_prgname(char *programname)
>  	return np;
>  }
>  
> -static void print_help(char *programname, struct Command *cmd)
> +static void print_help(char *programname, struct Command *cmd, int helptype)
>  {
>  	char	*pc;
>  
>  	printf("\t%s %s ", programname, cmd->verb );
>  
> -	for(pc = cmd->help; *pc; pc++){
> -		putchar(*pc);
> -		if(*pc == '\n')
> -			printf("\t\t");
> -	}
> +        if (helptype == ADVANCED_HELP && cmd->adv_help)
> +                for(pc = cmd->adv_help; *pc; pc++){
> +                        putchar(*pc);
> +                        if(*pc == '\n')
> +                                printf("\t\t");
> +                }
> +        else
> +	        for(pc = cmd->help; *pc; pc++){
> +		        putchar(*pc);
> +		        if(*pc == '\n')
> +			        printf("\t\t");
> +	        }
> +
>  	putchar('\n');
>  }
>  
> @@ -148,10 +184,10 @@ static void help(char *np)
>  
>  	printf("Usage:\n");
>  	for( cp = commands; cp->verb; cp++ )
> -		print_help(np, cp);
> +		print_help(np, cp, BASIC_HELP);
>  
>  	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
> -	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command or\n\t\t"
> +	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command or"
>              "subset of commands.\n",np);
>  	printf("\n%s\n", BTRFS_BUILD_VERSION);
>  }
> @@ -257,15 +293,14 @@ static int prepare_args(int *ac, char ***av, char *prgname, struct Command *cmd
>  
>  
>  /*
> -
> -	This function perform the following jobs:
> +	This function performs the following jobs:
>  	- show the help if '--help' or 'help' or '-h' are passed
>  	- verify that a command is not ambiguous, otherwise show which
>  	  part of the command is ambiguous
> -	- if after a (even partial) command there is '--help' show the help
> +	- if after a (even partial) command there is '--help' show detailed help
>  	  for all the matching commands
> -	- if the command doesn't' match show an error
> -	- finally, if a command match, they return which command is matched and
> +	- if the command doesn't match show an error
> +	- finally, if a command matches, they return which command matched and
>  	  the arguments
>  
>  	The function return 0 in case of help is requested; <0 in case
> @@ -319,7 +354,7 @@ static int parse_args(int argc, char **argv,
>  		if(argc>i+1 && !strcmp(argv[i+1],"--help")){
>  			if(!helprequested)
>  				printf("Usage:\n");
> -			print_help(prgname, cp);
> +			print_help(prgname, cp, ADVANCED_HELP);
>  			helprequested=1;
>  			continue;
>  		}

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Helmut Hullen]

Hallo, Goffredo,

Du meintest am 23.01.11:


>> extend the
>>
>>         btrfs <cmd> --help
>>
>> command to print detailed help message if available but fallback to
>> basic help message if detailed is unavailable
>>
>> add detailed help message for 'filesystem defragment' command
>>
>> little tweaks in comments

> please update the man page too.


Is the patch part of the actually downloadable source?

Viele Gruesse!
Helmut

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Felix Blanke]

Hi,

no :)

All those patches need to be applied to the current git. I'm sure there will be an
e-mail if all those patches are applied.

Regards,
Felix

On 23. January 2011 - 15:26, Helmut Hullen wrote:
> Date:	23 Jan 2011 15:26:00 +0100
> From: Helmut Hullen <Hullen@t-online.de>
> To: linux-btrfs@vger.kernel.org
> Subject: Re: [PATCH 2/2] add detailed help messages to btrfs command
> 
> Hallo, Goffredo,
> 
> Du meintest am 23.01.11:
> 
> 
> >> extend the
> >>
> >>         btrfs <cmd> --help
> >>
> >> command to print detailed help message if available but fallback to
> >> basic help message if detailed is unavailable
> >>
> >> add detailed help message for 'filesystem defragment' command
> >>
> >> little tweaks in comments
> 
> > please update the man page too.
> 
> 
> Is the patch part of the actually downloadable source?
> 
> Viele Gruesse!
> Helmut
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
---end quoted text---

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Hubert Kario]

On Sunday 23 of January 2011 15:07:09 Goffredo Baroncelli wrote:
> Hello Hubert,
> 
> please update the man page too.

I started to do it but then I noticed that you have made a few changes to the
man page yourself.

I'm quite new to the git/mail patch system of releases and I'm not sure if I 
should post the patch from my tree with your patch applied or not.
You have modified some of the same lines I want to fix and as such my patch 
may not be cleanly applicable. So, which tree I should base my patch on?

Regards
Hubert Kario

> On 01/23/2011 01:42 PM, Hubert Kario wrote:
> > extend the
> > 
> >         btrfs <cmd> --help
> > 
> > command to print detailed help message if available but fallback to
> > basic help message if detailed is unavailable
> > 
> > add detailed help message for 'filesystem defragment' command
> > 
> > little tweaks in comments
> > 
> > Signed-off-by: Hubert Kario <kario@wit.edu.pl>
> > ---
> > 
> >  btrfs.c |  101
> >  ++++++++++++++++++++++++++++++++++++++++++-------------------- 1 files
> >  changed, 68 insertions(+), 33 deletions(-)
> > 
> > diff --git a/btrfs.c b/btrfs.c
> > index b84607a..bd6f6f8 100644
> > --- a/btrfs.c
> > +++ b/btrfs.c
> > @@ -23,6 +23,9 @@
> > 
> >  #include "btrfs_cmds.h"
> >  #include "version.h"
> > 
> > +#define BASIC_HELP 0
> > +#define ADVANCED_HELP 1
> > +
> > 
> >  typedef int (*CommandFunction)(int argc, char **argv);
> >  
> >  struct Command {
> > 
> > @@ -31,8 +34,10 @@ struct Command {
> > 
> >  				   if >= 0, number of arguments,
> >  				   if < 0, _minimum_ number of arguments */
> >  	
> >  	char	*verb;		/* verb */
> > 
> > -	char	*help;		/* help lines; form the 2nd onward they are
> > -				   indented */
> > +	char	*help;		/* help lines; from the 2nd line onward they
> > +                                   are automatically indented */
> > +        char    *adv_help;      /* advanced help message; from the 2nd
> > line +                                   onward they are automatically
> > indented */
> > 
> >  	/* the following fields are run-time filled by the program */
> >  	char	**cmds;		/* array of subcommands */
> > 
> > @@ -47,73 +52,96 @@ static struct Command commands[] = {
> > 
> >  	{ do_clone, 2,
> >  	
> >  	  "subvolume snapshot", "<source> [<dest>/]<name>\n"
> >  		
> >  		"Create a writable snapshot of the subvolume <source> with\n"
> > 
> > -		"the name <name> in the <dest> directory."
> > +		"the name <name> in the <dest> directory.",
> > +          NULL
> > 
> >  	},
> >  	{ do_delete_subvolume, 1,
> >  	
> >  	  "subvolume delete", "<subvolume>\n"
> > 
> > -		"Delete the subvolume <subvolume>."
> > +		"Delete the subvolume <subvolume>.",
> > +          NULL
> > 
> >  	},
> >  	{ do_create_subvol, 1,
> >  	
> >  	  "subvolume create", "[<dest>/]<name>\n"
> >  		
> >  		"Create a subvolume in <dest> (or the current directory if\n"
> > 
> > -		"not passed)."
> > +		"not passed).",
> > +          NULL
> > 
> >  	},
> >  	{ do_subvol_list, 1, "subvolume list", "<path>\n"
> > 
> > -		"List the snapshot/subvolume of a filesystem."
> > +		"List the snapshot/subvolume of a filesystem.",
> > +          NULL
> > 
> >  	},
> >  	{ do_find_newer, 2, "subvolume find-new", "<path> <last_gen>\n"
> > 
> > -		"List the recently modified files in a filesystem."
> > +		"List the recently modified files in a filesystem.",
> > +          NULL
> > 
> >  	},
> >  	{ do_defrag, -1,
> >  	
> >  	  "filesystem defragment", "[-vcf] [-s start] [-l len] [-t size]
> >  	  <file>|<dir> [<file>|<dir>...]\n"
> > 
> > -		"Defragment a file or a directory."
> > +		"Defragment a file or a directory.",
> > +          "[-vcf] [-s start] [-l len] [-t size] <file>|<dir>
> > [<file>|<dir>...]\n" +          "Defragment file data or directory
> > metadata.\n"
> > +                "-v         be verbose\n"
> > +                "-c         compress the file while defragmenting\n"
> > +                "-f         flush data to disk immediately after
> > defragmenting\n" +                "-s start   defragment only from byte
> > onward\n" +                "-l len     defragment only up to len
> > bytes\n"
> > +                "-t size    minimal size of file to be considered for
> > defragmenting\n"
> > 
> >  	},
> >  	{ do_set_default_subvol, 2,
> >  	
> >  	  "subvolume set-default", "<id> <path>\n"
> >  		
> >  		"Set the subvolume of the filesystem <path> which will be 
mounted\n"
> > 
> > -		"as default."
> > +		"as default.",
> > +          NULL
> > 
> >  	},
> >  	{ do_fssync, 1,
> >  	
> >  	  "filesystem sync", "<path>\n"
> > 
> > -		"Force a sync on the filesystem <path>."
> > +		"Force a sync on the filesystem <path>.",
> > +          NULL
> > 
> >  	},
> >  	{ do_resize, 2,
> >  	
> >  	  "filesystem resize", "[+/-]<newsize>[gkm]|max <filesystem>\n"
> >  		
> >  		"Resize the file system. If 'max' is passed, the filesystem\n"
> > 
> > -		"will occupe all available space on the device."
> > +		"will occupe all available space on the device.",
> > +          NULL
> > 
> >  	},
> >  	{ do_show_filesystem, 999,
> >  	
> >  	  "filesystem show", "[<uuid>|<label>]\n"
> >  		
> >  		"Show the info of a btrfs filesystem. If no <uuid> or 
<label>\n"
> > 
> > -		"is passed, info of all the btrfs filesystem are shown."
> > +		"is passed, info of all the btrfs filesystem are shown.",
> > +          NULL
> > 
> >  	},
> >  	{ do_df_filesystem, 1,
> >  	
> >  	  "filesystem df", "<path>\n"
> > 
> > -		"Show space usage information for a mount point\n."
> > +		"Show space usage information for a mount point.",
> > +          NULL
> > 
> >  	},
> >  	{ do_balance, 1,
> >  	
> >  	  "filesystem balance", "<path>\n"
> > 
> > -		"Balance the chunks across the device."
> > +		"Balance the chunks across the device.",
> > +          NULL
> > 
> >  	},
> >  	{ do_scan,
> >  	
> >  	  999, "device scan", "[<device> [<device>..]\n"
> >  		
> >  		"Scan all device for or the passed device for a btrfs\n"
> > 
> > -		"filesystem."
> > +		"filesystem.",
> > +          NULL
> > 
> >  	},
> >  	{ do_add_volume, -2,
> >  	
> >  	  "device add", "<dev> [<dev>..] <path>\n"
> > 
> > -		"Add a device to a filesystem."
> > +		"Add a device to a filesystem.",
> > +          NULL
> > 
> >  	},
> >  	{ do_remove_volume, -2,
> >  	
> >  	  "device delete", "<dev> [<dev>..] <path>\n"
> > 
> > -		"Remove a device from a filesystem."
> > +		"Remove a device from a filesystem.",
> > +          NULL
> > 
> >  	},
> >  	/* coming soon
> >  	{ 2, "filesystem label", "<label> <path>\n"
> > 
> > -		"Set the label of a filesystem"
> > +		"Set the label of a filesystem",
> > +          NULL
> > 
> >  	}
> >  	*/
> > 
> > -	{ 0, 0 , 0 }
> > +	{ 0, 0, 0, 0 }
> > 
> >  };
> >  
> >  static char *get_prgname(char *programname)
> > 
> > @@ -128,17 +156,25 @@ static char *get_prgname(char *programname)
> > 
> >  	return np;
> >  
> >  }
> > 
> > -static void print_help(char *programname, struct Command *cmd)
> > +static void print_help(char *programname, struct Command *cmd, int
> > helptype)
> > 
> >  {
> >  
> >  	char	*pc;
> >  	
> >  	printf("\t%s %s ", programname, cmd->verb );
> > 
> > -	for(pc = cmd->help; *pc; pc++){
> > -		putchar(*pc);
> > -		if(*pc == '\n')
> > -			printf("\t\t");
> > -	}
> > +        if (helptype == ADVANCED_HELP && cmd->adv_help)
> > +                for(pc = cmd->adv_help; *pc; pc++){
> > +                        putchar(*pc);
> > +                        if(*pc == '\n')
> > +                                printf("\t\t");
> > +                }
> > +        else
> > +	        for(pc = cmd->help; *pc; pc++){
> > +		        putchar(*pc);
> > +		        if(*pc == '\n')
> > +			        printf("\t\t");
> > +	        }
> > +
> > 
> >  	putchar('\n');
> >  
> >  }
> > 
> > @@ -148,10 +184,10 @@ static void help(char *np)
> > 
> >  	printf("Usage:\n");
> >  	for( cp = commands; cp->verb; cp++ )
> > 
> > -		print_help(np, cp);
> > +		print_help(np, cp, BASIC_HELP);
> > 
> >  	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
> > 
> > -	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command
> > or\n\t\t" +	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a
> > command or"
> > 
> >              "subset of commands.\n",np);
> >  	
> >  	printf("\n%s\n", BTRFS_BUILD_VERSION);
> >  
> >  }
> > 
> > @@ -257,15 +293,14 @@ static int prepare_args(int *ac, char ***av, char
> > *prgname, struct Command *cmd
> > 
> >  /*
> > 
> > -
> > -	This function perform the following jobs:
> > 
> > +	This function performs the following jobs:
> >  	- show the help if '--help' or 'help' or '-h' are passed
> >  	- verify that a command is not ambiguous, otherwise show which
> >  	
> >  	  part of the command is ambiguous
> > 
> > -	- if after a (even partial) command there is '--help' show the help
> > +	- if after a (even partial) command there is '--help' show detailed
> > help
> > 
> >  	  for all the matching commands
> > 
> > -	- if the command doesn't' match show an error
> > -	- finally, if a command match, they return which command is matched 
and
> > +	- if the command doesn't match show an error
> > +	- finally, if a command matches, they return which command matched and
> > 
> >  	  the arguments
> >  	
> >  	The function return 0 in case of help is requested; <0 in case
> > 
> > @@ -319,7 +354,7 @@ static int parse_args(int argc, char **argv,
> > 
> >  		if(argc>i+1 && !strcmp(argv[i+1],"--help")){
> >  		
> >  			if(!helprequested)
> >  			
> >  				printf("Usage:\n");
> > 
> > -			print_help(prgname, cp);
> > +			print_help(prgname, cp, ADVANCED_HELP);
> > 
> >  			helprequested=1;
> >  			continue;
> >  		
> >  		}
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Goffredo Baroncelli]

There are a lot of patches regarding the btrfs-tool.
Unfortunately, the btrfs maintainer are very busy in other areas of the
project. I don't know when (if) these patches will be applied.
However I think that is better have a misaligned patch instead nothing.

Regards
G.Baroncelli


On 01/23/2011 03:28 PM, Hubert Kario wrote:
> On Sunday 23 of January 2011 15:07:09 Goffredo Baroncelli wrote:
>> Hello Hubert,
>>
>> please update the man page too.
> 
> I started to do it but then I noticed that you have made a few changes to the
> man page yourself.
> 
> I'm quite new to the git/mail patch system of releases and I'm not sure if I 
> should post the patch from my tree with your patch applied or not.
> You have modified some of the same lines I want to fix and as such my patch 
> may not be cleanly applicable. So, which tree I should base my patch on?
> 
> Regards
> Hubert Kario
> 
>> On 01/23/2011 01:42 PM, Hubert Kario wrote:
>>> extend the
>>>
>>>         btrfs <cmd> --help
>>>
>>> command to print detailed help message if available but fallback to
>>> basic help message if detailed is unavailable
>>>
>>> add detailed help message for 'filesystem defragment' command
>>>
>>> little tweaks in comments
>>>
>>> Signed-off-by: Hubert Kario <kario@wit.edu.pl>
>>> ---
>>>
>>>  btrfs.c |  101
>>>  ++++++++++++++++++++++++++++++++++++++++++-------------------- 1 files
>>>  changed, 68 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/btrfs.c b/btrfs.c
>>> index b84607a..bd6f6f8 100644
>>> --- a/btrfs.c
>>> +++ b/btrfs.c
>>> @@ -23,6 +23,9 @@
>>>
>>>  #include "btrfs_cmds.h"
>>>  #include "version.h"
>>>
>>> +#define BASIC_HELP 0
>>> +#define ADVANCED_HELP 1
>>> +
>>>
>>>  typedef int (*CommandFunction)(int argc, char **argv);
>>>  
>>>  struct Command {
>>>
>>> @@ -31,8 +34,10 @@ struct Command {
>>>
>>>  				   if >= 0, number of arguments,
>>>  				   if < 0, _minimum_ number of arguments */
>>>  	
>>>  	char	*verb;		/* verb */
>>>
>>> -	char	*help;		/* help lines; form the 2nd onward they are
>>> -				   indented */
>>> +	char	*help;		/* help lines; from the 2nd line onward they
>>> +                                   are automatically indented */
>>> +        char    *adv_help;      /* advanced help message; from the 2nd
>>> line +                                   onward they are automatically
>>> indented */
>>>
>>>  	/* the following fields are run-time filled by the program */
>>>  	char	**cmds;		/* array of subcommands */
>>>
>>> @@ -47,73 +52,96 @@ static struct Command commands[] = {
>>>
>>>  	{ do_clone, 2,
>>>  	
>>>  	  "subvolume snapshot", "<source> [<dest>/]<name>\n"
>>>  		
>>>  		"Create a writable snapshot of the subvolume <source> with\n"
>>>
>>> -		"the name <name> in the <dest> directory."
>>> +		"the name <name> in the <dest> directory.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_delete_subvolume, 1,
>>>  	
>>>  	  "subvolume delete", "<subvolume>\n"
>>>
>>> -		"Delete the subvolume <subvolume>."
>>> +		"Delete the subvolume <subvolume>.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_create_subvol, 1,
>>>  	
>>>  	  "subvolume create", "[<dest>/]<name>\n"
>>>  		
>>>  		"Create a subvolume in <dest> (or the current directory if\n"
>>>
>>> -		"not passed)."
>>> +		"not passed).",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_subvol_list, 1, "subvolume list", "<path>\n"
>>>
>>> -		"List the snapshot/subvolume of a filesystem."
>>> +		"List the snapshot/subvolume of a filesystem.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_find_newer, 2, "subvolume find-new", "<path> <last_gen>\n"
>>>
>>> -		"List the recently modified files in a filesystem."
>>> +		"List the recently modified files in a filesystem.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_defrag, -1,
>>>  	
>>>  	  "filesystem defragment", "[-vcf] [-s start] [-l len] [-t size]
>>>  	  <file>|<dir> [<file>|<dir>...]\n"
>>>
>>> -		"Defragment a file or a directory."
>>> +		"Defragment a file or a directory.",
>>> +          "[-vcf] [-s start] [-l len] [-t size] <file>|<dir>
>>> [<file>|<dir>...]\n" +          "Defragment file data or directory
>>> metadata.\n"
>>> +                "-v         be verbose\n"
>>> +                "-c         compress the file while defragmenting\n"
>>> +                "-f         flush data to disk immediately after
>>> defragmenting\n" +                "-s start   defragment only from byte
>>> onward\n" +                "-l len     defragment only up to len
>>> bytes\n"
>>> +                "-t size    minimal size of file to be considered for
>>> defragmenting\n"
>>>
>>>  	},
>>>  	{ do_set_default_subvol, 2,
>>>  	
>>>  	  "subvolume set-default", "<id> <path>\n"
>>>  		
>>>  		"Set the subvolume of the filesystem <path> which will be 
> mounted\n"
>>>
>>> -		"as default."
>>> +		"as default.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_fssync, 1,
>>>  	
>>>  	  "filesystem sync", "<path>\n"
>>>
>>> -		"Force a sync on the filesystem <path>."
>>> +		"Force a sync on the filesystem <path>.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_resize, 2,
>>>  	
>>>  	  "filesystem resize", "[+/-]<newsize>[gkm]|max <filesystem>\n"
>>>  		
>>>  		"Resize the file system. If 'max' is passed, the filesystem\n"
>>>
>>> -		"will occupe all available space on the device."
>>> +		"will occupe all available space on the device.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_show_filesystem, 999,
>>>  	
>>>  	  "filesystem show", "[<uuid>|<label>]\n"
>>>  		
>>>  		"Show the info of a btrfs filesystem. If no <uuid> or 
> <label>\n"
>>>
>>> -		"is passed, info of all the btrfs filesystem are shown."
>>> +		"is passed, info of all the btrfs filesystem are shown.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_df_filesystem, 1,
>>>  	
>>>  	  "filesystem df", "<path>\n"
>>>
>>> -		"Show space usage information for a mount point\n."
>>> +		"Show space usage information for a mount point.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_balance, 1,
>>>  	
>>>  	  "filesystem balance", "<path>\n"
>>>
>>> -		"Balance the chunks across the device."
>>> +		"Balance the chunks across the device.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_scan,
>>>  	
>>>  	  999, "device scan", "[<device> [<device>..]\n"
>>>  		
>>>  		"Scan all device for or the passed device for a btrfs\n"
>>>
>>> -		"filesystem."
>>> +		"filesystem.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_add_volume, -2,
>>>  	
>>>  	  "device add", "<dev> [<dev>..] <path>\n"
>>>
>>> -		"Add a device to a filesystem."
>>> +		"Add a device to a filesystem.",
>>> +          NULL
>>>
>>>  	},
>>>  	{ do_remove_volume, -2,
>>>  	
>>>  	  "device delete", "<dev> [<dev>..] <path>\n"
>>>
>>> -		"Remove a device from a filesystem."
>>> +		"Remove a device from a filesystem.",
>>> +          NULL
>>>
>>>  	},
>>>  	/* coming soon
>>>  	{ 2, "filesystem label", "<label> <path>\n"
>>>
>>> -		"Set the label of a filesystem"
>>> +		"Set the label of a filesystem",
>>> +          NULL
>>>
>>>  	}
>>>  	*/
>>>
>>> -	{ 0, 0 , 0 }
>>> +	{ 0, 0, 0, 0 }
>>>
>>>  };
>>>  
>>>  static char *get_prgname(char *programname)
>>>
>>> @@ -128,17 +156,25 @@ static char *get_prgname(char *programname)
>>>
>>>  	return np;
>>>  
>>>  }
>>>
>>> -static void print_help(char *programname, struct Command *cmd)
>>> +static void print_help(char *programname, struct Command *cmd, int
>>> helptype)
>>>
>>>  {
>>>  
>>>  	char	*pc;
>>>  	
>>>  	printf("\t%s %s ", programname, cmd->verb );
>>>
>>> -	for(pc = cmd->help; *pc; pc++){
>>> -		putchar(*pc);
>>> -		if(*pc == '\n')
>>> -			printf("\t\t");
>>> -	}
>>> +        if (helptype == ADVANCED_HELP && cmd->adv_help)
>>> +                for(pc = cmd->adv_help; *pc; pc++){
>>> +                        putchar(*pc);
>>> +                        if(*pc == '\n')
>>> +                                printf("\t\t");
>>> +                }
>>> +        else
>>> +	        for(pc = cmd->help; *pc; pc++){
>>> +		        putchar(*pc);
>>> +		        if(*pc == '\n')
>>> +			        printf("\t\t");
>>> +	        }
>>> +
>>>
>>>  	putchar('\n');
>>>  
>>>  }
>>>
>>> @@ -148,10 +184,10 @@ static void help(char *np)
>>>
>>>  	printf("Usage:\n");
>>>  	for( cp = commands; cp->verb; cp++ )
>>>
>>> -		print_help(np, cp);
>>> +		print_help(np, cp, BASIC_HELP);
>>>
>>>  	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
>>>
>>> -	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command
>>> or\n\t\t" +	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a
>>> command or"
>>>
>>>              "subset of commands.\n",np);
>>>  	
>>>  	printf("\n%s\n", BTRFS_BUILD_VERSION);
>>>  
>>>  }
>>>
>>> @@ -257,15 +293,14 @@ static int prepare_args(int *ac, char ***av, char
>>> *prgname, struct Command *cmd
>>>
>>>  /*
>>>
>>> -
>>> -	This function perform the following jobs:
>>>
>>> +	This function performs the following jobs:
>>>  	- show the help if '--help' or 'help' or '-h' are passed
>>>  	- verify that a command is not ambiguous, otherwise show which
>>>  	
>>>  	  part of the command is ambiguous
>>>
>>> -	- if after a (even partial) command there is '--help' show the help
>>> +	- if after a (even partial) command there is '--help' show detailed
>>> help
>>>
>>>  	  for all the matching commands
>>>
>>> -	- if the command doesn't' match show an error
>>> -	- finally, if a command match, they return which command is matched 
> and
>>> +	- if the command doesn't match show an error
>>> +	- finally, if a command matches, they return which command matched and
>>>
>>>  	  the arguments
>>>  	
>>>  	The function return 0 in case of help is requested; <0 in case
>>>
>>> @@ -319,7 +354,7 @@ static int parse_args(int argc, char **argv,
>>>
>>>  		if(argc>i+1 && !strcmp(argv[i+1],"--help")){
>>>  		
>>>  			if(!helprequested)
>>>  			
>>>  				printf("Usage:\n");
>>>
>>> -			print_help(prgname, cp);
>>> +			print_help(prgname, cp, ADVANCED_HELP);
>>>
>>>  			helprequested=1;
>>>  			continue;
>>>  		
>>>  		}
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Hubert Kario]

On Sunday 23 of January 2011 15:54:12 Goffredo Baroncelli wrote:
> There are a lot of patches regarding the btrfs-tool.
> Unfortunately, the btrfs maintainer are very busy in other areas of the
> project. I don't know when (if) these patches will be applied.
> However I think that is better have a misaligned patch instead nothing.

Just as I thought. I'll post it in a minute.

Regards
Hubert Kario

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Jan Schmidt]

Hi Hubert,

I have to admit I did not recognize this patch but now Hugo is forcing
me to use the "detailed help messages" and I've got an improvement to
suggest:

On 23.01.2011 13:42, Hubert Kario wrote:
> extend the
> 
>         btrfs <cmd> --help
> 
> command to print detailed help message if available but fallback to
> basic help message if detailed is unavailable
> 
> add detailed help message for 'filesystem defragment' command
> 
> little tweaks in comments
> 
> Signed-off-by: Hubert Kario <kario@wit.edu.pl>
> ---
>  btrfs.c |  101 ++++++++++++++++++++++++++++++++++++++++++--------------------
>  1 files changed, 68 insertions(+), 33 deletions(-)
> 
> diff --git a/btrfs.c b/btrfs.c
> index b84607a..bd6f6f8 100644
> --- a/btrfs.c
> +++ b/btrfs.c
> @@ -23,6 +23,9 @@
>  #include "btrfs_cmds.h"
>  #include "version.h"
>  
> +#define BASIC_HELP 0
> +#define ADVANCED_HELP 1
> +
>  typedef int (*CommandFunction)(int argc, char **argv);
>  
>  struct Command {
> @@ -31,8 +34,10 @@ struct Command {
>  				   if >= 0, number of arguments,
>  				   if < 0, _minimum_ number of arguments */
>  	char	*verb;		/* verb */
> -	char	*help;		/* help lines; form the 2nd onward they are
> -				   indented */
> +	char	*help;		/* help lines; from the 2nd line onward they 
> +                                   are automatically indented */
> +        char    *adv_help;      /* advanced help message; from the 2nd line 
> +                                   onward they are automatically indented */
>  	/* the following fields are run-time filled by the program */
>  	char	**cmds;		/* array of subcommands */
> @@ -47,73 +52,96 @@ static struct Command commands[] = {
>  	{ do_clone, 2,
>  	  "subvolume snapshot", "<source> [<dest>/]<name>\n"
>  		"Create a writable snapshot of the subvolume <source> with\n"
> -		"the name <name> in the <dest> directory."
> +		"the name <name> in the <dest> directory.",
> +          NULL
>  	},
>  	{ do_delete_subvolume, 1,
>  	  "subvolume delete", "<subvolume>\n"
> -		"Delete the subvolume <subvolume>."
> +		"Delete the subvolume <subvolume>.",
> +          NULL
>  	},
>  	{ do_create_subvol, 1,
>  	  "subvolume create", "[<dest>/]<name>\n"
>  		"Create a subvolume in <dest> (or the current directory if\n"
> -		"not passed)."
> +		"not passed).",
> +          NULL
>  	},
>  	{ do_subvol_list, 1, "subvolume list", "<path>\n"
> -		"List the snapshot/subvolume of a filesystem."
> +		"List the snapshot/subvolume of a filesystem.",
> +          NULL
>  	},
>  	{ do_find_newer, 2, "subvolume find-new", "<path> <last_gen>\n"
> -		"List the recently modified files in a filesystem."
> +		"List the recently modified files in a filesystem.",
> +          NULL
>  	},
>  	{ do_defrag, -1,
>  	  "filesystem defragment", "[-vcf] [-s start] [-l len] [-t size] <file>|<dir> [<file>|<dir>...]\n"
> -		"Defragment a file or a directory."
> +		"Defragment a file or a directory.",
> +          "[-vcf] [-s start] [-l len] [-t size] <file>|<dir> [<file>|<dir>...]\n"
> +          "Defragment file data or directory metadata.\n"
> +                "-v         be verbose\n"
> +                "-c         compress the file while defragmenting\n"
> +                "-f         flush data to disk immediately after defragmenting\n"
> +                "-s start   defragment only from byte onward\n"
> +                "-l len     defragment only up to len bytes\n"
> +                "-t size    minimal size of file to be considered for defragmenting\n"

Lots of too long lines.

I don't like to repeat the synopsis passage. How about adding the
general ->help when printing ->adv_help as well? This reduces the need
of duplication.

To prove my point, looking at the current version in Hugo's integration
branch, your two synopsis lines already got inconsistent regarding the
-c option :-)

>  	},
>  	{ do_set_default_subvol, 2,
>  	  "subvolume set-default", "<id> <path>\n"
>  		"Set the subvolume of the filesystem <path> which will be mounted\n"
> -		"as default."
> +		"as default.",
> +          NULL
>  	},
>  	{ do_fssync, 1,
>  	  "filesystem sync", "<path>\n"
> -		"Force a sync on the filesystem <path>."
> +		"Force a sync on the filesystem <path>.",
> +          NULL
>  	},
>  	{ do_resize, 2,
>  	  "filesystem resize", "[+/-]<newsize>[gkm]|max <filesystem>\n"
>  		"Resize the file system. If 'max' is passed, the filesystem\n"
> -		"will occupe all available space on the device."
> +		"will occupe all available space on the device.",
> +          NULL
>  	},
>  	{ do_show_filesystem, 999,
>  	  "filesystem show", "[<uuid>|<label>]\n"
>  		"Show the info of a btrfs filesystem. If no <uuid> or <label>\n"
> -		"is passed, info of all the btrfs filesystem are shown."
> +		"is passed, info of all the btrfs filesystem are shown.",
> +          NULL
>  	},
>  	{ do_df_filesystem, 1,
>  	  "filesystem df", "<path>\n"
> -		"Show space usage information for a mount point\n."
> +		"Show space usage information for a mount point.",
> +          NULL
>  	},
>  	{ do_balance, 1,
>  	  "filesystem balance", "<path>\n"
> -		"Balance the chunks across the device."
> +		"Balance the chunks across the device.",
> +          NULL
>  	},
>  	{ do_scan,
>  	  999, "device scan", "[<device> [<device>..]\n"
>  		"Scan all device for or the passed device for a btrfs\n"
> -		"filesystem."
> +		"filesystem.",
> +          NULL
>  	},
>  	{ do_add_volume, -2,
>  	  "device add", "<dev> [<dev>..] <path>\n"
> -		"Add a device to a filesystem."
> +		"Add a device to a filesystem.",
> +          NULL
>  	},
>  	{ do_remove_volume, -2,
>  	  "device delete", "<dev> [<dev>..] <path>\n"
> -		"Remove a device from a filesystem."
> +		"Remove a device from a filesystem.",
> +          NULL
>  	},
>  	/* coming soon
>  	{ 2, "filesystem label", "<label> <path>\n"
> -		"Set the label of a filesystem"
> +		"Set the label of a filesystem",
> +          NULL
>  	}
>  	*/
> -	{ 0, 0 , 0 }
> +	{ 0, 0, 0, 0 }
>  };
>  
>  static char *get_prgname(char *programname)
> @@ -128,17 +156,25 @@ static char *get_prgname(char *programname)
>  	return np;
>  }
>  
> -static void print_help(char *programname, struct Command *cmd)
> +static void print_help(char *programname, struct Command *cmd, int helptype)
>  {
>  	char	*pc;
>  
>  	printf("\t%s %s ", programname, cmd->verb );
>  
> -	for(pc = cmd->help; *pc; pc++){
> -		putchar(*pc);
> -		if(*pc == '\n')
> -			printf("\t\t");
> -	}
> +        if (helptype == ADVANCED_HELP && cmd->adv_help)

As mentioned above, I'd like to have ->help printed here, above. You can
make the loop in the else branch below unconditional and just put the
advanced part inside the if.

> +                for(pc = cmd->adv_help; *pc; pc++){
> +                        putchar(*pc);
> +                        if(*pc == '\n')
> +                                printf("\t\t");
> +                }
> +        else
> +	        for(pc = cmd->help; *pc; pc++){
> +		        putchar(*pc);
> +		        if(*pc == '\n')
> +			        printf("\t\t");
> +	        }
> +
>  	putchar('\n');
>  }
>  
> @@ -148,10 +184,10 @@ static void help(char *np)
>  
>  	printf("Usage:\n");
>  	for( cp = commands; cp->verb; cp++ )
> -		print_help(np, cp);
> +		print_help(np, cp, BASIC_HELP);
>  
>  	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
                            ^^^^^^^^^
You did not change this, but as we are here, ...

> -	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command or\n\t\t"
> +	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command or"
                             ^^^^^^^
... why not extending the general rule so that help messages will be
printed with --help and -h?

>              "subset of commands.\n",np);
>  	printf("\n%s\n", BTRFS_BUILD_VERSION);
>  }
> @@ -257,15 +293,14 @@ static int prepare_args(int *ac, char ***av, char *prgname, struct Command *cmd
>  
>  
>  /*
> -
> -	This function perform the following jobs:
> +	This function performs the following jobs:
>  	- show the help if '--help' or 'help' or '-h' are passed
>  	- verify that a command is not ambiguous, otherwise show which
>  	  part of the command is ambiguous
> -	- if after a (even partial) command there is '--help' show the help
> +	- if after a (even partial) command there is '--help' show detailed help
>  	  for all the matching commands
> -	- if the command doesn't' match show an error
> -	- finally, if a command match, they return which command is matched and
> +	- if the command doesn't match show an error
> +	- finally, if a command matches, they return which command matched and
>  	  the arguments
>  
>  	The function return 0 in case of help is requested; <0 in case
> @@ -319,7 +354,7 @@ static int parse_args(int argc, char **argv,
>  		if(argc>i+1 && !strcmp(argv[i+1],"--help")){
>  			if(!helprequested)
>  				printf("Usage:\n");
> -			print_help(prgname, cp);
> +			print_help(prgname, cp, ADVANCED_HELP);
>  			helprequested=1;
>  			continue;
>  		}

-Jan

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Goffredo Baroncelli]

Hi, all.

what about generating the man page on the basis of the btrfs help
detailed messages ?

My idea is the following:
before the function source associated to the command we can put a
comment with a detailed help. The comment may be:

[...]
/*** man:btrfs subvolume create
 *
 *  btrfs subvolume create <path>
 *     create a new subvolume
 *
 *  The command [b]btrfs subvolume create[b] is used.....
 *
 ***/

void do_create_subvolume(int argc, char **argv)
{
[...]

A script extracts from the comment in the source both:
- the text for the man page
- the text for the detailed help.

So we can reach the following goals:
- the help is linked to the code
- is less likely to forget to update the message
- the man page, the helps are always aligned

BR
G.Baroncelli



On 07/11/2011 05:13 PM, Jan Schmidt wrote:
> Hi Hubert,
> 
> I have to admit I did not recognize this patch but now Hugo is forcing
> me to use the "detailed help messages" and I've got an improvement to
> suggest:
> 
> On 23.01.2011 13:42, Hubert Kario wrote:
>> extend the
>>
>>         btrfs <cmd> --help
>>
>> command to print detailed help message if available but fallback to
>> basic help message if detailed is unavailable
>>
>> add detailed help message for 'filesystem defragment' command
>>
>> little tweaks in comments
>>
>> Signed-off-by: Hubert Kario <kario@wit.edu.pl>
>> ---
>>  btrfs.c |  101 ++++++++++++++++++++++++++++++++++++++++++--------------------
>>  1 files changed, 68 insertions(+), 33 deletions(-)
>>
>> diff --git a/btrfs.c b/btrfs.c
>> index b84607a..bd6f6f8 100644
>> --- a/btrfs.c
>> +++ b/btrfs.c
>> @@ -23,6 +23,9 @@
>>  #include "btrfs_cmds.h"
>>  #include "version.h"
>>  
>> +#define BASIC_HELP 0
>> +#define ADVANCED_HELP 1
>> +
>>  typedef int (*CommandFunction)(int argc, char **argv);
>>  
>>  struct Command {
>> @@ -31,8 +34,10 @@ struct Command {
>>  				   if >= 0, number of arguments,
>>  				   if < 0, _minimum_ number of arguments */
>>  	char	*verb;		/* verb */
>> -	char	*help;		/* help lines; form the 2nd onward they are
>> -				   indented */
>> +	char	*help;		/* help lines; from the 2nd line onward they 
>> +                                   are automatically indented */
>> +        char    *adv_help;      /* advanced help message; from the 2nd line 
>> +                                   onward they are automatically indented */
>>  	/* the following fields are run-time filled by the program */
>>  	char	**cmds;		/* array of subcommands */
>> @@ -47,73 +52,96 @@ static struct Command commands[] = {
>>  	{ do_clone, 2,
>>  	  "subvolume snapshot", "<source> [<dest>/]<name>\n"
>>  		"Create a writable snapshot of the subvolume <source> with\n"
>> -		"the name <name> in the <dest> directory."
>> +		"the name <name> in the <dest> directory.",
>> +          NULL
>>  	},
>>  	{ do_delete_subvolume, 1,
>>  	  "subvolume delete", "<subvolume>\n"
>> -		"Delete the subvolume <subvolume>."
>> +		"Delete the subvolume <subvolume>.",
>> +          NULL
>>  	},
>>  	{ do_create_subvol, 1,
>>  	  "subvolume create", "[<dest>/]<name>\n"
>>  		"Create a subvolume in <dest> (or the current directory if\n"
>> -		"not passed)."
>> +		"not passed).",
>> +          NULL
>>  	},
>>  	{ do_subvol_list, 1, "subvolume list", "<path>\n"
>> -		"List the snapshot/subvolume of a filesystem."
>> +		"List the snapshot/subvolume of a filesystem.",
>> +          NULL
>>  	},
>>  	{ do_find_newer, 2, "subvolume find-new", "<path> <last_gen>\n"
>> -		"List the recently modified files in a filesystem."
>> +		"List the recently modified files in a filesystem.",
>> +          NULL
>>  	},
>>  	{ do_defrag, -1,
>>  	  "filesystem defragment", "[-vcf] [-s start] [-l len] [-t size] <file>|<dir> [<file>|<dir>...]\n"
>> -		"Defragment a file or a directory."
>> +		"Defragment a file or a directory.",
>> +          "[-vcf] [-s start] [-l len] [-t size] <file>|<dir> [<file>|<dir>...]\n"
>> +          "Defragment file data or directory metadata.\n"
>> +                "-v         be verbose\n"
>> +                "-c         compress the file while defragmenting\n"
>> +                "-f         flush data to disk immediately after defragmenting\n"
>> +                "-s start   defragment only from byte onward\n"
>> +                "-l len     defragment only up to len bytes\n"
>> +                "-t size    minimal size of file to be considered for defragmenting\n"
> 
> Lots of too long lines.
> 
> I don't like to repeat the synopsis passage. How about adding the
> general ->help when printing ->adv_help as well? This reduces the need
> of duplication.
> 
> To prove my point, looking at the current version in Hugo's integration
> branch, your two synopsis lines already got inconsistent regarding the
> -c option :-)
> 
>>  	},
>>  	{ do_set_default_subvol, 2,
>>  	  "subvolume set-default", "<id> <path>\n"
>>  		"Set the subvolume of the filesystem <path> which will be mounted\n"
>> -		"as default."
>> +		"as default.",
>> +          NULL
>>  	},
>>  	{ do_fssync, 1,
>>  	  "filesystem sync", "<path>\n"
>> -		"Force a sync on the filesystem <path>."
>> +		"Force a sync on the filesystem <path>.",
>> +          NULL
>>  	},
>>  	{ do_resize, 2,
>>  	  "filesystem resize", "[+/-]<newsize>[gkm]|max <filesystem>\n"
>>  		"Resize the file system. If 'max' is passed, the filesystem\n"
>> -		"will occupe all available space on the device."
>> +		"will occupe all available space on the device.",
>> +          NULL
>>  	},
>>  	{ do_show_filesystem, 999,
>>  	  "filesystem show", "[<uuid>|<label>]\n"
>>  		"Show the info of a btrfs filesystem. If no <uuid> or <label>\n"
>> -		"is passed, info of all the btrfs filesystem are shown."
>> +		"is passed, info of all the btrfs filesystem are shown.",
>> +          NULL
>>  	},
>>  	{ do_df_filesystem, 1,
>>  	  "filesystem df", "<path>\n"
>> -		"Show space usage information for a mount point\n."
>> +		"Show space usage information for a mount point.",
>> +          NULL
>>  	},
>>  	{ do_balance, 1,
>>  	  "filesystem balance", "<path>\n"
>> -		"Balance the chunks across the device."
>> +		"Balance the chunks across the device.",
>> +          NULL
>>  	},
>>  	{ do_scan,
>>  	  999, "device scan", "[<device> [<device>..]\n"
>>  		"Scan all device for or the passed device for a btrfs\n"
>> -		"filesystem."
>> +		"filesystem.",
>> +          NULL
>>  	},
>>  	{ do_add_volume, -2,
>>  	  "device add", "<dev> [<dev>..] <path>\n"
>> -		"Add a device to a filesystem."
>> +		"Add a device to a filesystem.",
>> +          NULL
>>  	},
>>  	{ do_remove_volume, -2,
>>  	  "device delete", "<dev> [<dev>..] <path>\n"
>> -		"Remove a device from a filesystem."
>> +		"Remove a device from a filesystem.",
>> +          NULL
>>  	},
>>  	/* coming soon
>>  	{ 2, "filesystem label", "<label> <path>\n"
>> -		"Set the label of a filesystem"
>> +		"Set the label of a filesystem",
>> +          NULL
>>  	}
>>  	*/
>> -	{ 0, 0 , 0 }
>> +	{ 0, 0, 0, 0 }
>>  };
>>  
>>  static char *get_prgname(char *programname)
>> @@ -128,17 +156,25 @@ static char *get_prgname(char *programname)
>>  	return np;
>>  }
>>  
>> -static void print_help(char *programname, struct Command *cmd)
>> +static void print_help(char *programname, struct Command *cmd, int helptype)
>>  {
>>  	char	*pc;
>>  
>>  	printf("\t%s %s ", programname, cmd->verb );
>>  
>> -	for(pc = cmd->help; *pc; pc++){
>> -		putchar(*pc);
>> -		if(*pc == '\n')
>> -			printf("\t\t");
>> -	}
>> +        if (helptype == ADVANCED_HELP && cmd->adv_help)
> 
> As mentioned above, I'd like to have ->help printed here, above. You can
> make the loop in the else branch below unconditional and just put the
> advanced part inside the if.
> 
>> +                for(pc = cmd->adv_help; *pc; pc++){
>> +                        putchar(*pc);
>> +                        if(*pc == '\n')
>> +                                printf("\t\t");
>> +                }
>> +        else
>> +	        for(pc = cmd->help; *pc; pc++){
>> +		        putchar(*pc);
>> +		        if(*pc == '\n')
>> +			        printf("\t\t");
>> +	        }
>> +
>>  	putchar('\n');
>>  }
>>  
>> @@ -148,10 +184,10 @@ static void help(char *np)
>>  
>>  	printf("Usage:\n");
>>  	for( cp = commands; cp->verb; cp++ )
>> -		print_help(np, cp);
>> +		print_help(np, cp, BASIC_HELP);
>>  
>>  	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
>                             ^^^^^^^^^
> You did not change this, but as we are here, ...
> 
>> -	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command or\n\t\t"
>> +	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command or"
>                              ^^^^^^^
> .. why not extending the general rule so that help messages will be
> printed with --help and -h?
> 
>>              "subset of commands.\n",np);
>>  	printf("\n%s\n", BTRFS_BUILD_VERSION);
>>  }
>> @@ -257,15 +293,14 @@ static int prepare_args(int *ac, char ***av, char *prgname, struct Command *cmd
>>  
>>  
>>  /*
>> -
>> -	This function perform the following jobs:
>> +	This function performs the following jobs:
>>  	- show the help if '--help' or 'help' or '-h' are passed
>>  	- verify that a command is not ambiguous, otherwise show which
>>  	  part of the command is ambiguous
>> -	- if after a (even partial) command there is '--help' show the help
>> +	- if after a (even partial) command there is '--help' show detailed help
>>  	  for all the matching commands
>> -	- if the command doesn't' match show an error
>> -	- finally, if a command match, they return which command is matched and
>> +	- if the command doesn't match show an error
>> +	- finally, if a command matches, they return which command matched and
>>  	  the arguments
>>  
>>  	The function return 0 in case of help is requested; <0 in case
>> @@ -319,7 +354,7 @@ static int parse_args(int argc, char **argv,
>>  		if(argc>i+1 && !strcmp(argv[i+1],"--help")){
>>  			if(!helprequested)
>>  				printf("Usage:\n");
>> -			print_help(prgname, cp);
>> +			print_help(prgname, cp, ADVANCED_HELP);
>>  			helprequested=1;
>>  			continue;
>>  		}
> 
> -Jan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Jan Schmidt]

On 07/11/2011 08:38 PM, Goffredo Baroncelli wrote:
> what about generating the man page on the basis of the btrfs help
> detailed messages ?
> 
> My idea is the following:
> before the function source associated to the command we can put a
> comment with a detailed help. The comment may be:
> 
> [...]
> /*** man:btrfs subvolume create
>  *
>  *  btrfs subvolume create <path>
>  *     create a new subvolume
>  *
>  *  The command [b]btrfs subvolume create[b] is used.....
>  *
>  ***/
> 
> void do_create_subvolume(int argc, char **argv)
> {
> [...]

Reminds me of java, but nevertheless, I like the general idea.

> A script extracts from the comment in the source both:
> - the text for the man page
> - the text for the detailed help.

Does anybody have such a script around? I suppose we're not the first
ones writing help texts and man pages.

> So we can reach the following goals:
> - the help is linked to the code
> - is less likely to forget to update the message
> - the man page, the helps are always aligned

Only, we still will need like short and long help. E.g. the full text in
the man page may be inappropriate as a --help message. Also, we do need
a clever idea to get indentation right in the man pages. I fiddled a lot
on the man pages for scrub parameter indentation (to get the second line
describing a command line option indented correctly to start below the
text of the first line, that was).

-Jan

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Goffredo Baroncelli]

On 07/11/2011 09:11 PM, Jan Schmidt wrote:
> On 07/11/2011 08:38 PM, Goffredo Baroncelli wrote:
>> what about generating the man page on the basis of the btrfs help
>> detailed messages ?
>>
>> My idea is the following:
>> before the function source associated to the command we can put a
>> comment with a detailed help. The comment may be:
>>
>> [...]
>> /*** man:btrfs subvolume create
>>  *
>>  *  btrfs subvolume create <path>
>>  *     create a new subvolume
>>  *
>>  *  The command [b]btrfs subvolume create[b] is used.....
>>  *
>>  ***/
>>
>> void do_create_subvolume(int argc, char **argv)
>> {
>> [...]
> 
> Reminds me of java, but nevertheless, I like the general idea.
> 
>> A script extracts from the comment in the source both:
>> - the text for the man page
>> - the text for the detailed help.
> 
> Does anybody have such a script around? I suppose we're not the first
> ones writing help texts and man pages.
> 

I am trying to write a my own (yes I know, I suffer of the NIH syndrome
:-) ).

>> So we can reach the following goals:
>> - the help is linked to the code
>> - is less likely to forget to update the message
>> - the man page, the helps are always aligned
> 
> Only, we still will need like short and long help. E.g. the full text in
> the man page may be inappropriate as a --help message. Also, we do need
> a clever idea to get indentation right in the man pages. I fiddled a lot
> on the man pages for scrub parameter indentation (to get the second line
> describing a command line option indented correctly to start below the
> text of the first line, that was).

I agree for the short and long help. I think that we need to use some
tags for the bold, italic, Empty line...

> 
> -Jan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> .
> 

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 3959 bytes --]

On Mon, Jul 11, 2011 at 09:11:24PM +0200, Jan Schmidt wrote:
> On 07/11/2011 08:38 PM, Goffredo Baroncelli wrote:
> > what about generating the man page on the basis of the btrfs help
> > detailed messages ?
> > 
> > My idea is the following:
> > before the function source associated to the command we can put a
> > comment with a detailed help. The comment may be:
> > 
> > [...]
> > /*** man:btrfs subvolume create
> >  *
> >  *  btrfs subvolume create <path>
> >  *     create a new subvolume
> >  *
> >  *  The command [b]btrfs subvolume create[b] is used.....
> >  *
> >  ***/
> > 
> > void do_create_subvolume(int argc, char **argv)
> > {
> > [...]
> 
> Reminds me of java, but nevertheless, I like the general idea.

   In principle, it's attractive. I still have some reservations as to
the amount of effort involved in automating the process (vs manually
updating things), and in the levels of detail needed (see the
discussion below).

> > A script extracts from the comment in the source both:
> > - the text for the man page
> > - the text for the detailed help.

   Or possibly going the other direction: from the man page (which
contains all of the information we need to reproduce in the code), it
should be possible, with appropriate structuring, to retrieve the bits
that the code needs to know about, and insert them into a table in a
generated .c file. Just a thought. 

   Oh, and the current man page needs some major work on its
typography -- it's inconsistent with both itself, and with most other
man pages, as far as I can tell. I did have a patch for that, but it
was a long time ago, and clashed with almost everything.

> Does anybody have such a script around? I suppose we're not the first
> ones writing help texts and man pages.
> 
> > So we can reach the following goals:
> > - the help is linked to the code
> > - is less likely to forget to update the message
> > - the man page, the helps are always aligned
> 
> Only, we still will need like short and long help. E.g. the full text in
> the man page may be inappropriate as a --help message. Also, we do need
> a clever idea to get indentation right in the man pages. I fiddled a lot
> on the man pages for scrub parameter indentation (to get the second line
> describing a command line option indented correctly to start below the
> text of the first line, that was).

   We actually need three levels of help:

 * A one-liner "headline" version that comes in the synopsis section
   of the man page and the output of "btrfs --help"

    = btrfs filesystem defragment [-vf] [-c{zlib,lzo}] [-s <start>] <file>|<dir> [...]
	=   Defragment a file or directory

 * A collection of one-liners summarising all the switches, that comes
   as the output of "btrfs foo bar --help": a repeat of the "headline"
   version, plus a single (half-)line for each switch.

    = btrfs filesystem defragment [-vf] [-c{zlib,lzo}] [-s <start>] <file>|<dir> [...]
	=   Defragment a file or directory
	=   -v				verbose output
	=	-f				do the f thing
	=	-c				force compression algorithm (zlib or lzo)
	=   -s <start>		start the defrag from offset <start> in the file

 * A detailed description of the command as a whole and each option,
   which appears in the detail section of the man page.

    = btrfs filesystem defragment [-vf] [-c{zlib,lzo}] [-s <start>] <file>|<dir> [...]

	=   Defragment the given file(s) or directories. Defrag does not
	=   operate recursively, so if you want to defragment an entire
	=   subdirectory and all its children, you should use find(1) to list
	=   the files, and pass the list to btrfs fi defrag. [etc]
    =   -s <start>		Start the defragmentation operation at offset
	=                   <start> within the file.

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
        --- Great oxymorons of the world, no. 2: Common Sense ---        

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Hubert Kario]

On Tuesday 12 of July 2011 00:22:01 Hugo Mills wrote:
> On Mon, Jul 11, 2011 at 09:11:24PM +0200, Jan Schmidt wrote:
> > On 07/11/2011 08:38 PM, Goffredo Baroncelli wrote:
[snip]
> > > A script extracts from the comment in the source both:
> > > - the text for the man page
> > > - the text for the detailed help.
> 
>    Or possibly going the other direction: from the man page (which
> contains all of the information we need to reproduce in the code), it
> should be possible, with appropriate structuring, to retrieve the bits
> that the code needs to know about, and insert them into a table in a
> generated .c file. Just a thought.

I think that the first line in normal help message and advanced help message 
can and sometimes should be different.

The basic as concise as possible, while the advanced verbose and quite 
detailed (for example explaining what a filesystem scrub /is/)
 
>    Oh, and the current man page needs some major work on its
> typography -- it's inconsistent with both itself, and with most other
> man pages, as far as I can tell. I did have a patch for that, but it
> was a long time ago, and clashed with almost everything.

Yes, until we won't have a single current tree for btrfs-progs there will be 
inconsistencies that will need to be fixed later.

But I guess that with Hugo's tree we're getting there.
 
> > Does anybody have such a script around? I suppose we're not the first
> > ones writing help texts and man pages.
> > 
> > > So we can reach the following goals:
> > > - the help is linked to the code
> > > - is less likely to forget to update the message
> > > - the man page, the helps are always aligned
> > 
> > Only, we still will need like short and long help. E.g. the full text in
> > the man page may be inappropriate as a --help message. Also, we do need
> > a clever idea to get indentation right in the man pages. I fiddled a lot
> > on the man pages for scrub parameter indentation (to get the second line
> > describing a command line option indented correctly to start below the
> > text of the first line, that was).
> 
>    We actually need three levels of help:
> [snip]

I'd that the biggest problem, putting it all in code and formatting will be a 
real pain...

Hubert

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Hubert Kario]

On Monday 11 of July 2011 17:13:13 Jan Schmidt wrote:
> Hi Hubert,
> 
> I have to admit I did not recognize this patch but now Hugo is forcing
> me to use the "detailed help messages" and I've got an improvement to
> suggest:
> 
> On 23.01.2011 13:42, Hubert Kario wrote:
[snip]
> >  	{ do_defrag, -1,
> >  	
> >  	  "filesystem defragment", "[-vcf] [-s start] [-l len] [-t size]
> >  	  <file>|<dir> [<file>|<dir>...]\n"
> > 
> > -		"Defragment a file or a directory."
> > +		"Defragment a file or a directory.",
> > +          "[-vcf] [-s start] [-l len] [-t size] <file>|<dir>
> > [<file>|<dir>...]\n" +          "Defragment file data or directory
> > metadata.\n"
> > +                "-v         be verbose\n"
> > +                "-c         compress the file while defragmenting\n"
> > +                "-f         flush data to disk immediately after
> > defragmenting\n" +                "-s start   defragment only from byte
> > onward\n" +                "-l len     defragment only up to len
> > bytes\n"
> > +                "-t size    minimal size of file to be considered for
> > defragmenting\n"
> 
> Lots of too long lines.

you mean the code or the printed messages? 
messages fit a 80 column screen, I remember I double checked it

> 
> I don't like to repeat the synopsis passage. How about adding the
> general ->help when printing ->adv_help as well? This reduces the need
> of duplication.

I think I added it because of differences in formatting.
Also I'd say we don't want to overload the user with information when he 
mistypes a command so the main help command should be as concise as possible 
while the advanced may be much more detailed (looking at the mailing list, `fi 
df` could definitely use some more verbose help message)

> 
> To prove my point, looking at the current version in Hugo's integration
> branch, your two synopsis lines already got inconsistent regarding the
> -c option :-)

That's because the patches are submitted with base as Chris tree, not the 
Hugo's so the result is a real patchwork that needs some clean-up

[snip]

> > @@ -148,10 +184,10 @@ static void help(char *np)
> > 
> >  	printf("Usage:\n");
> >  	for( cp = commands; cp->verb; cp++ )
> > 
> > -		print_help(np, cp);
> > +		print_help(np, cp, BASIC_HELP);
> > 
> >  	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
> 
>                             ^^^^^^^^^
> You did not change this, but as we are here, ...

I wanted to leave as much code unchanged as possible (this /was/ my first 
patch to btrfs-tools)

> 
> > -	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command
> > or\n\t\t" +	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a
> > command or"
> 
>                              ^^^^^^^
> ... why not extending the general rule so that help messages will be
> printed with --help and -h?

We have to remember that this way we loose -h switch, quite intuitive to show 
base 2 sizes with `btrfs file df`... 

-- 
Hubert

Re: [PATCH 2/2] add detailed help messages to btrfs command

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 3804 bytes --]

On Tue, Jul 12, 2011 at 12:40:06PM +0200, Hubert Kario wrote:
> On Monday 11 of July 2011 17:13:13 Jan Schmidt wrote:
> > Hi Hubert,
> > 
> > I have to admit I did not recognize this patch but now Hugo is forcing
> > me to use the "detailed help messages" and I've got an improvement to
> > suggest:
> > 
> > On 23.01.2011 13:42, Hubert Kario wrote:
> [snip]
> > >  	{ do_defrag, -1,
> > >  	
> > >  	  "filesystem defragment", "[-vcf] [-s start] [-l len] [-t size]
> > >  	  <file>|<dir> [<file>|<dir>...]\n"
> > > 
> > > -		"Defragment a file or a directory."
> > > +		"Defragment a file or a directory.",
> > > +          "[-vcf] [-s start] [-l len] [-t size] <file>|<dir>
> > > [<file>|<dir>...]\n" +          "Defragment file data or directory
> > > metadata.\n"
> > > +                "-v         be verbose\n"
> > > +                "-c         compress the file while defragmenting\n"
> > > +                "-f         flush data to disk immediately after
> > > defragmenting\n" +                "-s start   defragment only from byte
> > > onward\n" +                "-l len     defragment only up to len
> > > bytes\n"
> > > +                "-t size    minimal size of file to be considered for
> > > defragmenting\n"
> > 
> > Lots of too long lines.
> 
> you mean the code or the printed messages? 
> messages fit a 80 column screen, I remember I double checked it
> 
> > 
> > I don't like to repeat the synopsis passage. How about adding the
> > general ->help when printing ->adv_help as well? This reduces the need
> > of duplication.
> 
> I think I added it because of differences in formatting.
> Also I'd say we don't want to overload the user with information when he 
> mistypes a command so the main help command should be as concise as possible 
> while the advanced may be much more detailed (looking at the mailing list, `fi 
> df` could definitely use some more verbose help message)
> 
> > 
> > To prove my point, looking at the current version in Hugo's integration
> > branch, your two synopsis lines already got inconsistent regarding the
> > -c option :-)
> 
> That's because the patches are submitted with base as Chris tree, not the 
> Hugo's so the result is a real patchwork that needs some clean-up
> 
> [snip]
> 
> > > @@ -148,10 +184,10 @@ static void help(char *np)
> > > 
> > >  	printf("Usage:\n");
> > >  	for( cp = commands; cp->verb; cp++ )
> > > 
> > > -		print_help(np, cp);
> > > +		print_help(np, cp, BASIC_HELP);
> > > 
> > >  	printf("\n\t%s help|--help|-h\n\t\tShow the help.\n",np);
> > 
> >                             ^^^^^^^^^
> > You did not change this, but as we are here, ...
> 
> I wanted to leave as much code unchanged as possible (this /was/ my first 
> patch to btrfs-tools)
> 
> > 
> > > -	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a command
> > > or\n\t\t" +	printf("\n\t%s <cmd> --help\n\t\tShow detailed help for a
> > > command or"
> > 
> >                              ^^^^^^^
> > ... why not extending the general rule so that help messages will be
> > printed with --help and -h?
> 
> We have to remember that this way we loose -h switch, quite intuitive to show 
> base 2 sizes with `btrfs file df`... 

   Indeed. To quote from "df --help":

  -h, --human-readable  print sizes in human readable format (e.g., 1K 234M 2G)
  -H, --si              likewise, but use powers of 1000 not 1024
[...]
      --help     display this help and exit

   I should probably resurrect my patch to implement this for btrfs.

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
   --- To an Englishman, 100 miles is a long way;  to an American, ---   
                        100 years is a long time.                        

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 190 bytes --]