From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 06 Sep 2021 09:49:36 GMT
Subject: Re: [PATCH v2 1/7] git-sh-setup: remove unused git_pager() function
Message-ID: <55293c43-811b-b030-9512-7525f5ebfd12@gmail.com>
In-Reply-To: <patch-v2-1.7-8eb1dfbff5d-20210906T070201Z-avarab@gmail.com>

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.

Best Wishes

Phillip

> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>   git-sh-setup.sh  | 16 ----------------
>   t/t7006-pager.sh | 13 -------------
>   2 files changed, 29 deletions(-)
> 
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 10d97641856..7ee4b0edff5 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -157,22 +157,6 @@ git_editor() {
>   	eval "$GIT_EDITOR" '"$@"'
>   }
>   
> -git_pager() {
> -	if test -t 1
> -	then
> -		GIT_PAGER=$(git var GIT_PAGER)
> -	else
> -		GIT_PAGER=cat
> -	fi
> -	for vardef in @@PAGER_ENV@@
> -	do
> -		var=${vardef%%=*}
> -		eval ": \"\${$vardef}\" && export $var"
> -	done
> -
> -	eval "$GIT_PAGER" '"$@"'
> -}
> -
>   sane_grep () {
>   	GREP_OPTIONS= LC_ALL=C grep @@SANE_TEXT_GREP@@ "$@"
>   }
> diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
> index 0e7cf75435e..08f712a4497 100755
> --- a/t/t7006-pager.sh
> +++ b/t/t7006-pager.sh
> @@ -49,19 +49,6 @@ test_expect_success TTY 'LESS and LV envvars are set for pagination' '
>   	grep ^LV= pager-env.out
>   '
>   
> -test_expect_success !MINGW,TTY 'LESS and LV envvars set by git-sh-setup' '
> -	(
> -		sane_unset LESS LV &&
> -		PAGER="env >pager-env.out; wc" &&
> -		export PAGER &&
> -		PATH="$(git --exec-path):$PATH" &&
> -		export PATH &&
> -		test_terminal sh -c ". git-sh-setup && git_pager"
> -	) &&
> -	grep ^LESS= pager-env.out &&
> -	grep ^LV= pager-env.out
> -'
> -
>   test_expect_success TTY 'some commands do not use a pager' '
>   	rm -f paginated.out &&
>   	test_terminal git rev-list HEAD &&
>