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

2014-02-02 Thread Eric Sunshine
Patch 17/17 of v4 failed to arrive in my inbox for some reason, so
I'll just reply to v3 since there's another error I noticed which is
still present in v4, plus a comment specific to v4 (see below).

On Mon, Jan 27, 2014 at 3:33 PM, Christian Couder
chrisc...@tuxfamily.org wrote:
 From: Eric Sunshine sunsh...@sunshineco.com
 On Sun, Jan 26, 2014 at 12:00 PM, Christian Couder
 chrisc...@tuxfamily.org wrote:
 +By default, a 'token=value' or 'token:value' argument will be added
 +only if no trailer with the same (token, value) pair is already in the
 +message. The 'token' and 'value' parts will be trimmed to remove
 +starting and trailing white spaces, and the resulting trimmed 'token'

 Other git documentation uniformly spells it as whitespace rather
 than white spaces.

 You are right I will change that.

The rest of git documentation consistently spells it whitespace, but
v4 uses whitespaces.

 +infile=file::
 +   Read the commit message from `file` instead of the standard
 +   input.

I didn't notice this when reviewing v3, and the same text appears in
v4. There are a couple extra hyphens before 'infile', and you want 
and  around file, so:

s/infile=file/--infile=file/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

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

 Christian Couder chrisc...@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.

 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-01-29 Thread Junio C Hamano
Christian Couder chrisc...@tuxfamily.org writes:

 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.

It is a bad design taste to structure the command line argument in
such a way that it takes

git cmd xyzzy=frotz nitfol=rezrov some other args

where these 'xyzzy', 'nitfol', etc. form an unbound set.  It
prevents future enhancements to the command from allowing anything
that contain '=' in some other args part.

Allowing not just '=' but also ':' makes it even worse.

But these are issues in the design itself.  Not the issue in the
patch 17/17 that is trying to document the design.  This patch under
discussion documents the bad design correctly ;-)


--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-01-27 Thread Christian Couder
From: Eric Sunshine sunsh...@sunshineco.com

 On Sun, Jan 26, 2014 at 12:00 PM, Christian Couder
 chrisc...@tuxfamily.org wrote:
 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 ---
 diff --git a/Documentation/git-interpret-trailers.txt 
 b/Documentation/git-interpret-trailers.txt
 new file mode 100644
 index 000..f74843e
 --- /dev/null
 +++ b/Documentation/git-interpret-trailers.txt
 @@ -0,0 +1,137 @@
 +git-interpret-trailers(1)
 +=
 +
 +NAME
 +
 +git-interpret-trailers - help add stuctured information into commit messages
 +
 +SYNOPSIS
 +
 +[verse]
 +'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

 +DESCRIPTION
 +---
 +Help add RFC 822 like headers, called 'trailers', at the end of the
 
 s/822 like/822-like/

Ok.

 Was the suggestion, early in the discussion, to use footer rather
 than trailer dismissed?

During the early discussions people initially talked about footer
while Junio and others talked about trailer.

See:

http://thread.gmane.org/gmane.comp.version-control.git/236429/
http://thread.gmane.org/gmane.comp.version-control.git/236770/

I have no preference for one or the other, but by default I use what
Junio uses.

 +otherwise free-form part of a commit message.
 +
 +Unless `--infile=file` is used, this command is a filter. It reads the
 +standard input for a commit message and apply the `token` arguments,
 
 s/apply/applies/

Ok.

 +if any, to this message. The resulting message is emited on the
 +standard output.
 +
 +Some configuration variables control the way the `token` arguments are
 +applied to the message and the way any existing trailer in the message
 +is changed. They also make it possible to automatically add some
 +trailers.
 +
 +By default, a 'token=value' or 'token:value' argument will be added
 +only if no trailer with the same (token, value) pair is already in the
 +message. The 'token' and 'value' parts will be trimmed to remove
 +starting and trailing white spaces, and the resulting trimmed 'token'
 
 Other git documentation uniformly spells it as whitespace rather
 than white spaces.

You are right I will change that.

 +and 'value' will appear in the message like this:
 +
 +
 +token: value
 +
 +
 +By default, if there are already trailers with the same 'token' the
 +trailer will appear just after the last trailer with the same
 
 It might be a bit clearer to say the _new_ trailer will appear

Ok.

 +'token'. Otherwise it will appear at the end of the message.
 +
 +Note that 'trailers' do not follow and are not intended to follow many
 +rules that are in RFC 822. For example they do not follow the line
 +breaking rules, the encoding rules and probably many other rules.
 +
 +Trailers have become a de facto standard way to add helpful structured
 +information into commit messages. For example the well known
 +Signed-off-by:  trailer is used by many projects like the Linux
 +kernel and Git.
 
 This justification paragraph might make more sense near or at the
 very the top of the Description section.

Yeah, or maybe I can remove it entirely.

 +OPTIONS
 +---
 +--trim-empty::
 +   If the 'value' part of any trailer contains onlywhite spaces,
 
 s/onlywhite spaces/only whitespace/

