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

From: Hariom verma <hariom18599@gmail.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Hariom Verma via GitGitGadget <gitgitgadget@gmail.com>,
	git <git@vger.kernel.org>,
	Christian Couder <christian.couder@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 3/3] ref-filter: use pretty.c logic for trailers
Date: Fri, 5 Feb 2021 00:16:41 +0530	[thread overview]
Message-ID: <CA+CkUQ_YDxF+fphzyQRD1OkFh7NGEmHUABvRiAjL-H52MHyH3Q@mail.gmail.com> (raw)
In-Reply-To: <875z3ep30j.fsf@evledraar.gmail.com>

Hi,

On Sun, Jan 31, 2021 at 2:15 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> Given that the goal of this series is to unify this parsing logic
> between log/for-each-ref, why do we need to then copy/paste the exact
> same docs we have in pretty-formats.txt?
>
> At the very least we should move this to pretty-formats-trailers.txt or
> something, and just include it in both places, or better yet just refer
> to the relevan parts of "git log"'s man page, no?

Ok. I will refer to the trailers part of "pretty-formats"'s man page
in "git-for-each-ref"'s man page.

> And similarly, here we have now mostly duplicated tests for this between
> here and t/t4205-log-pretty-formats.sh.
>
> I think the right thing to do is to start by moving the tests that are
> now in t/t4205-log-pretty-formats.sh relevant to this formatting into
> its own file or something.
>
> Then instead of duplicating the tests here, just prepare them to be
> changed so that we can add both "git log" and a "git for-each-ref"
> invocation to some for-loop, so we'll test both.

With this unified trailer logic, "git log" and "git for-each-ref"
still behave differently.
For e.g.: "git log" does nothing for unknown/incorrect trailer option,
whereas "git for-each-ref" stops.

Even if we move trailer related tests for both into a new file, I
guess we still need to test trailers for both "git log" and "git
for-each-ref" separately?

Thanks,
Hariom.