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

From: Hariom verma <hariom18599@gmail.com>
Date: Feb 4, 2021, 18:46 UTC
Message-ID: <CA+CkUQ_YDxF+fphzyQRD1OkFh7NGEmHUABvRiAjL-H52MHyH3Q@mail.gmail.com>
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:

Show 8 quoted lines

>
> 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.

Show 10 quoted lines

> 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.