Libguestfs - Dec 2014 - [PATCH v2] fish: show synopsis if command syntax is wrong

This patch lets guestfish show command synopsis if the syntax of command issued
by user is wrong, rather than telling user that the number of parameters is
wrong.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---

changes in v2:
 - fix the error of ' error: control reaches end of non-void function
[-Werror=return-type]'
 - change the return value from -2 to -1

 fish/cmds-gperf.h |  1 +
 generator/fish.ml | 38 +++++++++++++++-----------------------
 2 files changed, 16 insertions(+), 23 deletions(-)

diff --git a/fish/cmds-gperf.h b/fish/cmds-gperf.h
index 74db69d..bcb3b5d 100644
--- a/fish/cmds-gperf.h
+++ b/fish/cmds-gperf.h
@@ -25,6 +25,7 @@
 struct command_entry {
   const char *name;             /* Short name. */
   const char *help;             /* Online help. */
+  const char *synopsis;         /* Synopsis. */
 
   /* The run_* function. */
   int (*run) (const char *cmd, size_t argc, char *argv[]);
diff --git a/generator/fish.ml b/generator/fish.ml
index 3f53ffa..52cdfd5 100644
--- a/generator/fish.ml
+++ b/generator/fish.ml
@@ -181,7 +181,7 @@ Guestfish will prompt for these separately."
         else "" in
 
       let pod -        sprintf "%s - %s\n\n=head1 SYNOPSIS\n\n
%s\n\n=head1 DESCRIPTION\n\n%s%s%s"
+        sprintf "%s - %s\n\n=head1 SYNOPSIS\n\n%s\n\n=head1
DESCRIPTION\n\n%s%s%s"
           name2 shortdesc synopsis longdesc warnings describe_alias in
       let text          String.concat "\n" (pod2text ~trim:false
~discard:false "NAME" pod)
@@ -190,6 +190,7 @@ Guestfish will prompt for these separately."
       pr "struct command_entry %s_cmd_entry = {\n" name;
       pr "  .name = \"%s\",\n" name2;
       pr "  .help = \"%s\",\n" (c_quote text);
+      pr "  .synopsis = \"%s\",\n" (c_quote synopsis);
       pr "  .run = run_%s\n" name;
       pr "};\n";
       pr "\n";
@@ -393,30 +394,14 @@ Guestfish will prompt for these separately."
 
       if argc_minimum = argc_maximum then (
         pr "  if (argc != %d) {\n" argc_minimum;
-        if argc_minimum = 0 then (
-          pr "    fprintf (stderr, _(\"%%s should have no
parameters\\n\"), cmd);\n";
-        ) else (
-          pr "    fprintf (stderr, ngettext(\"%%s should have %%d
parameter\\n\",\n";
-          pr "                              \"%%s should have %%d
parameters\\n\",\n";
-          pr "                              %d),\n"
-            argc_minimum;
-          pr "                     cmd, %d);\n"
-            argc_minimum;
-        )
+          pr "    ret = -2;\n";
       ) else if argc_minimum = 0 then (
         pr "  if (argc > %d) {\n" argc_maximum;
-        pr "    fprintf (stderr, ngettext(\"%%s should have at most
%%d parameter\\n\",\n";
-        pr "                              \"%%s should have at most
%%d parameters\\n\",\n";
-        pr "                              %d),\n"
-          argc_maximum;
-        pr "                     cmd, %d);\n"
-          argc_maximum;
+        pr "    ret = -2;\n";
       ) else (
         pr "  if (argc < %d || argc > %d) {\n" argc_minimum
argc_maximum;
-        pr "    fprintf (stderr, _(\"%%s should have %%d-%%d
parameter(s)\\n\"), cmd, %d, %d);\n"
-          argc_minimum argc_maximum;
+        pr "    ret = -2;\n";
       );
-      pr "    fprintf (stderr, _(\"type 'help %%s' for help
on %%s\\n\"), cmd, cmd);\n";
       pr "    goto out_noargs;\n";
       pr "  }\n";
 
@@ -694,16 +679,23 @@ Guestfish will prompt for these separately."
   pr "run_action (const char *cmd, size_t argc, char *argv[])\n";
   pr "{\n";
   pr "  const struct command_table *ct;\n";
+  pr "  int ret = -1;\n";
   pr "\n";
   pr "  ct = lookup_fish_command (cmd, strlen (cmd));\n";
-  pr "  if (ct)\n";
-  pr "    return ct->entry->run (cmd, argc, argv);\n";
+  pr "  if (ct) {\n";
+  pr "    ret = ct->entry->run (cmd, argc, argv);\n";
+  pr "    if (ret == -2) {\n";
+  pr "      fprintf (stderr, _(\"usage: %%s\\n\"),
ct->entry->synopsis);\n";
+  pr "      fprintf (stderr, _(\"type 'help %%s' for more
help on %%s\\n\"), cmd, cmd);\n";
+  pr "      ret = -1;\n";
+  pr "    }\n";
+  pr "  }\n";
   pr "  else {\n";
   pr "    fprintf (stderr, _(\"%%s: unknown command\\n\"),
