Re: More builtin git-am issues..

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]