On Sat, Sep 5, 2015 at 9:39 PM, Junio C Hamano <gits...@pobox.com> wrote: > Junio C Hamano <gits...@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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
