From: "Ingo Brückl" <ib@wupperonline•de>
To: git@vger•kernel.org
Subject: Re: [PATCH] Utilize config variable pager.stash in stash list command
Date: Tue, 16 Aug 2011 12:10:45 +0200 [thread overview]
Message-ID: <4e4a4743.4e230d8a.bm000@wupperonline.de> (raw)
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:
>> 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
> +++ 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
next prev parent reply other threads:[~2011-08-16 10:32 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-14 14:31 [PATCH] Utilize config variable pager.stash in stash list command Ingo Brückl
2011-08-15 23:47 ` Jeff King
2011-08-16 10:10 ` Ingo Brückl [this message]
2011-08-16 11:24 ` Ingo Brückl
2011-08-16 22:56 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4e4a4743.4e230d8a.bm000@wupperonline.de \
--to=ib@wupperonline$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox