Re: [PATCH v3 17/17] Documentation: add documentation for 'git interpret-trailers'

From: Christian Couder <chriscool@tuxfamily.org>
Date: Jan 29, 2014, 20:01 UTC
Message-ID: <20140129.210102.2157679962537036887.chriscool@tuxfamily.org>
In-Reply-To: <xmqqfvo9nmul.fsf@gitster.dls.corp.google.com>

From: Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v3 17/17] Documentation: add documentation for 'git interpret-trailers'
Date: Mon, 27 Jan 2014 13:20:18 -0800

Show 16 quoted lines

> Christian Couder <chriscool@tuxfamily.org> writes:
> 
>>>> +'git interpret-trailers' [--trim-empty] [--infile=file] [<token[=value]>...]
>>> 
>>> Would it be more consistent with existing documentation to format this as so?
>>> 
>>>   [--infile=<file>] [<token>[=<value>]]...
>>
>> No, it would be very inconsistent:
>>
>> $ grep '\.\.\.\]' *.txt | wc -l
>> 103
>> $ grep '\]\.\.\.' *.txt | wc -l
>> 0
> 
> I have a feeling that you are missing the point Eric is making.

Yeah I realize I missed some of his points. Sorry about that.

> The
> value given to the --infile option can be anything, i.e. 'file'
> there is a placeholder, hence "--infile=<file>" not "--infile=file"
> as you wrote.

Ok, I agree with that.

> Also I think "[<token>[=<value>]]..." is the correct way to spell
> that there can be 0 or more "<token>[=<value>]".

Perhaps but it is not very consistent with what there is in other synopsis strings. For example the commands that accept a path have a synopsis that ends with "[--] [<path>...]" and not "[--] [<path>]...".

Perhaps [(<token>[=<value>])...] is more correct, but not worth the added complexity.

Show 6 quoted lines

> "<token[=value]>"
> in the original does not make any sense, as <> is meant to say "this
> thing is a placeholder", and we do not try to say, with the string
> inside <>, what shape that placeholder takes.  In fact '=' part is
> _not_ a placeholder but is required syntactically when you want to
> supply a value to the token, so the original doubly is incorrect.

Yeah perhaps, except that in the code ':' is accepted instead of '=', and the documention says that.

So perhaps [(<token>[(=|:)<value>])...] is even more correct.

> I find it a bad taste to allow unbound set of <token> on the LHS of
> '=' on the command line, but that is a separate issue in the design,
> not in the documentation of the design.

I don't understand this sentence, sorry.

Thanks, Christian.