Re: [PATCH v3 3/3] ref-filter: use pretty.c logic for trailers

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 8, 2021, 18:29 UTC
Message-ID: <xmqqv9b25s7f.fsf@gitster.c.googlers.com>
In-Reply-To: <CA+CkUQ_cdUmuP+_yUeCytn=6cc8SjMBE1aTLzWJL-U_V01uzog@mail.gmail.com>

Hariom verma <hariom18599@gmail.com> writes:

> I suspect that "fix" for "log --pretty" isn't going to work here.
>
> Even if we apply the same "log --pretty"'s fix here. I think we still
> end up having an empty blank line between each ref item.

After sleeping on it and seeing a result of an experiment like this one, I think that might be unavoidable.

    $ git for-each-ref \
	--format="One%0a%(trailers:key=Signed-off-by:)Two%0a" \
	refs/heads/js/range-diff-wo-dotdot
    One
    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
    Two
    $ exit

People who write such "Two" without prefixing it with a newline "%0a" themselves may view such a "fix" a regression.

It is sad that this %(trailers) itself is relatively a new thing, and I had thought that all the other ingredients are designed to strip the trailing newline, e.g. try this:

    $ git for-each-ref \
	--format="%(subject)%0a%(trailers:key=Signed-off-by:)" \
	refs/heads/js/range-diff-wo-dotdot
    range-diff(docs): explain how to specify commit ranges
    Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>

Notice that %(subject) is followed explicitly by %0a. I think %(author:date), etc. would do the same. But %(trailers) behave differently, and that is because it expects to be multi-line and perhaps to mimic %(body)? In any case, it may be too late to change its behaviour. At least I do not think of a good waoy to do so.

By the way, when merged to 'seen' (you can try the above that shows %(subject) followed by %(trailers) with the tip of 'seen'), it dies like this:

    $ git for-each-ref \
	--format="%(subject)%0a%(trailers:key=Signed-off-by:)" \
	refs/heads/js/range-diff-wo-dotdot
    free(): double free detected in tcache 2
    Aborted

There must be some interaction with another topic but I didn't dig deeper.

Thanks.