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

From: Ingo Brückl <ib@wupperonline.de>
Date: Aug 16, 2011, 10:10 UTC
Message-ID: <4e4a4743.4e230d8a.bm000@wupperonline.de>
In-Reply-To: <20110815234714.GB4699@sigill.intra.peff.net>

Jeff King wrote on Mon, 15 Aug 2011 16:47:14 -0700:

> 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(-)

Show 12 quoted lines

> diff --git a/git.c b/git.c
> index 8828c18..47a6d3d 100644
> +++ 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

Actually, I only wanted to change the stash list behavior (but better should have used $(git config --get pager.stash.list) for that). Unfortunately, it is impossible then to force the pager with --paginate again.

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

Yours surely is a far better approach, although it only can handle the main command (stash), not the sub-command (list), but this is totally in accordance with everything else in git.

With "pager.stash false" (which would then require --paginate for a lot of stash commands), I found that a paginated output of 'git -p stash show -p' loses the diff colors, but that seems unrelated to your patch. It still is strange though.

Ingo