Re: [PATCH] Utilize config variable pager.stash in stash list command

From: Jeff King <peff@peff.net>
Date: Aug 15, 2011, 23:47 UTC
Message-ID: <20110815234714.GB4699@sigill.intra.peff.net>
In-Reply-To: <4e47dcf9.55313988.bm000@wupperonline.de>

On Sun, Aug 14, 2011 at 04:31:49PM +0200, Ingo Brückl wrote:

Show 19 quoted lines

> Signed-off-by: Ingo Brückl <ib@wupperonline.de>
> ---
>  By now stash list ignores it.
> 
>  git-stash.sh |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index f4e6f05..7bb0856 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -264,7 +264,8 @@ have_stash () {
> 
>  list_stash () {
>  	have_stash || return 0
> -	git log --format="%gd: %gs" -g "$@" $ref_stash --
> +	test "$(git config --get pager.stash)" = "false" && no_pager=--no-pager
> +	git $no_pager log --format="%gd: %gs" -g "$@" $ref_stash --
>  }

It's not quite as simple as this these days. The pager.* variables can also point to a program to run as a pager for this specific command.

This stuff is supposed to be handled by the "git" wrapper itself, which will either run the pager (if the config is boolean true, or a specific command), or will set an environment variable to avoid running one for any subcommand (if it's boolean false).

However, we don't respect pager.* config for external commands there at all. I think this was due to some initialization-order bugs that made it hard for us to look at config before exec'ing external commands. But perhaps they are gone, as the patch below[1] seems to work OK for me.

---
 git.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git.c b/git.c
index 8828c18..47a6d3d 100644
--- a/git.c
+++ b/git.c
@@ -459,6 +459,8 @@ static void execv_dashed_external(const char **argv)
 	const char *tmp;
 	int status;
 
+	if (use_pager == -1)
+		use_pager = check_pager_config(argv[0]);
 	commit_pager_choice();
 
 	strbuf_addf(&cmd, "git-%s", argv[0]);

-Peff

[1] I posted this in a similar discussion several months ago:

    http://thread.gmane.org/gmane.comp.version-control.git/161756/focus=161771

I think what it really needs is more testing to see if looking at the
config then has any unintended side effects.