[Libguestfs] [PATCH] fish: help command return error for non-existent commands (RHBZ#597145).
Matthew Booth
mbooth at redhat.com
Wed Jun 2 12:57:41 UTC 2010
On 01/06/10 16:30, Richard W.M. Jones wrote:
>> From ea5c6cf0341fdd8072b12723f9794506841bc688 Mon Sep 17 00:00:00 2001
> From: Richard Jones<rjones at 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
More information about the Libguestfs
mailing list