[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH] fish: help command return error for non-existent commands (RHBZ#597145).



On 01/06/10 16:30, Richard W.M. Jones wrote:
From ea5c6cf0341fdd8072b12723f9794506841bc688 Mon Sep 17 00:00:00 2001
From: Richard Jones<rjones redhat com>
Date: Tue, 1 Jun 2010 16:27:33 +0100
Subject: [PATCH] fish: help command return error for non-existent commands (RHBZ#597145).

With this change, the exit status indicates error for non-existent
commands.

$ guestfish -h foo
foo: command not known, use -h to list all commands
$ echo $?
1
$ guestfish help foo
foo: command not known, use -h to list all commands
$ echo $?
1
---
  fish/fish.c      |   78 ++++++++++++++++++++++++++++++++++++++----------------
  fish/fish.h      |    4 +-
  src/generator.ml |    8 +++--
  3 files changed, 62 insertions(+), 28 deletions(-)

diff --git a/fish/fish.c b/fish/fish.c
index 557a6ac..470c625 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -1166,7 +1190,9 @@ display_builtin_command (const char *cmd)
                "\n"
                "    Size can be specified using standard suffixes, eg. '1M'.\n"
                ));
-  else if (STRCASEEQ (cmd, "supported"))
+    return 0;
+  }
+  else if (STRCASEEQ (cmd, "supported")) {
      printf (_("supported - list supported groups of commands\n"
                "     supported\n"
                "\n"
@@ -1176,15 +1202,21 @@ display_builtin_command (const char *cmd)
                "\n"
                "    See also guestfs(3) section AVAILABILITY.\n"
                ));
-  else if (STRCASEEQ (cmd, "time"))
+    return 0;
+  }
+  else if (STRCASEEQ (cmd, "time")) {
      printf (_("time - measure time taken to run command\n"
                "    time<command>  [<args>  ...]\n"
                "\n"
                "    This runs<command>  as usual, and prints the elapsed\n"
                "    time afterwards.\n"));
-  else
+    return 0;
+  }
+  else {
      fprintf (stderr, _("%s: command not known, use -h to list all commands\n"),
               cmd);
+    return -1;
+  }
  }

You could get rid of all of the above 'return 0' statements by allowing them to fall through and having a single 'return 0' at the end of the function. This would also be clearer IMHO.

On a related note, I'd still put curlies round all the single-line if statements.



  /* This is printed when the user types in an unknown command for the
diff --git a/fish/fish.h b/fish/fish.h
index b98faf0..9f64979 100644
--- a/fish/fish.h
+++ b/fish/fish.h
@@ -55,7 +55,7 @@ extern int command_num;
  extern int issue_command (const char *cmd, char *argv[], const char *pipe);
  extern void pod2text (const char *name, const char *shortdesc, const char *body);
  extern void list_builtin_commands (void);
-extern void display_builtin_command (const char *cmd);
+extern int display_builtin_command (const char *cmd);
  extern void free_strings (char **argv);
  extern int count_strings (char *const *argv);
  extern void print_strings (char *const *argv);
@@ -71,7 +71,7 @@ extern void extended_help_message (void);

  /* in cmds.c (auto-generated) */
  extern void list_commands (void);
-extern void display_command (const char *cmd);
+extern int display_command (const char *cmd);
  extern int run_action (const char *cmd, int argc, char *argv[]);

  /* in completion.c (auto-generated) */
diff --git a/src/generator.ml b/src/generator.ml
index 2c33049..571ea3a 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -7575,7 +7575,7 @@ and generate_fish_cmds () =
    pr "\n";

    (* display_command function, which implements guestfish -h cmd *)
-  pr "void display_command (const char *cmd)\n";
+  pr "int display_command (const char *cmd)\n";
    pr "{\n";
    List.iter (
      fun (name, style, _, flags, _, shortdesc, longdesc) ->
@@ -7623,15 +7623,17 @@ and generate_fish_cmds () =
          pr " || STRCASEEQ (cmd, \"%s\")" name2;
        if name<>  alias then
          pr " || STRCASEEQ (cmd, \"%s\")" alias;
-      pr ")\n";
+      pr ") {\n";
        pr "    pod2text (\"%s\", _(\"%s\"), %S);\n"
          name2 shortdesc
          ("=head1 SYNOPSIS\n\n " ^ synopsis ^ "\n\n" ^
           "=head1 DESCRIPTION\n\n" ^
           longdesc ^ warnings ^ describe_alias);
+      pr "    return 0;\n";
+      pr "  }\n";
        pr "  else\n"
    ) all_functions;
-  pr "    display_builtin_command (cmd);\n";
+  pr "    return display_builtin_command (cmd);\n";
    pr "}\n";
    pr "\n";

I guess it doesn't matter so much here, though.

Apart from the style point, though, ACK.

Matt
--
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]