Hi Junio, On Wed, 18 Jan 2017, Junio C Hamano wrote:
> "Philip Oakley" <philipoak...@iee.org> 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
