| Submitter | Kevin Wolf |
|---|---|
| Date | May 28, 2013, 3:27 p.m. |
| Message ID | <1369754856-30036-9-git-send-email-kwolf@redhat.com> |
| Download | mbox | patch |
| Permalink | /patch/246901/ |
| State | New |
| Headers | show |
Comments
On 05/28/2013 09:27 AM, Kevin Wolf wrote: > No reason to treat it different from other commands. Move it to > qemu-io-cmds.c, adapt the coding style and register it like any other > command. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > cmd.c | 79 ---------------------------------------------------------- > cmd.h | 1 - > qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 66 insertions(+), 81 deletions(-) > -void > -help_init(void) > -{ > - help_cmd.name = _("help"); > - help_cmd.altname = _("?"); > - help_cmd.cfunc = help_f; > - help_cmd.argmin = 0; > - help_cmd.argmax = 1; > - help_cmd.flags = CMD_FLAG_GLOBAL; > - help_cmd.args = _("[command]"); > - help_cmd.oneline = _("help for one or all commands"); The old code marked strings for translation with _(), ... > +static const cmdinfo_t help_cmd = { > + .name = "help", > + .altname = "?", > + .cfunc = help_f, > + .argmin = 0, > + .argmax = 1, > + .flags = CMD_FLAG_GLOBAL, > + .args = "[command]", > + .oneline = "help for one or all commands", ...whereas the new code uses fixed literals. Then again, po/messages.po doesn't contain any mention of cmd.c, so it's not like we were actually translating the strings, even if they were marked for translation. So no real change. Reviewed-by: Eric Blake <eblake@redhat.com>
Am 29.05.2013 um 23:25 hat Eric Blake geschrieben: > On 05/28/2013 09:27 AM, Kevin Wolf wrote: > > No reason to treat it different from other commands. Move it to > > qemu-io-cmds.c, adapt the coding style and register it like any other > > command. > > > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > > --- > > cmd.c | 79 ---------------------------------------------------------- > > cmd.h | 1 - > > qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- > > 3 files changed, 66 insertions(+), 81 deletions(-) > > > -void > > -help_init(void) > > -{ > > - help_cmd.name = _("help"); > > - help_cmd.altname = _("?"); > > - help_cmd.cfunc = help_f; > > - help_cmd.argmin = 0; > > - help_cmd.argmax = 1; > > - help_cmd.flags = CMD_FLAG_GLOBAL; > > - help_cmd.args = _("[command]"); > > - help_cmd.oneline = _("help for one or all commands"); > > The old code marked strings for translation with _(), ... > > > +static const cmdinfo_t help_cmd = { > > + .name = "help", > > + .altname = "?", > > + .cfunc = help_f, > > + .argmin = 0, > > + .argmax = 1, > > + .flags = CMD_FLAG_GLOBAL, > > + .args = "[command]", > > + .oneline = "help for one or all commands", > > ...whereas the new code uses fixed literals. Then again, po/messages.po > doesn't contain any mention of cmd.c, so it's not like we were actually > translating the strings, even if they were marked for translation. So > no real change. In fact, the real reason is that cmd.c has this line: #define _(x) x /* not gettext support yet */ Apart from that, translating command names has never been a great idea anyway, so at least that part wouldn't a big loss even if gettext was actually used... Kevin
On Tue, May 28, 2013 at 05:27:28PM +0200, Kevin Wolf wrote: > No reason to treat it different from other commands. Move it to > qemu-io-cmds.c, adapt the coding style and register it like any other > command. > > Signed-off-by: Kevin Wolf <kwolf@redhat.com> > --- > cmd.c | 79 ---------------------------------------------------------- > cmd.h | 1 - > qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- > 3 files changed, 66 insertions(+), 81 deletions(-) > > diff --git a/cmd.c b/cmd.c > index 7ae978f..2941ad3 100644 > --- a/cmd.c > +++ b/cmd.c 'help' is a generic command, it's not specific to qemu-io. IMO cmd.c is the right place for it since other command-line tools might also need 'help'.
Patch
diff --git a/cmd.c b/cmd.c index 7ae978f..2941ad3 100644 --- a/cmd.c +++ b/cmd.c @@ -439,82 +439,3 @@ quit_init(void) add_command(&quit_cmd); } - -/* from libxcmd/help.c */ - -static cmdinfo_t help_cmd; -static void help_onecmd(const char *cmd, const cmdinfo_t *ct); -static void help_oneline(const char *cmd, const cmdinfo_t *ct); - -static void -help_all(void) -{ - const cmdinfo_t *ct; - - for (ct = cmdtab; ct < &cmdtab[ncmds]; ct++) - help_oneline(ct->name, ct); - printf(_("\nUse 'help commandname' for extended help.\n")); -} - -static int -help_f( - BlockDriverState *bs, - int argc, - char **argv) -{ - const cmdinfo_t *ct; - - if (argc == 1) { - help_all(); - return 0; - } - ct = find_command(argv[1]); - if (ct == NULL) { - printf(_("command %s not found\n"), argv[1]); - return 0; - } - help_onecmd(argv[1], ct); - return 0; -} - -static void -help_onecmd( - const char *cmd, - const cmdinfo_t *ct) -{ - help_oneline(cmd, ct); - if (ct->help) - ct->help(); -} - -static void -help_oneline( - const char *cmd, - const cmdinfo_t *ct) -{ - if (cmd) - printf("%s ", cmd); - else { - printf("%s ", ct->name); - if (ct->altname) - printf("(or %s) ", ct->altname); - } - if (ct->args) - printf("%s ", ct->args); - printf("-- %s\n", ct->oneline); -} - -void -help_init(void) -{ - help_cmd.name = _("help"); - help_cmd.altname = _("?"); - help_cmd.cfunc = help_f; - help_cmd.argmin = 0; - help_cmd.argmax = 1; - help_cmd.flags = CMD_FLAG_GLOBAL; - help_cmd.args = _("[command]"); - help_cmd.oneline = _("help for one or all commands"); - - add_command(&help_cmd); -} diff --git a/cmd.h b/cmd.h index d676408..89e7c6e 100644 --- a/cmd.h +++ b/cmd.h @@ -42,7 +42,6 @@ typedef struct cmdinfo { extern cmdinfo_t *cmdtab; extern int ncmds; -void help_init(void); void quit_init(void); typedef int (*checkfunc_t)(BlockDriverState *bs, const cmdinfo_t *ci); diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index bdfe5f2..e2d65d7 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -1795,6 +1795,71 @@ static const cmdinfo_t abort_cmd = { .oneline = "simulate a program crash using abort(3)", }; +static void help_oneline(const char *cmd, const cmdinfo_t *ct) +{ + if (cmd) { + printf("%s ", cmd); + } else { + printf("%s ", ct->name); + if (ct->altname) { + printf("(or %s) ", ct->altname); + } + } + + if (ct->args) { + printf("%s ", ct->args); + } + printf("-- %s\n", ct->oneline); +} + +static void help_onecmd(const char *cmd, const cmdinfo_t *ct) +{ + help_oneline(cmd, ct); + if (ct->help) { + ct->help(); + } +} + +static void help_all(void) +{ + const cmdinfo_t *ct; + + for (ct = cmdtab; ct < &cmdtab[ncmds]; ct++) { + help_oneline(ct->name, ct); + } + printf("\nUse 'help commandname' for extended help.\n"); +} + +static int help_f(BlockDriverState *bs, int argc, char **argv) +{ + const cmdinfo_t *ct; + + if (argc == 1) { + help_all(); + return 0; + } + + ct = find_command(argv[1]); + if (ct == NULL) { + printf("command %s not found\n", argv[1]); + return 0; + } + + help_onecmd(argv[1], ct); + return 0; +} + +static const cmdinfo_t help_cmd = { + .name = "help", + .altname = "?", + .cfunc = help_f, + .argmin = 0, + .argmax = 1, + .flags = CMD_FLAG_GLOBAL, + .args = "[command]", + .oneline = "help for one or all commands", +}; + static int init_check_command(BlockDriverState *bs, const cmdinfo_t *ct) { if (ct->flags & CMD_FLAG_GLOBAL) { @@ -1834,7 +1899,7 @@ bool qemuio_command(const char *cmd) static void __attribute((constructor)) init_qemuio_commands(void) { /* initialize commands */ - help_init(); + add_command(&help_cmd); add_command(&read_cmd); add_command(&readv_cmd); add_command(&write_cmd);
No reason to treat it different from other commands. Move it to qemu-io-cmds.c, adapt the coding style and register it like any other command. Signed-off-by: Kevin Wolf <kwolf@redhat.com> --- cmd.c | 79 ---------------------------------------------------------- cmd.h | 1 - qemu-io-cmds.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 81 deletions(-)