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

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

 



Hi Junio,

On Wed, 18 Jan 2017, Junio C Hamano wrote:

> "Philip Oakley" <philipoakley@xxxxxxx> writes:
> 
> >>> +`OPT_STRING_LIST(short, long, &list, arg_str, description)`::
> >>> + Introduce an option with string argument.
> >>> + The string argument is stored as an element in `&list` which must be a
> >>> + struct string_list. Reset the list using `--no-option`.
> >>> +
> >>
> >> I do not know if it is clear enough that 'option' in the last
> >> sentence is a placeholder.  I then wondered if spelling it as
> >> `--no-<long>` would make it a bit clearer, but that is ugly.
> >
> > Bikeshedding:: `--no-<option>` maybe, i.e. just surround the option
> > word with the angle brackets to indicate it is to be replaced by the
> > real option's name.
> 
> Yeah, I bikeshedded that myself, and rejected it because there is no
> <option> mentioned anywhere in the enumeration header.

As I pointed out in a previous review round: the surrounding test uses
--no-option (I agree that it is tedious to go back to the original code
for review, rather than a mere patch review that lacks context, but I
suspected that Jake did not come up with that `--no-option` himself), so
by our own recommendation (imitate the surrounding, existing code/text)
Jake did exactly the right thing:

$ git grep -e --no-option upstream/master -- Documentation/technical/api-parse-options.txt
upstream/master:Documentation/technical/api-parse-options.txt:	`--option` and set to zero with `--no-option`.
upstream/master:Documentation/technical/api-parse-options.txt:	(even if initially negative), and `--no-option` resets it to
upstream/master:Documentation/technical/api-parse-options.txt:	zero. To determine if `--option` or `--no-option` was encountered at
upstream/master:Documentation/technical/api-parse-options.txt:	`--no-option` was seen.
upstream/master:Documentation/technical/api-parse-options.txt:	reset to zero with `--no-option`.

Ciao,
Johannes




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]