Johan Herland <johan@xxxxxxxxxxx> writes:
Thanks for looking this over. I am mostly in agreement and will
elide the parts I do not have much to add.
>> This command should help with RFC 822 style headers, called
>> "trailers", that are found at the end of commit messages.
>
> As has been asked earlier in this discussion, we should probably
> specify explicitly what we _mean_ with "RFC822-style"
> headers/footers/trailers, and exactly how closely we follow the actual
> RFC... E.g. do we make use of the linebreaking rules? encoding
> handling? etc... We may want to take a more relaxed approach (after
> all, we're not including a complete RFC822/RFC2822 implementation),
> but we should at least state so, and possibly how/why we do so.
Indeed; especially I do not think we would want to do the 2822
contination lines, or 2047 MIME quoting, ever.
>> For a long time, these trailers have become a de facto standard
>> way to add helpful information into commit messages.
"helpful" is not the key aspect of these footers. They are added at
the end to introduce some structure into a free-form part of the
commit objects.
>> The following features are implemented:
>> - the result is printed on stdout
>> - the [<token[=value]>...] arguments are interpreted
>> - a commit message passed using the "--infile=file" option is interpreted
>
> If the output is written to stdout, then why is not the input taken
> from stdin? Or vice versa: why not --outfile?
I'd say just make it a filter without --in/outfile ;-)
>> +{
>> + char *end = strchr(arg, '=');
>> + if (!end)
>> + end = strchr(arg, ':');
>
> So both '=' (preferred) and ':' are accepted as field/value
> separators. That's ok for the command-line, I believe.
Why?
Sometimes you have to be loose from the beginning _if_ some existing
uses and established conventions make it easier for the users, but
if you do not have to start from being loose, it is almost always a
mistake to do so. The above code just closed the door to use ":"
for some other useful purposes we may later discover, and will make
us regret for doing so.
> From the enum values, I assume that these declare the desired behavior
> when there are two (or more?) or the same footer (i.e. same "field
> name"). However, it's not (yet) obvious to me in which order they are
> processed. There are several opportunities for multiple instances of
> the same "field name":
>
> - Two (or more) occurences in the infile
> - Two (or more) occurences on the command line
> - One occurence in the infile, and one of the command line
Also, there is a distinction between fields with the same field name
appearing twice and fields with the same field name and the same
field value appearing twice. Some headers do want to have them, some
do not.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html