Re: [PATCH v13] [GSOC] commit: add --trailer option

It probably would have been better to do so before the feature got
unleased to the public, but doing such a change retroactively would
introduce regression for those who were using ARG that happens to be
safe from shell quoting rules.

For example, if the trailer.*.command were

        echo '$ARG'

and the argument 'h e l l o' were to be given to it, then the
current code would have textually expanded $ARG with the argument
and caused

        echo 'h e l l o'

to run, which would have been "fine" [*1*].

But exporting the environment ARG would "break" such a setting that
has been "working perfectly well" for the user.  Because of the
single-quotes around $ARG, the command now will give literal four
letter string $ARG and not 'h e l l o'.

We should think such potential ramifications of changing it (and
also not changing it) through before deciding what to do about it.

Although I have a feeling that not many people would miss '$ARG'
inside a pair of single-quotes to be replaced textually and it would
be OK to make a backward incompatible bugfix, the safer and better
way is not all that difficult, so I am inclined to suggest going the
usual "deprecate and replace and then later remove" dance.

I think what you mean is that my patch breaks the principle of
"forward compatibility", which may make it impossible for users who
previously worked with `'$ARG'`.

The normal sequence of replacing a "sort of works but not
recommended" feature with a "better and safer, but can break a
setting that has been 'working'" feature is:

 - Announce deprecation of trailer.x.command and add and advertise a
   similar traier.x.cmd that (1) exports environment variable ARG,
   or (2) passes the argument as a positional parameter [*], as a
   replacement.  Explain the reason for deprecation (i.e. unsafe
   substitution that works only once).  When .cmd exists, .command
   is ignored for the corresponding trailer.x

As your example below:

cmd = test -n "$1" && git log --author="$1" -1 --format='%aN <%aE>'

I would like to use this way.

It may be a bit cumbersome to do "deprecate and replace and
then later remove", Should documents tell users that the old method
"trailer.command" has been replaced by "trialer.cmd"? Or tell users that
"trailer.cmd" is a new feature?

Keep these doubts, I will try go to code it.

 - Wait for a few releases and then remove trailer.x.command.

and that is the safest way to fix this "bug".


[Footnotes]

*1* If the argument were

        ';rm -rf .;'

    then it wouldn't have been fine, though, and that is how the
    current code solicited "Huh?"  reaction out of me.

A little scary. :-)

*2* If we passed the argument as a positional parameter, the example
    you gave in the quoted part of the message would become
    something like this:

      [trailer "sign"]
        cmd = test -n "$1" && git log --author="$1" -1 --format='%aN <%aE>'