Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sep 6, 2021, 22:27 UTC
Message-ID: <87czplnxn3.fsf@evledraar.gmail.com>
In-Reply-To: <55293c43-811b-b030-9512-7525f5ebfd12@gmail.com>

On Mon, Sep 06 2021, Phillip Wood wrote:

Show 19 quoted lines

> Hi Ævar
>
> On 06/09/2021 08:05, Ævar Arnfjörð Bjarmason wrote:
>> Remove the git_pager() function last referenced by non-test code in
>> 49eb8d39c78 (Remove contrib/examples/*, 2018-03-25).
>> We can also remove the test for this added in 995bc22d7f8 (pager:
>> move
>> pager-specific setup into the build, 2016-08-04), the test that
>> actually matters is the one added in e54c1f2d253 (pager: set LV=-c
>> alongside LESS=FRSX, 2014-01-06) just above the removed test.
>> I.e. we don't care if the "LESS" and "LV" variables are set by
>> git-sh-setup anymore, no built-in uses them, we do care that pager.c
>> sets them, which we still test for.
>
> git_pager() might not be documented but I think it is useful for
> script authors and I wouldn't be surprised if someone out there is
> using it. The same goes for peel_committish(). It does not seem like a
> huge maintenance burden to keep and maybe document these two
> functions.

The git_pager() and peel_committish() seem to thoroughly be in the same camp as the now-removed git-parse-remote.sh (see a89a2fbfccd (parse-remote: remove this now-unused library, 2020-11-14)) and say its get_remote_merge_branch(). I.e. we carried it for a while, but the function was never publicly documented.

I think rather than document these it makes sense to just kick that maintenance burden over to whoever decided they'd rely on undocumented shellscript functions git was shipping.

In these cases they can rather easily use the documented GIT_PAGER environment variable directly, and their own invocation of "git rev-parse" for peel_committish().