3:55 a.m.
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(a)cn.fujitsu.com>
---
fish/cmds-gperf.h | 1 +
generator/fish.ml | 33 ++++++++++++---------------------
2 files changed, 13 insertions(+), 21 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..951b376 100644
--- a/generator/fish.ml
+++ b/generator/fish.ml
@@ -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,10 +679,16 @@ 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 " }\n";
+ pr " }\n";
pr " else {\n";
pr " fprintf (stderr, _(\"%%s: unknown command\\n\"), cmd);\n";
pr " if (command_num == 1)\n";
--
1.9.3
8 a.m.
On Friday 28 November 2014 17:55:51 Hu Tao wrote:
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.
The idea seems sound to me.
Shouldn't that be done also for fish commands? If so, just make
"synopsis" a function taking args and optargs, and use it in both
places.
@@ -694,10 +679,16 @@ 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 " }\n";
+ pr " }\n";
There is an issue here: in the true branch of "if (ct)", there is no
more return, and thus builds fails when enabling -Werror
(--enable-werror for configure):
cmds.c: In function 'run_action':
cmds.c:23233:1: error: control reaches end of non-void function [-Werror=return-type]
}
^
Also, the return code -2 must be turned as -1.
--
Pino Toscano
8:25 p.m.
On Fri, Nov 28, 2014 at 03:00:39PM +0100, Pino Toscano wrote:
On Friday 28 November 2014 17:55:51 Hu Tao wrote:
> 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.
The idea seems sound to me.
Shouldn't that be done also for fish commands? If so, just make
"synopsis" a function taking args and optargs, and use it in both
places.
fish commands do not have args and optargs in their definitions in file
actions.ml. Also, fish commands' synopsises are printed out by inline
codes, see run_alloc() for an example.
Surely we can reconstruct fish commands' definitions to take advantage
of the auto-generation of synopsis done in this patch, but that can be
another patch. I'll add this to my todo list.
> @@ -694,10 +679,16 @@ 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 " }\n";
> + pr " }\n";
There is an issue here: in the true branch of "if (ct)", there is no
more return, and thus builds fails when enabling -Werror
(--enable-werror for configure):
cmds.c: In function 'run_action':
cmds.c:23233:1: error: control reaches end of non-void function [-Werror=return-type]
}
^
Also, the return code -2 must be turned as -1.
Thanks, both fixed.
Regards,
Hu
--
Pino Toscano
_______________________________________________
Libguestfs mailing list
Libguestfs(a)redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs
12:42 a.m.
On Mon, Dec 01, 2014 at 10:25:51AM +0800, Hu Tao wrote:
On Fri, Nov 28, 2014 at 03:00:39PM +0100, Pino Toscano wrote:
> On Friday 28 November 2014 17:55:51 Hu Tao wrote:
> > 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.
>
> The idea seems sound to me.
>
> Shouldn't that be done also for fish commands? If so, just make
> "synopsis" a function taking args and optargs, and use it in both
> places.
fish commands do not have args and optargs in their definitions in file
actions.ml. Also, fish commands' synopsises are printed out by inline
codes, see run_alloc() for an example.
Surely we can reconstruct fish commands' definitions to take advantage
of the auto-generation of synopsis done in this patch, but that can be
another patch. I'll add this to my todo list.
After some dig, the only problem I don't know how to solve is commands
with variable-length parameters, such as copy-in. Its synopsis is:
copy-in local [local ...] /remotedir
At first think its style could be:
"style = RErr, [StringList "local"], [];
But it seems the syntax corresponding to StringList is:
copy-in "local local" /remotedir
e.g. its a one parameter of List type, but not several parameters. Any
ideas?
Regards,
Hu
3:44 a.m.
On Mon, Dec 01, 2014 at 02:42:54PM +0800, Hu Tao wrote:
On Mon, Dec 01, 2014 at 10:25:51AM +0800, Hu Tao wrote:
> On Fri, Nov 28, 2014 at 03:00:39PM +0100, Pino Toscano wrote:
> > On Friday 28 November 2014 17:55:51 Hu Tao wrote:
> > > 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.
> >
> > The idea seems sound to me.
> >
> > Shouldn't that be done also for fish commands? If so, just make
> > "synopsis" a function taking args and optargs, and use it in both
> > places.
>
> fish commands do not have args and optargs in their definitions in file
> actions.ml. Also, fish commands' synopsises are printed out by inline
> codes, see run_alloc() for an example.
>
> Surely we can reconstruct fish commands' definitions to take advantage
> of the auto-generation of synopsis done in this patch, but that can be
> another patch. I'll add this to my todo list.
After some dig, the only problem I don't know how to solve is commands
with variable-length parameters, such as copy-in. Its synopsis is:
copy-in local [local ...] /remotedir
At first think its style could be:
"style = RErr, [StringList "local"], [];
But it seems the syntax corresponding to StringList is:
copy-in "local local" /remotedir
e.g. its a one parameter of List type, but not several parameters. Any
ideas?
As you've discovered, guestfish commands don't use the 'style' field
at all. Each command receives an argc/argv list of strings and parses
it however it wants to. It's not restricted by the usual rules for
other API functions and so can use variable length lists of arguments.
So there is no way to get from the struct in actions.ml to a synopsis.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
4:22 a.m.
On Mon, Dec 01, 2014 at 09:44:41AM +0000, Richard W.M. Jones wrote:
On Mon, Dec 01, 2014 at 02:42:54PM +0800, Hu Tao wrote:
> On Mon, Dec 01, 2014 at 10:25:51AM +0800, Hu Tao wrote:
> > On Fri, Nov 28, 2014 at 03:00:39PM +0100, Pino Toscano wrote:
> > > On Friday 28 November 2014 17:55:51 Hu Tao wrote:
> > > > 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.
> > >
> > > The idea seems sound to me.
> > >
> > > Shouldn't that be done also for fish commands? If so, just make
> > > "synopsis" a function taking args and optargs, and use it in
both
> > > places.
> >
> > fish commands do not have args and optargs in their definitions in file
> > actions.ml. Also, fish commands' synopsises are printed out by inline
> > codes, see run_alloc() for an example.
> >
> > Surely we can reconstruct fish commands' definitions to take advantage
> > of the auto-generation of synopsis done in this patch, but that can be
> > another patch. I'll add this to my todo list.
>
> After some dig, the only problem I don't know how to solve is commands
> with variable-length parameters, such as copy-in. Its synopsis is:
>
> copy-in local [local ...] /remotedir
>
> At first think its style could be:
>
> "style = RErr, [StringList "local"], [];
>
> But it seems the syntax corresponding to StringList is:
>
> copy-in "local local" /remotedir
>
> e.g. its a one parameter of List type, but not several parameters. Any
> ideas?
As you've discovered, guestfish commands don't use the 'style' field
at all. Each command receives an argc/argv list of strings and parses
it however it wants to. It's not restricted by the usual rules for
other API functions and so can use variable length lists of arguments.
So there is no way to get from the struct in actions.ml to a synopsis.
Okay. Thanks for your clarification!
Regards,
Hu
3643
days inactive
3646
days old
5 comments
3 participants
participants (3)
-
Hu Tao -
Pino Toscano -
Richard W.M. Jones