* Force usage of pager for diff, show, etc when piping to non-TTY
@ 2023-08-16 0:09 Patrick
2023-08-16 2:57 ` Jeff King
0 siblings, 1 reply; 7+ messages in thread
From: Patrick @ 2023-08-16 0:09 UTC (permalink / raw
To: git
Hi,
I noticed there is no option I can pass to git to use the pager set in
my gitconfig even when piping to non-TTY. Is there a workaround? If
not, may I request this as a new feature?
Use case: integrate tools like Delta or diff-so-fancy when building
wrappers around git commands. See
https://github.com/dandavison/delta/discussions/840 and
https://github.com/PatrickF1/fzf.fish/discussions/202 for examples.
Thanks,
Patrick
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Force usage of pager for diff, show, etc when piping to non-TTY
2023-08-16 0:09 Force usage of pager for diff, show, etc when piping to non-TTY Patrick
@ 2023-08-16 2:57 ` Jeff King
2023-08-16 16:30 ` Patrick
0 siblings, 1 reply; 7+ messages in thread
From: Jeff King @ 2023-08-16 2:57 UTC (permalink / raw
To: Patrick; +Cc: git
On Tue, Aug 15, 2023 at 05:09:13PM -0700, Patrick wrote:
> I noticed there is no option I can pass to git to use the pager set in
> my gitconfig even when piping to non-TTY. Is there a workaround? If
> not, may I request this as a new feature?
I don't think there is a workaround. We have "git --paginate", but it
really means "paginate this command using the default rules, even if it
is not a command that is usually paginated".
Looking at the code in setup_pager(), I think the check for "is stdout a
tty" is fed directly into the decision of whether to use a pager.
There's no way to override it within Git. You'd have to trick Git by
opening a pty in the calling program and pumping the data through it
(which is what our test suite does; see t/test-terminal.perl).
So I think it would need a new option. But...
> Use case: integrate tools like Delta or diff-so-fancy when building
> wrappers around git commands. See
> https://github.com/dandavison/delta/discussions/840 and
> https://github.com/PatrickF1/fzf.fish/discussions/202 for examples.
I'm not quite sure that's what you want. When a user configures a custom
pager using a prettifier tool like that, they usually further pipe the
output to a pager like "less". E.g., I have:
[pager]
log = diff-highlight | less
in my config. If you are trying to save output that looks like what the
user would see on their tty, you want the first half of that pipeline
(the diff-highlight here), but not the second (less). Of course it
mostly works because less is smart enough to behave like a noop "cat"
when stdout isn't a tty. So it might be OK in practice.
I think your script might be better off doing the piping itself. In
theory you could ask Git what the configured pager is and the run it
yourself, but:
1. You can use "git var GIT_PAGER" to get the default pager, but not
command-specific ones. So you'd have to check "git config
pager.log", etc, yourself, which means reimplementing some of Git's
logic.
2. You'd get a string like "diff-highlight | less", and then you'd
have to decide whether to parse off the "| less" part yourself,
which is obviously error-prone since the string can be an
arbitrarily complex shell expression.
When we were faced with a similar situation within Git, we ended up
adding a new config option: interactive.diffFilter. This is used by "add
-p", etc, to filter diffs that are shown to the user. It does mean the
user has to repeat themselves (I have to set it to "diff-highlight"
separately from my settings for pager.log, pager.diff, etc). But it's
unambiguous, and it gives the user the flexibility of configuring
various outputs differently if they choose.
So depending on what your script does, you could use a similar config
option. Or even just use interactive.diffFilter if your use case is
conceptually the same.
-Peff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Force usage of pager for diff, show, etc when piping to non-TTY
2023-08-16 2:57 ` Jeff King
@ 2023-08-16 16:30 ` Patrick
2023-08-17 5:44 ` Jeff King
0 siblings, 1 reply; 7+ messages in thread
From: Patrick @ 2023-08-16 16:30 UTC (permalink / raw
To: Jeff King; +Cc: git
Hi Jeff, thank you for your expert and thorough response!
You make a good point that users usually pipe the output of a prettier
tool to a pager themselves. In the case of delta, delta can be
configured to selectively pipe to a pager itself, so delta's docs
doesn't tell users to pipe: `core.pager = delta` (source:
https://dandavison.github.io/delta/configuration.html)
However, for diff-so-fancy, it might not work cleanly because users
are told to explicitly pipe it to a pager. `core.pager "diff-so-fancy
| less --tabs=4 -RFX"` (source:
https://github.com/so-fancy/diff-so-fancy#with-git).
I haven't looked at other tools but it's clear this approach is not
guaranteed to work for all configurations so I should look into
something else.
Jeff, would you be so kind as to elaborate more on the
interactive.diffFilter approach? My understanding is that
interactive.diffFilter is only used for git add -p or git reset -p.
However, the limitation for my use case is I need to use the pager
for git log and git show so that won't work. So then, you are
suggesting that I ask my users to opt in by setting an arbitrary git
config like fzf.pager and then read out the pager from that git var?
On Tue, Aug 15, 2023 at 7:57 PM Jeff King <peff@peff.net> wrote:
>
> On Tue, Aug 15, 2023 at 05:09:13PM -0700, Patrick wrote:
>
> > I noticed there is no option I can pass to git to use the pager set in
> > my gitconfig even when piping to non-TTY. Is there a workaround? If
> > not, may I request this as a new feature?
>
> I don't think there is a workaround. We have "git --paginate", but it
> really means "paginate this command using the default rules, even if it
> is not a command that is usually paginated".
>
> Looking at the code in setup_pager(), I think the check for "is stdout a
> tty" is fed directly into the decision of whether to use a pager.
> There's no way to override it within Git. You'd have to trick Git by
> opening a pty in the calling program and pumping the data through it
> (which is what our test suite does; see t/test-terminal.perl).
>
> So I think it would need a new option. But...
>
> > Use case: integrate tools like Delta or diff-so-fancy when building
> > wrappers around git commands. See
> > https://github.com/dandavison/delta/discussions/840 and
> > https://github.com/PatrickF1/fzf.fish/discussions/202 for examples.
>
> I'm not quite sure that's what you want. When a user configures a custom
> pager using a prettifier tool like that, they usually further pipe the
> output to a pager like "less". E.g., I have:
>
> [pager]
> log = diff-highlight | less
>
> in my config. If you are trying to save output that looks like what the
> user would see on their tty, you want the first half of that pipeline
> (the diff-highlight here), but not the second (less). Of course it
> mostly works because less is smart enough to behave like a noop "cat"
> when stdout isn't a tty. So it might be OK in practice.
>
> I think your script might be better off doing the piping itself. In
> theory you could ask Git what the configured pager is and the run it
> yourself, but:
>
> 1. You can use "git var GIT_PAGER" to get the default pager, but not
> command-specific ones. So you'd have to check "git config
> pager.log", etc, yourself, which means reimplementing some of Git's
> logic.
>
> 2. You'd get a string like "diff-highlight | less", and then you'd
> have to decide whether to parse off the "| less" part yourself,
> which is obviously error-prone since the string can be an
> arbitrarily complex shell expression.
>
> When we were faced with a similar situation within Git, we ended up
> adding a new config option: interactive.diffFilter. This is used by "add
> -p", etc, to filter diffs that are shown to the user. It does mean the
> user has to repeat themselves (I have to set it to "diff-highlight"
> separately from my settings for pager.log, pager.diff, etc). But it's
> unambiguous, and it gives the user the flexibility of configuring
> various outputs differently if they choose.
>
> So depending on what your script does, you could use a similar config
> option. Or even just use interactive.diffFilter if your use case is
> conceptually the same.
>
> -Peff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Force usage of pager for diff, show, etc when piping to non-TTY
2023-08-16 16:30 ` Patrick
@ 2023-08-17 5:44 ` Jeff King
2023-08-17 17:06 ` Patrick
0 siblings, 1 reply; 7+ messages in thread
From: Jeff King @ 2023-08-17 5:44 UTC (permalink / raw
To: Patrick; +Cc: git
On Wed, Aug 16, 2023 at 09:30:38AM -0700, Patrick wrote:
> Jeff, would you be so kind as to elaborate more on the
> interactive.diffFilter approach? My understanding is that
> interactive.diffFilter is only used for git add -p or git reset -p.
> However, the limitation for my use case is I need to use the pager
> for git log and git show so that won't work. So then, you are
> suggesting that I ask my users to opt in by setting an arbitrary git
> config like fzf.pager and then read out the pager from that git var?
Yes, they'd have to set a new config variable. Though if you are really
just filtering diffs and the semantics would be the same as
interactive.diffFilter, I would probably just use that. You could also
introduce a new foo.diffFilter option, and if unset have it default to
the value of interactive.diffFilter. That provides flexibility without
forcing users to repeat themselves.
That said, I would not be surprised if many users who set pager.log do
not even know about interactive.diffFilter. It's a bit more obscure, and
came later.
If you do want to follow that approach, the simplest example is probably
just to see how it was added to the perl code in 01143847db
(add--interactive: allow custom diff highlighting programs, 2016-02-27):
https://github.com/git/git/commit/01143847dbf4fbf27268650f3ace16eac03b3130
In shell it might look something like:
filter=$(git config --get interactive.difffilter)
if test -n "$filter"; then
git show ... | eval "$filter"
else
git show ...
fi
-Peff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Force usage of pager for diff, show, etc when piping to non-TTY
2023-08-17 5:44 ` Jeff King
@ 2023-08-17 17:06 ` Patrick
2023-08-17 19:47 ` Jeff King
0 siblings, 1 reply; 7+ messages in thread
From: Patrick @ 2023-08-17 17:06 UTC (permalink / raw
To: Jeff King; +Cc: git
Hi Jeff, I think I finally get it now. Thank you for so patiently
explaining this to me in different ways. I'm sorry I was slow to
understand heh.
Just to be really clear, the filter in `interactive.diffFilter` is
meant as a filter for transforming (think a photo filter), as opposed
to a filter that removes elements, correct? I think that's what I got
tripped up on when you explained the first two times.
On Wed, Aug 16, 2023 at 10:44 PM Jeff King <peff@peff.net> wrote:
>
> On Wed, Aug 16, 2023 at 09:30:38AM -0700, Patrick wrote:
>
> > Jeff, would you be so kind as to elaborate more on the
> > interactive.diffFilter approach? My understanding is that
> > interactive.diffFilter is only used for git add -p or git reset -p.
> > However, the limitation for my use case is I need to use the pager
> > for git log and git show so that won't work. So then, you are
> > suggesting that I ask my users to opt in by setting an arbitrary git
> > config like fzf.pager and then read out the pager from that git var?
>
> Yes, they'd have to set a new config variable. Though if you are really
> just filtering diffs and the semantics would be the same as
> interactive.diffFilter, I would probably just use that. You could also
> introduce a new foo.diffFilter option, and if unset have it default to
> the value of interactive.diffFilter. That provides flexibility without
> forcing users to repeat themselves.
>
> That said, I would not be surprised if many users who set pager.log do
> not even know about interactive.diffFilter. It's a bit more obscure, and
> came later.
>
> If you do want to follow that approach, the simplest example is probably
> just to see how it was added to the perl code in 01143847db
> (add--interactive: allow custom diff highlighting programs, 2016-02-27):
>
> https://github.com/git/git/commit/01143847dbf4fbf27268650f3ace16eac03b3130
>
> In shell it might look something like:
>
> filter=$(git config --get interactive.difffilter)
> if test -n "$filter"; then
> git show ... | eval "$filter"
> else
> git show ...
> fi
>
> -Peff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Force usage of pager for diff, show, etc when piping to non-TTY
2023-08-17 17:06 ` Patrick
@ 2023-08-17 19:47 ` Jeff King
2023-08-17 20:03 ` Patrick
0 siblings, 1 reply; 7+ messages in thread
From: Jeff King @ 2023-08-17 19:47 UTC (permalink / raw
To: Patrick; +Cc: git
On Thu, Aug 17, 2023 at 10:06:58AM -0700, Patrick wrote:
> Just to be really clear, the filter in `interactive.diffFilter` is
> meant as a filter for transforming (think a photo filter), as opposed
> to a filter that removes elements, correct? I think that's what I got
> tripped up on when you explained the first two times.
Yes, exactly. In retrospect, the name is a little ambiguous. :)
Glad you've figured it out.
-Peff
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: Force usage of pager for diff, show, etc when piping to non-TTY
2023-08-17 19:47 ` Jeff King
@ 2023-08-17 20:03 ` Patrick
0 siblings, 0 replies; 7+ messages in thread
From: Patrick @ 2023-08-17 20:03 UTC (permalink / raw
To: Jeff King; +Cc: git
Thank you for clarifying, Jeff. And thank you to all the git
contributors who not only maintain git for all the developers
everywhere, but also provide first class support :)
On Thu, Aug 17, 2023 at 12:47 PM Jeff King <peff@peff.net> wrote:
>
> On Thu, Aug 17, 2023 at 10:06:58AM -0700, Patrick wrote:
>
> > Just to be really clear, the filter in `interactive.diffFilter` is
> > meant as a filter for transforming (think a photo filter), as opposed
> > to a filter that removes elements, correct? I think that's what I got
> > tripped up on when you explained the first two times.
>
> Yes, exactly. In retrospect, the name is a little ambiguous. :)
> Glad you've figured it out.
>
> -Peff
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-08-17 20:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-16 0:09 Force usage of pager for diff, show, etc when piping to non-TTY Patrick
2023-08-16 2:57 ` Jeff King
2023-08-16 16:30 ` Patrick
2023-08-17 5:44 ` Jeff King
2023-08-17 17:06 ` Patrick
2023-08-17 19:47 ` Jeff King
2023-08-17 20:03 ` Patrick
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).