Junio C Hamano <[email protected]> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
