> On Sep 10, 2018, at 4:16 AM, Kevin Wolf <kw...@redhat.com> wrote:
>
> Am 08.09.2018 um 05:16 hat Programmingkid geschrieben:
>>
>>> On Sep 7, 2018, at 11:13 PM, Peter Maydell <peter.mayd...@linaro.org> wrote:
>>>
>>> On 8 September 2018 at 04:01, John Arbuckle <programmingk...@gmail.com>
>>> 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.
Even though I would prefer to leave the img_cmds array alone, I do
see the advantages of your suggestion. All the code that checks which
command is being used could go. I will ad a help_text field to this
array.
> 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.
Yeah it does look very unreadable like that.
> 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.
Using lines longer than 80 would make the patch a lot easier to read. I will do
this.
Thank you.
