Re: [PATCH 1/5] doc: add documentation for OPT_STRING_LIST

From: Jacob Keller <jacob.keller@gmail.com>
Date: Jan 13, 2017, 00:51 UTC
Message-ID: <CA+P7+xoJfiqHy28z8ynonYR3ujoCUB4_diGJHP3XSp=iWpTB5A@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.20.1701121044120.3469@virtualbox>

On Thu, Jan 12, 2017 at 1:47 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 21 quoted lines

> Hi Jake,
>
> On Wed, 11 Jan 2017, Jacob Keller wrote:
>
>> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
>> index 27bd701c0d68..15e876e4c804 100644
>> --- a/Documentation/technical/api-parse-options.txt
>> +++ b/Documentation/technical/api-parse-options.txt
>> @@ -168,6 +168,11 @@ There are some macros to easily define options:
>>       Introduce an option with string argument.
>>       The string argument is put into `str_var`.
>>
>> +`OPT_STRING_LIST(short, long, &list, arg_str, description)`::
>> +     Introduce an option with a string argument. Repeated invocations
>> +     accumulate into a list of strings. Reset and clear the list with
>> +     `--no-option`.
>
> One suggestions: as the list parameter is not type-safe (apart from
> checking that it can be cast to a `void *`), it would be good to mention
> in the documentation that `list` must be of type `struct string_list`.
>

Ok.

> I was about to suggest that `--no-option` may be misleading, as the
> command-line option is not really called `--option` in almost all cases,
> but I see that the rest of that document uses that convention to refer to
> the negated option already...

Also, I am merely documenting what already existed, since the original commit failed to add documentation. I don't know if we could make a better implementation, but it certainly would be a behavior change for the current users.

Thanks, Jake

>
> Ciao,
> Dscho