Ok.

 +   the whole trailer will be removed from the resulting message.
 +
 +infile=file::
 +   Read the commit message from `file` instead of the standard
 +   input.
 +
 +CONFIGURATION VARIABLES
 +---
 +
 +trailer.token.key::
 +   This 'key' will be used instead of 'token' in the
 +   trailer. After some alphanumeric characters, it can contain
 +   some non alphanumeric characters like ':', '=' or '#' that will
 +   be used instead of ':' to separate the token from the value in
 +   the trailer, though the default ':' is more standard.
 +
 +trailer.token.where::
 +   This can be either `after`, which is the default, or
 +   `before`. If it is `before`, then a trailer with the specified
 +   token, will appear before, instead of after, other trailers
 +   with the same token, or otherwise at the beginning, instead of
 +   at the end, of all the trailers.
 +
 +trailer.token.ifexist::
 +   This option makes it possible to chose what action will be
 
 s/chose/choose/

Ok.

 +   performed when there is already at least one trailer with the
 +   same token in the message.
 ++
 +The valid values for this option are: `addIfDifferent` (this is the
 +default), `addIfDifferentNeighbor`, `add`, `overwrite` or `doNothing`.
 ++
 +With `addIfDifferent`, a new trailer will be added only if no trailer
 +with the 

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

2014-01-27 Thread Junio C Hamano
Christian Couder chrisc...@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.  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.

Also I think [token[=value]]... is the correct way to spell
that there can be 0 or more token[=value].  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.

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.

Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2014-01-26 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-interpret-trailers.txt | 137 +++
 1 file changed, 137 insertions(+)
 create mode 100644 Documentation/git-interpret-trailers.txt

diff --git a/Documentation/git-interpret-trailers.txt 
b/Documentation/git-interpret-trailers.txt
new file mode 100644
index 000..f74843e
--- /dev/null
+++ b/Documentation/git-interpret-trailers.txt
@@ -0,0 +1,137 @@
+git-interpret-trailers(1)
+=
+
+NAME
+
+git-interpret-trailers - help add stuctured information into commit messages
+
+SYNOPSIS
+
+[verse]
+'git interpret-trailers' [--trim-empty] [--infile=file] [token[=value]...]
+
+DESCRIPTION
+---
+Help add RFC 822 like headers, called 'trailers', at the end of the
+otherwise free-form part of a commit message.
+
+Unless `--infile=file` is used, this command is a filter. It reads the
+standard input for a commit message and apply the `token` arguments,
+if any, to this message. The resulting message is emited on the
+standard output.
+
+Some configuration variables control the way the `token` arguments are
+applied to the message and the way any existing trailer in the message
+is changed. They also make it possible to automatically add some
+trailers.
+
+By default, a 'token=value' or 'token:value' argument will be added
+only if no trailer with the same (token, value) pair is already in the
+message. The 'token' and 'value' parts will be trimmed to remove
+starting and trailing white spaces, and the resulting trimmed 'token'
+and 'value' will appear in the message like this:
+
+
+token: value
+
+
+By default, if there are already trailers with the same 'token' the
+trailer will appear just after the last trailer with the same
+'token'. Otherwise it will appear at the end of the message.
+
+Note that 'trailers' do not follow and are not intended to follow many
+rules that are in RFC 822. For example they do not follow the line
+breaking rules, the encoding rules and probably many other rules.
+
+Trailers have become a de facto standard way to add helpful structured
+information into commit messages. For example the well known
+Signed-off-by:  trailer is used by many projects like the Linux
+kernel and Git.
+
+OPTIONS
+---
+--trim-empty::
+   If the 'value' part of any trailer contains onlywhite spaces,
+   the whole trailer will be removed from the resulting message.
+
+infile=file::
+   Read the commit message from `file` instead of the standard
+   input.
+
+CONFIGURATION VARIABLES
+---
+
+trailer.token.key::
+   This 'key' will be used instead of 'token' in the
+   trailer. After some alphanumeric characters, it can contain
+   some non alphanumeric characters like ':', '=' or '#' that will
+   be used instead of ':' to separate the token from the value in
+   the trailer, though the default ':' is more standard.
+
+trailer.token.where::
+   This can be either `after`, which is the default, or
+   `before`. If it is `before`, then a trailer with the specified
+   token, will appear before, instead of after, other trailers
+   with the same token, or otherwise at the beginning, instead of
+   at the end, of all the trailers.
+
+trailer.token.ifexist::
+   This option makes it possible to chose what action will be
+   performed when there is already at least one trailer with the
+   same token in the message.
++
+The valid values for this option are: `addIfDifferent` (this is the
+default), `addIfDifferentNeighbor`, `add`, `overwrite` or `doNothing`.
++
+With `addIfDifferent`, a new trailer will be added only if no trailer
+with the same (token, value) pair is already in the message.
++
+With `addIfDifferentNeighbor`, a new trailer will be added only if no
+trailer with the same (token, value) pair is above or below the line
+where the new trailer will be added.
++
+With `add`, a new trailer will be added, even if some trailers with
+the same (token, value) pair are already in the message.
++
+With `overwrite`, the new trailer will overwrite an existing trailer
+with the same token.
++
+With `doNothing`, nothing will be done, that is no new trailer will be
+added if there is already one with the same token in the message.
+
+trailer.token.ifmissing::
+   This option makes it possible to chose what action will be
+   performed when there is not yet any trailer with the same
+   token in the message.
++
+The valid values for this option are: `add` (this is the default) and
+`doNothing`.
++
+With `add`, a new trailer will be added.
++
+With `doNothing`, nothing will be done.
+
+trailer.token.command::
+   This option can be used to specify a shell command that will
+   be used to automatically add or modify a trailer with the
+   specified 'token'.
++
+When this 

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