cmd);\n";
   pr "    if (command_num == 1)\n";
   pr "      extended_help_message ();\n";
-  pr "    return -1;\n";
   pr "  }\n";
+  pr "  return ret;\n";
   pr "}\n"
 
 and generate_fish_cmds_h () -- 
1.9.3

How about the following?  I've made it use a named value
(RUN_WRONG_ARGS == -2) instead of just the magic number.

Also I removed an incorrect hunk from the original patch (space before
synopsis string is required).

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Mon, Dec 01, 2014 at 06:01:25PM +0000, Richard W.M. Jones
wrote:
> 
> How about the following?  I've made it use a named value
> (RUN_WRONG_ARGS == -2) instead of just the magic number.
Yes, it looks better!
> 
> Also I removed an incorrect hunk from the original patch (space before
> synopsis string is required).
The space before synopsis makes the text of synopsis misaligned. I don't
know why it's required, but if it does, the patch looks good to me.

Regards,
Hu
> 
> Rich.
> 
> -- 
> Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-top is 'top' for virtual machines.  Tiny program with many
> powerful monitoring features, net stats, disk stats, logging, etc.
> http://people.redhat.com/~rjones/virt-top
> >From 4c65e5564c525e7b4ab5d3268f37547b853c0d03 Mon Sep 17 00:00:00 2001
> From: Hu Tao <hutao@cn.fujitsu.com>
> Date: Mon, 1 Dec 2014 18:26:12 +0800
> Subject: [PATCH] fish: show synopsis if command syntax is wrong
> 
> This patch lets guestfish show command synopsis if the syntax of command
issued
> by user is wrong, rather than telling user that the number of parameters is
wrong.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  fish/cmds-gperf.h |  1 +
>  generator/fish.ml | 48 +++++++++++++++++++++++++-----------------------
>  2 files changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/fish/cmds-gperf.h b/fish/cmds-gperf.h
> index 74db69d..bcb3b5d 100644
> --- a/fish/cmds-gperf.h
> +++ b/fish/cmds-gperf.h
> @@ -25,6 +25,7 @@
>  struct command_entry {
>    const char *name;             /* Short name. */
>    const char *help;             /* Online help. */
> +  const char *synopsis;         /* Synopsis. */
>  
>    /* The run_* function. */
>    int (*run) (const char *cmd, size_t argc, char *argv[]);
> diff --git a/generator/fish.ml b/generator/fish.ml
> index 3f53ffa..b609ce9 100644
> --- a/generator/fish.ml
> +++ b/generator/fish.ml
> @@ -98,6 +98,10 @@ let generate_fish_cmds () >    pr "/* Valid
suffixes allowed for numbers.  See Gnulib xstrtol function. */\n";
>    pr "static const char *xstrtol_suffixes =
\"0kKMGTPEZY\";\n";
>    pr "\n";
> +  pr "/* Return these errors from run_* functions. */\n";
> +  pr "#define RUN_ERROR -1\n";
> +  pr "#define RUN_WRONG_ARGS -2\n";
> +  pr "\n";
>  
>    List.iter (
>      fun { name = name } ->
> @@ -129,6 +133,7 @@ let generate_fish_cmds () >        pr "struct
command_entry %s_cmd_entry = {\n" name;
>        pr "  .name = \"%s\",\n" name2;
>        pr "  .help = \"%s\",\n" (c_quote text);
> +      pr "  .synopsis = NULL,\n";
>        pr "  .run = run_%s\n" name;
>        pr "};\n";
>        pr "\n";
> @@ -190,6 +195,7 @@ Guestfish will prompt for these separately."
>        pr "struct command_entry %s_cmd_entry = {\n" name;
>        pr "  .name = \"%s\",\n" name2;
>        pr "  .help = \"%s\",\n" (c_quote text);
> +      pr "  .synopsis = \"%s\",\n" (c_quote synopsis);
>        pr "  .run = run_%s\n" name;
>        pr "};\n";
>        pr "\n";
> @@ -337,7 +343,7 @@ Guestfish will prompt for these separately."
>        pr "static int\n";
>        pr "run_%s (const char *cmd, size_t argc, char *argv[])\n"
name;
>        pr "{\n";
> -      pr "  int ret = -1;\n";
> +      pr "  int ret = RUN_ERROR;\n";
>        (match ret with
>         | RErr
>         | RInt _
> @@ -393,30 +399,14 @@ Guestfish will prompt for these separately."
>  
>        if argc_minimum = argc_maximum then (
>          pr "  if (argc != %d) {\n" argc_minimum;
> -        if argc_minimum = 0 then (
> -          pr "    fprintf (stderr, _(\"%%s should have no
parameters\\n\"), cmd);\n";
> -        ) else (
> -          pr "    fprintf (stderr, ngettext(\"%%s should have
%%d parameter\\n\",\n";
> -          pr "                              \"%%s should have
%%d parameters\\n\",\n";
> -          pr "                              %d),\n"
> -            argc_minimum;
> -          pr "                     cmd, %d);\n"
> -            argc_minimum;
> -        )
> +          pr "    ret = RUN_WRONG_ARGS;\n";
>        ) else if argc_minimum = 0 then (
>          pr "  if (argc > %d) {\n" argc_maximum;
> -        pr "    fprintf (stderr, ngettext(\"%%s should have at
most %%d parameter\\n\",\n";
> -        pr "                              \"%%s should have at
most %%d parameters\\n\",\n";
> -        pr "                              %d),\n"
> -          argc_maximum;
> -        pr "                     cmd, %d);\n"
> -          argc_maximum;
> +        pr "    ret = RUN_WRONG_ARGS;\n";
>        ) else (
>          pr "  if (argc < %d || argc > %d) {\n"
argc_minimum argc_maximum;
> -        pr "    fprintf (stderr, _(\"%%s should have %%d-%%d
parameter(s)\\n\"), cmd, %d, %d);\n"
> -          argc_minimum argc_maximum;
> +        pr "    ret = RUN_WRONG_ARGS;\n";
>        );
> -      pr "    fprintf (stderr, _(\"type 'help %%s' for
help on %%s\\n\"), cmd, cmd);\n";
>        pr "    goto out_noargs;\n";
>        pr "  }\n";
>  
> @@ -694,16 +684,28 @@ Guestfish will prompt for these separately."
>    pr "run_action (const char *cmd, size_t argc, char
*argv[])\n";
>    pr "{\n";
>    pr "  const struct command_table *ct;\n";
> +  pr "  int ret = -1;\n";
>    pr "\n";
>    pr "  ct = lookup_fish_command (cmd, strlen (cmd));\n";
> -  pr "  if (ct)\n";
> -  pr "    return ct->entry->run (cmd, argc, argv);\n";
> +  pr "  if (ct) {\n";
> +  pr "    ret = ct->entry->run (cmd, argc, argv);\n";
> +  pr "    /* run function may return magic value -2 (RUN_WRONG_ARGS)
to indicate\n";
> +  pr "     * that this function should print the command
synopsis.\n";
> +  pr "     */\n";
> +  pr "    if (ret == RUN_WRONG_ARGS) {\n";
> +  pr "      fprintf (stderr, _(\"error: incorrect number of
arguments\\n\"));\n";
> +  pr "      if (ct->entry->synopsis)\n";
> +  pr "        fprintf (stderr, _(\"usage: %%s\\n\"),
ct->entry->synopsis);\n";
> +  pr "      fprintf (stderr, _(\"type 'help %%s' for
more help on %%s\\n\"), cmd, cmd);\n";
> +  pr "      ret = -1;\n";
> +  pr "    }\n";
> +  pr "  }\n";
>    pr "  else {\n";
>    pr "    fprintf (stderr, _(\"%%s: unknown command\\n\"),
cmd);\n";
>    pr "    if (command_num == 1)\n";
>    pr "      extended_help_message ();\n";
> -  pr "    return -1;\n";
>    pr "  }\n";
> +  pr "  return ret;\n";
>    pr "}\n"
>  
>  and generate_fish_cmds_h () > -- 
> 2.1.0
>