qemu-block
[Top][All Lists]
Advanced

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

Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img.c: add help for each comm


From: Kevin Wolf
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH] qemu-img.c: add help for each command
Date: Mon, 10 Sep 2018 10:16:49 +0200
User-agent: Mutt/1.9.1 (2017-09-22)

Am 08.09.2018 um 05:16 hat Programmingkid geschrieben:
> 
> > On Sep 7, 2018, at 11:13 PM, Peter Maydell <address@hidden> wrote:
> > 
> > On 8 September 2018 at 04:01, John Arbuckle <address@hidden> wrote:
> > 
> >> +    /* print the help for this command */
> >> +    if (strcmp("--help", argv[optind + 1]) == 0) {
> >> +        if (strcmp("amend", cmdname) == 0) {
> >> +            help_amend();
> >> +        } else if (strcmp("bench", cmdname) == 0) {
> >> +            help_bench();
> >> +        } else if (strcmp("check", cmdname) == 0) {
> >> +            help_check();
> >> +        } else if (strcmp("commit", cmdname) == 0) {
> >> +            help_commit();
> >> +        } else if (strcmp("compare", cmdname) == 0) {
> >> +            help_compare();
> >> +        } else if (strcmp("convert", cmdname) == 0) {
> >> +            help_convert();
> >> +        } else if (strcmp("create", cmdname) == 0) {
> >> +            help_create();
> >> +        } else if (strcmp("dd", cmdname) == 0) {
> >> +            help_dd();
> >> +        } else if (strcmp("info", cmdname) == 0) {
> >> +            help_info();
> >> +        } else if (strcmp("map", cmdname) == 0) {
> >> +            help_map();
> >> +        } else if (strcmp("measure", cmdname) == 0) {
> >> +            help_measure();
> >> +        } else if (strcmp("snapshot", cmdname) == 0) {
> >> +            help_snapshot();
> >> +        } else if (strcmp("rebase", cmdname) == 0) {
> >> +            help_rebase();
> >> +        } else if (strcmp("resize", cmdname) == 0) {
> >> +            help_resize();
> > 
> > Any time you find yourself writing very repetitive code like
> > this, it's a good idea to ask yourself "is there a way to make
> > this data-driven?".
> > 
> > thanks
> > -- PMM
> 
> Did you want me to loop thru an array of strings instead?

There is already an array of all subcommands, img_cmds. You should
probably add another field there for the help text.

Also, your line wrapping (even mid-word!) is awful. I'm not sure we
really have to fill 80 characters in the output and can't simply keep
the lines a bit shorter so that 80 characters in the source are enough.

But if we do want to use the full 80 characters in the output, I'd
rather break the limit from the coding style and use longer lines in the
source coe than doing what you did.

Kevin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]