2014-01-26 Thread Eric Sunshine
On Sun, Jan 26, 2014 at 12:00 PM, Christian Couder
chrisc...@tuxfamily.org wrote:
 Signed-off-by: Christian Couder chrisc...@tuxfamily.org
 ---
 diff --git a/Documentation/git-interpret-trailers.txt 
 b/Documentation/git-interpret-trailers.txt
 new file mode 100644
 index 000..f74843e
 --- /dev/null
 +++ b/Documentation/git-interpret-trailers.txt
 @@ -0,0 +1,137 @@
 +git-interpret-trailers(1)
 +=
 +
 +NAME
 +
 +git-interpret-trailers - help add stuctured information into commit messages
 +
 +SYNOPSIS
 +
 +[verse]
 +'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]]...

 +DESCRIPTION
 +---
 +Help add RFC 822 like headers, called 'trailers', at the end of the

s/822 like/822-like/

Was the suggestion, early in the discussion, to use footer rather
than trailer dismissed?

 +otherwise free-form part of a commit message.
 +
 +Unless `--infile=file` is used, this command is a filter. It reads the
 +standard input for a commit message and apply the `token` arguments,

s/apply/applies/

 +if any, to this message. The resulting message is emited on the
 +standard output.
 +
 +Some configuration variables control the way the `token` arguments are
 +applied to the message and the way any existing trailer in the message
 +is changed. They also make it possible to automatically add some
 +trailers.
 +
 +By default, a 'token=value' or 'token:value' argument will be added
 +only if no trailer with the same (token, value) pair is already in the
 +message. The 'token' and 'value' parts will be trimmed to remove
 +starting and trailing white spaces, and the resulting trimmed 'token'

Other git documentation uniformly spells it as whitespace rather
than white spaces.

 +and 'value' will appear in the message like this:
 +
 +
 +token: value
 +
 +
 +By default, if there are already trailers with the same 'token' the
 +trailer will appear just after the last trailer with the same

It might be a bit clearer to say the _new_ trailer will appear

 +'token'. Otherwise it will appear at the end of the message.
 +
 +Note that 'trailers' do not follow and are not intended to follow many
 +rules that are in RFC 822. For example they do not follow the line
 +breaking rules, the encoding rules and probably many other rules.
 +
 +Trailers have become a de facto standard way to add helpful structured
 +information into commit messages. For example the well known
 +Signed-off-by:  trailer is used by many projects like the Linux
 +kernel and Git.

This justification paragraph might make more sense near or at the
very the top of the Description section.

 +OPTIONS
 +---
 +--trim-empty::
 +   If the 'value' part of any trailer contains onlywhite spaces,

s/onlywhite spaces/only whitespace/

 +   the whole trailer will be removed from the resulting message.
 +
 +infile=file::
 +   Read the commit message from `file` instead of the standard
 +   input.
 +
 +CONFIGURATION VARIABLES
 +---
 +
 +trailer.token.key::
 +   This 'key' will be used instead of 'token' in the
 +   trailer. After some alphanumeric characters, it can contain
 +   some non alphanumeric characters like ':', '=' or '#' that will
 +   be used instead of ':' to separate the token from the value in
 +   the trailer, though the default ':' is more standard.
 +
 +trailer.token.where::
 +   This can be either `after`, which is the default, or
 +   `before`. If it is `before`, then a trailer with the specified
 +   token, will appear before, instead of after, other trailers
 +   with the same token, or otherwise at the beginning, instead of
 +   at the end, of all the trailers.
 +
 +trailer.token.ifexist::
 +   This option makes it possible to chose what action will be

s/chose/choose/

 +   performed when there is already at least one trailer with the
 +   same token in the message.
 ++
 +The valid values for this option are: `addIfDifferent` (this is the
 +default), `addIfDifferentNeighbor`, `add`, `overwrite` or `doNothing`.
 ++
 +With `addIfDifferent`, a new trailer will be added only if no trailer
 +with the same (token, value) pair is already in the message.
 ++
 +With `addIfDifferentNeighbor`, a new trailer will be added only if no
 +trailer with the same (token, value) pair is above or below the line
 +where the new trailer will be added.
 ++
 +With `add`, a new trailer will be added, even if some trailers with
 +the same (token, value) pair are already in the message.
 ++
 +With `overwrite`, the new trailer will overwrite an existing trailer
 +with the same token.
 ++
 +With `doNothing`, nothing will be done, that is no new trailer will be
 +added if there is already one with the same token in the