Re: More builtin git-am issues..

[Christian Couder <christian.couder@gmail.com>]

On Sat, Sep 5, 2015 at 9:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> To salvage "interpret-trailers" needs a lot more, as we are
>> realizing that the definition that led to its external design does
>> not match the way users use footers in the real world.  This affects
>> the internal data representation and the whole thing needs to be
>> rethought.
>
> Note that I am not saying that you personally did any bad job while
> working on the interpret-trailers topic.  We collectively designed
> its feature based on a much narrower definition of what the trailer
> block is than what is used in the real world in practice, so we do
> not have a good way to locate an existing entry that is not a (key,
> value), no matter what the syntax to denote which is key and which
> is value is, insert a new entry that is not a (key, value), or
> remove an existing entry that is not a (key, value), all of which
> will be necessary to mutate trailer blocks people use in the real
> life.
>
> I think a good way forward would be to go like this:
>
>  * a helper function that starts from a flat <buf, len> (or a
>    strbuf) and identifies the end of the body of the message,
>    i.e. find "^---$", "^Conflicts:$", etc. and skip blank lines
>    backwards.  That is what ignore_non_trailer() in commit.c does,
>    and that can be shared across everybody that mutates a log
>    message.
>
>  * a helper function that takes the result of the above as a flat
>    <buf, len> (or a strbuf) and identifies the beginning of a
>    trailer block.  That may be just the matter of scanning backwards
>    from the end of the trailer block ignore_non_trailer() identified
>    for the first blank line, as I agree with Linus's "So quite
>    frankly, at that point if git doesn't recognize it as a sign-off
>    block, I don't think it's a big deal" in the thread.
>
>    Not having that and not calling that function can reintroduce the
>    recent "interpret-trailers corner case" bug Matthieu brought up.
>
> With these, everybody except interpret-trailers that mutates a log
> message can add a new signoff consistently.  And then, building on
> these, "interpret-trailers" can be written like this:
>
>  (1) starting from a flat <buf, len> (or a strbuf), using the above
>      helpers, identify the parts of the log message that is the
>      trailer block (and you will know what is before and what is
>      after the trailer block).
>
>  (2) keep the part before the trailer block and the part after the
>      trailer block (this could be empty) in one strbuf each; we do
>      not want to mutate these parts, and it is pointless to split
>      them further into individual lines.
>
>  (3) express the lines in the trailer block in a richer data
>      structure that is easier to manipulate (i.e. reorder the lines,
>      insert, delete, etc.) and work on it.
>
>  (4) when manipulation of the trailer block is finished, reconstruct
>      the resulting message by concatenating the "before trailer"
>      part, "trailer" part, and "after trailer" part.
>
> As to the exact design of "a richer data structure" and the
> manipulation we may want on the trailer, I currently do not have a
> strong "it should be this way" opinion yet, but after looking at
> various examples Linus gave us in the discussion, my gut feelig is
> that it would be best to keep the operation simple and generic,
> e.g. "find a line that matches this regexp and replace it with this
> line", "insert this line at the end", "delete all lines that match
> this regexp", etc.

I will see what I can do about this.

Thanks,
Christian.