Re: [PATCH v9 11/11] Documentation: add documentation for 'git interpret-trailers'
Christian Couder writes: > From: Junio C Hamano >> >> A different way to sell a colon, e.g. >> >> Consider the instruction sed takes on its command line. >> (e.g. "sed 's/frotz/nitfol/' > form, you would always give it as the value of an '-e' option >> (e.g. "sed -e 's/frotz/nitfol' > be loose in limited occassions. "Key:value" is like that, and >> in the most general form, it actually needs to be spelled as >> "-e 'Key:value'". >> >> is possible, but I do not think it is a particularly good analogy, >> because what you have as the alternative is "Key=value", and not >> "-e 'Key:value'", or "--Key=value" (the last would probably be the >> most natural way to express this). > > The analogy that I would use is rather that Perl lets people use > 's:foo:bar:' as well as 's=foo=bar=' instead of 's/foo/bar/' if they > prefer. I could *almost* buy that, but that does not hold as you are not allowing (and I do not think you need to in this case) the user to pick any termination character like "s|foo|bar|". The only thing you are doing is forbidding both ":" and "=" from the set of allowed characters for labels. However. I think we could buy the syntax if the "Key:value" form were the *only* form, *without* accepting "Key=value". The latter is a poor attempt to pretend as if it is a normal command line option, but because that form does not even take double-dashes at the beginning, it even fails to mimic as a command line option. It would be one way to reduce the unnecessary cognitive load from the users when learning the Git command line argument convention to reject the "key=value" form and only stick to "key: value" form. After all, because of the shape of the footer we add to the log message (i.e. a keyword label followed by a colon followed by a SP followed by the value), it is clear that we can use ":" as the separator without inconveniencing the users who want to use some unusual characters in the label part, but there is no strong reason to reject an equal sign in the label. -- 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 v9 11/11] Documentation: add documentation for 'git interpret-trailers'
Christian Couder writes: >> I do not see these two as valid arguments to make the command line >> more complex to the end users > > I don't think that it makes the command more complex to the end users. > >> ---who now need to know that only this >> command treats its command line in a funny way, accepting a colon in >> place of an equal sign. I meant that it makes learning the "command line syntax of Git" more complex to new users. If one is too narrowly focused on this single command, it may not seem that "the command" is not made more complex, but I am more interested in making sure that the entire Git command line experience does not get unnecessarily harder to learn. -- 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 v9 11/11] Documentation: add documentation for 'git interpret-trailers'
From: Junio C Hamano > > Christian Couder writes: > >> First accepting both ':' and '=' means one can see the "git >> interpret-trailers" as acting on trailers only. Not just on trailers >> from the intput message and option parameters from the command line. > > Sorry, you lost me. What does "acting on trailers only" really > mean? It means that the command can seen as processing only trailers, (from stdin and from its arguments), sorry if I used the wrong verb. > Do you mean the command should/can be run without any command > line options, pick up the existing "Signed-off-by:" and friends in > its input and emit its output, somehow taking these existing ones as > its instruction regarding how to transform the input to its output? > >> And second there is also a practical advantage, as the user can >> copy-paste trailers directly from other messages into the command line >> to pass them as arguments to "git interpret-trailers" without the need >> to replace the ':' with '='. Even if this command is not often used >> directly by users, it might simplify scripts using it. >> >> Third there is a technical advantage which is that the code that >> parses arguments from the command line can be the same as the code >> that parses trailers from the input message. > > I do not see these two as valid arguments to make the command line > more complex to the end users I don't think that it makes the command more complex to the end users. > ---who now need to know that only this > command treats its command line in a funny way, accepting a colon in > place of an equal sign. It accepts both. So if they think that it is like a regular command, which uses '=' for (key, value) pairs, it will work, and if they think it works on trailers, which use ':' for (key, value) pairs, it will also work. > A different way to sell a colon, e.g. > > Consider the instruction sed takes on its command line. > (e.g. "sed 's/frotz/nitfol/' form, you would always give it as the value of an '-e' option > (e.g. "sed -e 's/frotz/nitfol' be loose in limited occassions. "Key:value" is like that, and > in the most general form, it actually needs to be spelled as > "-e 'Key:value'". > > is possible, but I do not think it is a particularly good analogy, > because what you have as the alternative is "Key=value", and not > "-e 'Key:value'", or "--Key=value" (the last would probably be the > most natural way to express this). The analogy that I would use is rather that Perl lets people use 's:foo:bar:' as well as 's=foo=bar=' instead of 's/foo/bar/' if they prefer. -- 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 v9 11/11] Documentation: add documentation for 'git interpret-trailers'
Christian Couder writes: > On Sat, Apr 5, 2014 at 12:42 AM, Junio C Hamano wrote: >> Junio C Hamano writes: >> >>> Christian Couder writes: >>> "The following features are planned but not yet implemented: >>> - add more tests related to commands >>> - add examples in documentation >>> - integration with "git commit"" >> >> I was planning to merge the series to 'next', but perhaps we should >> wait at least for the first two items (I do not think the third one >> is necessary to block the series). > > I will send soon a new version of the series with more tests and fixes. > It will also contains a patch that adds an empty line before the > trailers in the output if there is not already one. Ah, yes, that one was mentioned in the reviews, I remember. > After that I plan to work on adding examples to the documentation. OK, thanks. > First accepting both ':' and '=' means one can see the "git > interpret-trailers" as acting on trailers only. Not just on trailers > from the intput message and option parameters from the command line. Sorry, you lost me. What does "acting on trailers only" really mean? Do you mean the command should/can be run without any command line options, pick up the existing "Signed-off-by:" and friends in its input and emit its output, somehow taking these existing ones as its instruction regarding how to transform the input to its output? > And second there is also a practical advantage, as the user can > copy-paste trailers directly from other messages into the command line > to pass them as arguments to "git interpret-trailers" without the need > to replace the ':' with '='. Even if this command is not often used > directly by users, it might simplify scripts using it. > > Third there is a technical advantage which is that the code that > parses arguments from the command line can be the same as the code > that parses trailers from the input message. I do not see these two as valid arguments to make the command line more complex to the end users---who now need to know that only this command treats its command line in a funny way, accepting a colon in place of an equal sign. A different way to sell a colon, e.g. Consider the instruction sed takes on its command line. (e.g. "sed 's/frotz/nitfol/' http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 11/11] Documentation: add documentation for 'git interpret-trailers'
On Sat, Apr 5, 2014 at 12:42 AM, Junio C Hamano wrote: > Junio C Hamano writes: > >> Christian Couder writes: >> "The following features are planned but not yet implemented: >> - add more tests related to commands >> - add examples in documentation >> - integration with "git commit"" > > I was planning to merge the series to 'next', but perhaps we should > wait at least for the first two items (I do not think the third one > is necessary to block the series). I will send soon a new version of the series with more tests and fixes. It will also contains a patch that adds an empty line before the trailers in the output if there is not already one. After that I plan to work on adding examples to the documentation. >>> Why support both '=' and ':'? Using just one would make it easier to >>> grep through scripts to see who is adding signoffs. >> >> That was already discussed previously. > > I do recall it was discussed previously, but given that a new reader > posed the same question, I am not sure if the end result in this > patch under discussion sufficiently answers the question in a > satisfactory way. > >> The reason is that people are used to "token=value" for command line >> arguments, but trailers appears in the result as "token: value", so it >> is better for the user if we support both. > > While I do understand the part before ", so" on the second line, I > do not see why that leads to the conclusion at all. > > Yes, because it is a well-established convention to separate option > name with its parameter with '=', accepting "--option=parameter" > makes sense. That may result in a string "Option: parameter" added > to the output from the command. I am not sure why that output has > anything to do with how the command line should be specified. First accepting both ':' and '=' means one can see the "git interpret-trailers" as acting on trailers only. Not just on trailers from the intput message and option parameters from the command line. But from trailers both from the input message and being passed as arguments. In my opinion it is good if it can be seen this way, as it may simplifies the user's mental model of how the command works. And second there is also a practical advantage, as the user can copy-paste trailers directly from other messages into the command line to pass them as arguments to "git interpret-trailers" without the need to replace the ':' with '='. Even if this command is not often used directly by users, it might simplify scripts using it. Third there is a technical advantage which is that the code that parses arguments from the command line can be the same as the code that parses trailers from the input message. 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 v9 11/11] Documentation: add documentation for 'git interpret-trailers'
Junio C Hamano writes: > Christian Couder writes: > ... >>> Why support both '=' and ':'? Using just one would make it easier to >>> grep through scripts to see who is adding signoffs. >> >> That was already discussed previously. > > I do recall it was discussed previously, but given that a new reader > posed the same question, I am not sure if the end result in this > patch under discussion sufficiently answers the question in a > satisfactory way. > > Thanks. Sorry, I sent out only a half-response before finishing. >> The reason is that people are used to "token=value" for command line >> arguments, but trailers appears in the result as "token: value", so it >> is better for the user if we support both. While I do understand the part before ", so" on the second line, I do not see why that leads to the conclusion at all. Yes, because it is a well-established convention to separate option name with its parameter with '=', accepting "--option=parameter" makes sense. That may result in a string "Option: parameter" added to the output from the command. I am not sure why that output has anything to do with how the command line should be specified. -- 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 v9 11/11] Documentation: add documentation for 'git interpret-trailers'
Christian Couder writes: > "The following features are planned but not yet implemented: > - add more tests related to commands > - add examples in documentation > - integration with "git commit"" I was planning to merge the series to 'next', but perhaps we should wait at least for the first two items (I do not think the third one is necessary to block the series). >>> +By default, a 'token=value' or 'token:value' argument will be added >>> +only if >> >> Why support both '=' and ':'? Using just one would make it easier to >> grep through scripts to see who is adding signoffs. > > That was already discussed previously. I do recall it was discussed previously, but given that a new reader posed the same question, I am not sure if the end result in this patch under discussion sufficiently answers the question in a satisfactory way. 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
Re: [PATCH v9 11/11] Documentation: add documentation for 'git interpret-trailers'
Hi, On Wed, Apr 2, 2014 at 2:39 AM, Jonathan Nieder wrote: > (culling cc list) > Hi, > > Christian Couder wrote: > >> [Subject: Documentation: add documentation for 'git interpret-trailers'] >> >> Signed-off-by: Christian Couder > > This should be squashed into the patch that introduces the > interpret-trailers command, IMHO (or if it should be reviewed > separately, it can be an earlier patch). That way, someone looking at > when the command was introduced and wanting to understand what it was > originally meant to do has the information close by. Well, the series is not very long, so this patch is quite close to the beginning anyway. > Thanks for picking up the 'git commit --fixes' topic and your steady > work improving the series. > > [...] >> --- /dev/null >> +++ b/Documentation/git-interpret-trailers.txt >> @@ -0,0 +1,123 @@ >> +git-interpret-trailers(1) >> += >> + >> +NAME >> + >> +git-interpret-trailers - help add stuctured information into commit messages >> + >> +SYNOPSIS >> + >> +[verse] >> +'git interpret-trailers' [--trim-empty] [([(=|:)])...] >> + >> +DESCRIPTION >> +--- >> +Help add RFC 822-like headers, called 'trailers', at the end of the >> +otherwise free-form part of a commit message. >> + >> +This command is a filter. It reads the standard input for a commit >> +message and applies the `token` arguments, if any, to this >> +message. The resulting message is emited on the standard output. > > Do you have an example? Does it work like this? > > $ git interpret-trailers 'signoff=Jonathan Nieder > ' < > foo bar baz qux > > EOF > foo bar baz qux > > Signed-off-by: Jonathan Nieder > $ Yeah, that's the idea. But you need to run something like: $ git config trailer.signoff.key "Signed-off-by:" to configure it properly first. By the way trying your example, I found that it is not currently adding an empty line before the s-o-b. I will have a look. > A short EXAMPLES section could help. Yeah, it is planned, but not yet implemented, as written in patch 0/11: "The following features are planned but not yet implemented: - add more tests related to commands - add examples in documentation - integration with "git commit"" > If I am understanding it correctly, would a name like 'git add-trailers' > work? It could work but it can modify, not just add trailers. > How do I read back the trailers later? Why do you want to read them back? Right now it should be used in hooks to modify commit messages. > [...] >> +By default, a 'token=value' or 'token:value' argument will be added >> +only if > > Why support both '=' and ':'? Using just one would make it easier to > grep through scripts to see who is adding signoffs. That was already discussed previously. The reason is that people are used to "token=value" for command line arguments, but trailers appears in the result as "token: value", so it is better for the user if we support both. 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 v9 11/11] Documentation: add documentation for 'git interpret-trailers'
(culling cc list) Hi, Christian Couder wrote: > [Subject: Documentation: add documentation for 'git interpret-trailers'] > > Signed-off-by: Christian Couder This should be squashed into the patch that introduces the interpret-trailers command, IMHO (or if it should be reviewed separately, it can be an earlier patch). That way, someone looking at when the command was introduced and wanting to understand what it was originally meant to do has the information close by. Thanks for picking up the 'git commit --fixes' topic and your steady work improving the series. [...] > --- /dev/null > +++ b/Documentation/git-interpret-trailers.txt > @@ -0,0 +1,123 @@ > +git-interpret-trailers(1) > += > + > +NAME > + > +git-interpret-trailers - help add stuctured information into commit messages > + > +SYNOPSIS > + > +[verse] > +'git interpret-trailers' [--trim-empty] [([(=|:)])...] > + > +DESCRIPTION > +--- > +Help add RFC 822-like headers, called 'trailers', at the end of the > +otherwise free-form part of a commit message. > + > +This command is a filter. It reads the standard input for a commit > +message and applies the `token` arguments, if any, to this > +message. The resulting message is emited on the standard output. Do you have an example? Does it work like this? $ git interpret-trailers 'signoff=Jonathan Nieder ' < foo bar baz qux > EOF foo bar baz qux Signed-off-by: Jonathan Nieder $ A short EXAMPLES section could help. If I am understanding it correctly, would a name like 'git add-trailers' work? How do I read back the trailers later? [...] > +By default, a 'token=value' or 'token:value' argument will be added > +only if Why support both '=' and ':'? Using just one would make it easier to grep through scripts to see who is adding signoffs. Hope that helps, Jonathan -- 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 v9 11/11] Documentation: add documentation for 'git interpret-trailers'
Signed-off-by: Christian Couder --- Documentation/git-interpret-trailers.txt | 123 +++ 1 file changed, 123 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..75ae386 --- /dev/null +++ b/Documentation/git-interpret-trailers.txt @@ -0,0 +1,123 @@ +git-interpret-trailers(1) += + +NAME + +git-interpret-trailers - help add stuctured information into commit messages + +SYNOPSIS + +[verse] +'git interpret-trailers' [--trim-empty] [([(=|:)])...] + +DESCRIPTION +--- +Help add RFC 822-like headers, called 'trailers', at the end of the +otherwise free-form part of a commit message. + +This command is a filter. It reads the standard input for a commit +message and applies 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 whitespace, 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 +new 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. + +OPTIONS +--- +--trim-empty:: + If the 'value' part of any trailer contains only whitespace, + the whole trailer will be removed from the resulting message. + +CONFIGURATION VARIABLES +--- + +trailer..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..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..ifexist:: + This option makes it possible to choose 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..ifmissing:: + This option makes it possible to choose 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..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 option is specified, it is like if a special 'token=value' +argument is added at the end of the command line, where 'value' will +be given by the standard output of the specified command. ++ +If the command contains the `$ARG` string, this string will be +replaced with the 'value' part of an existing trailer with the same +token, if any, before the command is launched. + +SEE ALSO + +linkgit:git-commit[1] +
