(no subject)

[Avishay Matayev]

Hi there!

Fugitive[1] is a vim plugin that wraps git and many of its commands
into the editor in a really awesome way, I won't meddle into it too
much as you can read about it in its README, but as you understand, it
uses git, a lot.

Some git commands use a pager, which is usually a program that needs a
pty to function properly (`less`, for example). Fugitive can't really
use a pty for the pager as vim runs its subprocesses without a pty.
Therefore Fugitive just creates its own pager (which is a simple
window in vim) and pastes the git command output there.

The only problem left is that Fugitive can't reliably know when git
decides to use the pager, for example `git reflog show` does raise the
pager while `git reflog expire` does not. Fugitive currently maintains
an (very possibly) incomplete list of commands that need a pager but
maintaining it manually isn't ideal.

I started discussing this on an issue in Fugitive's github page[2] and
Tim Pope (the creator and maintainer of Fugitive, thank you!)
explained that `git` doesn't use a pager if there is no pty so it's
impossible to override its behavior.

We had some ideas how to make this feasible (as you can read on the
thread) but for brevity's sake I'll present the best (IMO) idea:
Essentially, at `pager.c`, don't short-circuit in `git_pager` (or
`setup_pager`?) due to pty absence if a new environment variable is
present, perhaps something like `GIT_PAGER_FORCE` which will override
the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to
apply custom logic through to pager to know if one exists and present
the window in vim.

I will appreciate any written thoughts on the matter, thank you :)

P.S. I am a complete newbie in regards to mailing lists etiquette,
pardon me if I've done anything incorrect
P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all
changes to `pager.c`, sorry if that was wrong of me (You probably got
this mail twice because of a misconfiguration, oops)


1. https://github.com/tpope/vim-fugitive
2. https://github.com/tpope/vim-fugitive/issues/1772

(no subject)

[Felipe Contreras]

Avishay Matayev wrote:

> Some git commands use a pager, which is usually a program that needs a
> pty to function properly (`less`, for example). Fugitive can't really
> use a pty for the pager as vim runs its subprocesses without a pty.
> Therefore Fugitive just creates its own pager (which is a simple
> window in vim) and pastes the git command output there.

That's not necessarily true; vim uses a bunch of convoluted logic to
sometimes use a pty, but that depends on the mode used (graphical vs.
console), and a bunch of other things.

Even more complicated when you throw nvim into the mix.

But yeah, for the sake of simplication let's say that's true.

> I started discussing this on an issue in Fugitive's github page[2] and
> Tim Pope (the creator and maintainer of Fugitive, thank you!)
> explained that `git` doesn't use a pager if there is no pty so it's
> impossible to override its behavior.

That is true.

> We had some ideas how to make this feasible (as you can read on the
> thread) but for brevity's sake I'll present the best (IMO) idea:
> Essentially, at `pager.c`, don't short-circuit in `git_pager` (or
> `setup_pager`?) due to pty absence if a new environment variable is
> present, perhaps something like `GIT_PAGER_FORCE` which will override
> the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to
> apply custom logic through to pager to know if one exists and present
> the window in vim.

Seems like a completely sensible request to me, except I would do it a
slightly different place:

--- a/pager.c
+++ b/pager.c
@@ -101,7 +101,7 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)
 
 void setup_pager(void)
 {
-       const char *pager = git_pager(isatty(1));
+       const char *pager = git_pager(git_env_bool("GIT_PAGER_FORCE", isatty(1)));
 
        if (!pager)
                return;

I'm Cc'ing Jonathan Nieder who seems to have toched these lines.

Cheers.

-- 
Felipe Contreras

Why empty subject? (was Re: )

[Bagas Sanjaya]

Hi,

On 23/06/21 05.40, Avishay Matayev wrote:
> P.S. I am a complete newbie in regards to mailing lists etiquette,
> pardon me if I've done anything incorrect
> P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all
> changes to `pager.c`, sorry if that was wrong of me (You probably got
> this mail twice because of a misconfiguration, oops)

Why did you send the thread with empty subject?

-- 
An old man doll... just what I always wanted! - Clara

Re: Why empty subject? (was Re: )

[Avishay Matayev]

On Wed, 23 Jun 2021, 11:13 am Bagas Sanjaya, <bagasdotme@gmail.com> wrote:
>
> Hi,
>
> On 23/06/21 05.40, Avishay Matayev wrote:
> > P.S. I am a complete newbie in regards to mailing lists etiquette,
> > pardon me if I've done anything incorrect
> > P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all
> > changes to `pager.c`, sorry if that was wrong of me (You probably got
> > this mail twice because of a misconfiguration, oops)
>
> Why did you send the thread with empty subject?
>
> --
> An old man doll... just what I always wanted! - Clara

Oh, my bad.

It was "Forcing git to use a pager without a tty" and I sent it with
my initial mail that got rejected because I didn't send it as plain
text. I copied the mail and fixed it but forgot to reuse the subject,
oops!

I'll change the subject in the other reply, sorry about that.

Avishay

Re: Forcing git to use a pager without a tty

[Avishay Matayev]

On Wed, 23 Jun 2021 at 04:57, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
>
> Avishay Matayev wrote:
>
> > Some git commands use a pager, which is usually a program that needs a
> > pty to function properly (`less`, for example). Fugitive can't really
> > use a pty for the pager as vim runs its subprocesses without a pty.
> > Therefore Fugitive just creates its own pager (which is a simple
> > window in vim) and pastes the git command output there.
>
> That's not necessarily true; vim uses a bunch of convoluted logic to
> sometimes use a pty, but that depends on the mode used (graphical vs.
> console), and a bunch of other things.
>
> Even more complicated when you throw nvim into the mix.
>
> But yeah, for the sake of simplication let's say that's true.

True, I did oversimplify.

> > We had some ideas how to make this feasible (as you can read on the
> > thread) but for brevity's sake I'll present the best (IMO) idea:
> > Essentially, at `pager.c`, don't short-circuit in `git_pager` (or
> > `setup_pager`?) due to pty absence if a new environment variable is
> > present, perhaps something like `GIT_PAGER_FORCE` which will override
> > the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to
> > apply custom logic through to pager to know if one exists and present
> > the window in vim.
>
> Seems like a completely sensible request to me, except I would do it a
> slightly different place:
>
> --- a/pager.c
> +++ b/pager.c
> @@ -101,7 +101,7 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)
>
>  void setup_pager(void)
>  {
> -       const char *pager = git_pager(isatty(1));
> +       const char *pager = git_pager(git_env_bool("GIT_PAGER_FORCE", isatty(1)));
>
>         if (!pager)
>                 return;
>
> I'm Cc'ing Jonathan Nieder who seems to have toched these lines.
>
> Cheers.
>
> --
> Felipe Contreras

Well I'm glad to hear you find it sensible, though your patch suggests
that `GIT_PAGER_FORCE` will be a boolean, while I expect it to be an
actual pager, which would make more sense IMO.

Avishay

Re: Forcing git to use a pager without a tty

[Phillip Wood]

Hi Avishay

On 22/06/2021 23:40, Avishay Matayev wrote:
> Hi there!
> 
> Fugitive[1] is a vim plugin that wraps git and many of its commands
> into the editor in a really awesome way, I won't meddle into it too
> much as you can read about it in its README, but as you understand, it
> uses git, a lot.
> 
> Some git commands use a pager, which is usually a program that needs a
> pty to function properly (`less`, for example). Fugitive can't really
> use a pty for the pager as vim runs its subprocesses without a pty.
> Therefore Fugitive just creates its own pager (which is a simple
> window in vim) and pastes the git command output there.

If I understand correctly fugitive is reading the output of the git 
command over a pipe and putting it into a vim buffer?

> The only problem left is that Fugitive can't reliably know when git
> decides to use the pager, for example `git reflog show` does raise the
> pager while `git reflog expire` does not. Fugitive currently maintains
> an (very possibly) incomplete list of commands that need a pager but
> maintaining it manually isn't ideal.

I don't understand, if as you say above there isn't a pty then git wont 
use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem 
to, so I'm not sure what you mean by 'Fugitive can't reliably know when 
git decides to use the pager'

> I started discussing this on an issue in Fugitive's github page[2] and
> Tim Pope (the creator and maintainer of Fugitive, thank you!)
> explained that `git` doesn't use a pager if there is no pty so it's
> impossible to override its behavior.
> 
> We had some ideas how to make this feasible (as you can read on the
> thread) but for brevity's sake I'll present the best (IMO) idea:
> Essentially, at `pager.c`, don't short-circuit in `git_pager` (or
> `setup_pager`?) due to pty absence if a new environment variable is
> present, perhaps something like `GIT_PAGER_FORCE` which will override
> the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to
> apply custom logic through to pager to know if one exists and present
> the window in vim.

I note that the latest comment [1] on the github issue talks about a 
different solution that would cause all git commands to behave as if 
there was a pty present.

[1] https://github.com/tpope/vim-fugitive/issues/1772#issuecomment-866401942

Best Wishes

Phillip

> I will appreciate any written thoughts on the matter, thank you :)
> 
> P.S. I am a complete newbie in regards to mailing lists etiquette,
> pardon me if I've done anything incorrect
> P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all
> changes to `pager.c`, sorry if that was wrong of me (You probably got
> this mail twice because of a misconfiguration, oops)
> 
> 
> 1. https://github.com/tpope/vim-fugitive
> 2. https://github.com/tpope/vim-fugitive/issues/1772
> 

Re: Forcing git to use a pager without a tty

[Phillip Wood]

On 23/06/2021 10:05, Phillip Wood wrote:
> Hi Avishay
> 
> On 22/06/2021 23:40, Avishay Matayev wrote:
>> Hi there!
>>
>> Fugitive[1] is a vim plugin that wraps git and many of its commands
>> into the editor in a really awesome way, I won't meddle into it too
>> much as you can read about it in its README, but as you understand, it
>> uses git, a lot.
>>
>> Some git commands use a pager, which is usually a program that needs a
>> pty to function properly (`less`, for example). Fugitive can't really
>> use a pty for the pager as vim runs its subprocesses without a pty.
>> Therefore Fugitive just creates its own pager (which is a simple
>> window in vim) and pastes the git command output there.
> 
> If I understand correctly fugitive is reading the output of the git 
> command over a pipe and putting it into a vim buffer?
> 
>> The only problem left is that Fugitive can't reliably know when git
>> decides to use the pager, for example `git reflog show` does raise the
>> pager while `git reflog expire` does not. Fugitive currently maintains
>> an (very possibly) incomplete list of commands that need a pager but
>> maintaining it manually isn't ideal.
> 
> I don't understand, if as you say above there isn't a pty then git wont 
> use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem 
> to,

Sorry that is confused. GIT_PAGER_IN_USE only causes git to act as if a 
pager is being used (for example so it colors things as if it was 
outputting to a tty), it does not invoke the pager

  so I'm not sure what you mean by 'Fugitive can't reliably know when
> git decides to use the pager'

I'm still confused by this - if there is no tty git wont use a pager.

Best Wishes

Phillip

>> I started discussing this on an issue in Fugitive's github page[2] and
>> Tim Pope (the creator and maintainer of Fugitive, thank you!)
>> explained that `git` doesn't use a pager if there is no pty so it's
>> impossible to override its behavior.
>>
>> We had some ideas how to make this feasible (as you can read on the
>> thread) but for brevity's sake I'll present the best (IMO) idea:
>> Essentially, at `pager.c`, don't short-circuit in `git_pager` (or
>> `setup_pager`?) due to pty absence if a new environment variable is
>> present, perhaps something like `GIT_PAGER_FORCE` which will override
>> the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to
>> apply custom logic through to pager to know if one exists and present
>> the window in vim.
> 
> I note that the latest comment [1] on the github issue talks about a 
> different solution that would cause all git commands to behave as if 
> there was a pty present.
> 
> [1] 
> https://github.com/tpope/vim-fugitive/issues/1772#issuecomment-866401942
> 
> Best Wishes
> 
> Phillip
> 
>> I will appreciate any written thoughts on the matter, thank you :)
>>
>> P.S. I am a complete newbie in regards to mailing lists etiquette,
>> pardon me if I've done anything incorrect
>> P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all
>> changes to `pager.c`, sorry if that was wrong of me (You probably got
>> this mail twice because of a misconfiguration, oops)
>>
>>
>> 1. https://github.com/tpope/vim-fugitive
>> 2. https://github.com/tpope/vim-fugitive/issues/1772
>>

Re: Forcing git to use a pager without a tty

[Avishay Matayev]

Hello Phillip!


On Wed, 23 Jun 2021 at 12:29, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 23/06/2021 10:05, Phillip Wood wrote:
> > Hi Avishay
> >
> > On 22/06/2021 23:40, Avishay Matayev wrote:
> >> Hi there!
> >>
> >> Fugitive[1] is a vim plugin that wraps git and many of its commands
> >> into the editor in a really awesome way, I won't meddle into it too
> >> much as you can read about it in its README, but as you understand, it
> >> uses git, a lot.
> >>
> >> Some git commands use a pager, which is usually a program that needs a
> >> pty to function properly (`less`, for example). Fugitive can't really
> >> use a pty for the pager as vim runs its subprocesses without a pty.
> >> Therefore Fugitive just creates its own pager (which is a simple
> >> window in vim) and pastes the git command output there.
> >
> > If I understand correctly fugitive is reading the output of the git
> > command over a pipe and putting it into a vim buffer?
> >
AFAIK, no. It could read the output through vim's jobstart API (that calls a
callback when there's activity on stdout for example) or by
redirecting the output
to a temporary file and reading it.

> >> The only problem left is that Fugitive can't reliably know when git
> >> decides to use the pager, for example `git reflog show` does raise the
> >> pager while `git reflog expire` does not. Fugitive currently maintains
> >> an (very possibly) incomplete list of commands that need a pager but
> >> maintaining it manually isn't ideal.
> >
> > I don't understand, if as you say above there isn't a pty then git wont
> > use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem
> > to,
>
> Sorry that is confused. GIT_PAGER_IN_USE only causes git to act as if a
> pager is being used (for example so it colors things as if it was
> outputting to a tty), it does not invoke the pager
>
>   so I'm not sure what you mean by 'Fugitive can't reliably know when
> > git decides to use the pager'
>
> I'm still confused by this - if there is no tty git wont use a pager.
>
That's the problem, git doesn't tell whoever calls it if it is going
to use the pager,
and that information is useful for Fugitive. Why? Because even if
fugitive just naively
reads git's output, it doesn't know whether it needs to open a pager.

i.e If the user calls `git log`, the log is printed through the pager.
However, if a user
calls the exact same command through Fugitive. Fugitive needs to know
ahead if the
resulting command is going to need a pager (AKA a new window in vim) and prepare
accordingly. However, since git does not tell us when it's going to
open a page, we
have to book-keep commands and whether they are going to use the pager or not.

> Best Wishes
>
> Phillip
>
> >> I started discussing this on an issue in Fugitive's github page[2] and
> >> Tim Pope (the creator and maintainer of Fugitive, thank you!)
> >> explained that `git` doesn't use a pager if there is no pty so it's
> >> impossible to override its behavior.
> >>
> >> We had some ideas how to make this feasible (as you can read on the
> >> thread) but for brevity's sake I'll present the best (IMO) idea:
> >> Essentially, at `pager.c`, don't short-circuit in `git_pager` (or
> >> `setup_pager`?) due to pty absence if a new environment variable is
> >> present, perhaps something like `GIT_PAGER_FORCE` which will override
> >> the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to
> >> apply custom logic through to pager to know if one exists and present
> >> the window in vim.
> >
> > I note that the latest comment [1] on the github issue talks about a
> > different solution that would cause all git commands to behave as if
> > there was a pty present.
> >
I hadn't seen that comment when I submitted the initial mail, and in fact - this
might actually be a better solution for this problem (and several others). If we
can force git to think that it has a pty stdout, even though it
doesn't. We can just
read its output and convert it to the way we want it displayed in vim.

> > [1]
> > https://github.com/tpope/vim-fugitive/issues/1772#issuecomment-866401942
> >
> > Best Wishes
> >
> > Phillip
> >
> >> I will appreciate any written thoughts on the matter, thank you :)
> >>
> >> P.S. I am a complete newbie in regards to mailing lists etiquette,
> >> pardon me if I've done anything incorrect
> >> P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all
> >> changes to `pager.c`, sorry if that was wrong of me (You probably got
> >> this mail twice because of a misconfiguration, oops)
> >>
> >>
> >> 1. https://github.com/tpope/vim-fugitive
> >> 2. https://github.com/tpope/vim-fugitive/issues/1772
> >>
>

1. https://github.com/tpope/vim-fugitive/blob/79e2bd381ad6e7f3d70ce4a8ec9f3f107b40f053/autoload/fugitive.vim#L2951-L2956

This is the current proposal:

--- a/pager.c
+++ b/pager.c
@@ -45,7 +45,8 @@ const char *git_pager(int stdout_is_tty)
 {
        const char *pager;

-       if (!stdout_is_tty)
+
+       if (!stdout_is_tty && !(is_terminal_dumb() &&
git_env_bool("GIT_FORCE_TTY_DUMB", 0)))
                return NULL;

        pager = getenv("GIT_PAGER");


Avishay

Re: Forcing git to use a pager without a tty

[Ævar Arnfjörð Bjarmason]

On Thu, Jun 24 2021, Avishay Matayev wrote:

It's interesting that your original mail didn't end up in our main
archive:
https://lore.kernel.org/git/CAJ-0OswsrnAuCwU6U=S2i1qKkg=66U-8RHSGqD2kh9T_30Yw9w@mail.gmail.com/,
but e.g. marc.info has it (and it's in my mailbox):
https://marc.info/?l=git&m=162440160200930

> On Wed, 23 Jun 2021 at 12:29, Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> On 23/06/2021 10:05, Phillip Wood wrote:
>> > Hi Avishay
>> >
>> > On 22/06/2021 23:40, Avishay Matayev wrote:
>> >> Hi there!
>> >>
>> >> Fugitive[1] is a vim plugin that wraps git and many of its commands
>> >> into the editor in a really awesome way, I won't meddle into it too
>> >> much as you can read about it in its README, but as you understand, it
>> >> uses git, a lot.
>> >>
>> >> Some git commands use a pager, which is usually a program that needs a
>> >> pty to function properly (`less`, for example). Fugitive can't really
>> >> use a pty for the pager as vim runs its subprocesses without a pty.
>> >> Therefore Fugitive just creates its own pager (which is a simple
>> >> window in vim) and pastes the git command output there.
>> >
>> > If I understand correctly fugitive is reading the output of the git
>> > command over a pipe and putting it into a vim buffer?
>> >
> AFAIK, no. It could read the output through vim's jobstart API (that calls a
> callback when there's activity on stdout for example) or by
> redirecting the output
> to a temporary file and reading it.

...

>> >> The only problem left is that Fugitive can't reliably know when git
>> >> decides to use the pager, for example `git reflog show` does raise the
>> >> pager while `git reflog expire` does not. Fugitive currently maintains
>> >> an (very possibly) incomplete list of commands that need a pager but
>> >> maintaining it manually isn't ideal.
>> >
>> > I don't understand, if as you say above there isn't a pty then git wont
>> > use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem
>> > to,
>>
>> Sorry that is confused. GIT_PAGER_IN_USE only causes git to act as if a
>> pager is being used (for example so it colors things as if it was
>> outputting to a tty), it does not invoke the pager
>>
>>   so I'm not sure what you mean by 'Fugitive can't reliably know when
>> > git decides to use the pager'
>>
>> I'm still confused by this - if there is no tty git wont use a pager.
>>
> That's the problem, git doesn't tell whoever calls it if it is going
> to use the pager,
> and that information is useful for Fugitive. Why? Because even if
> fugitive just naively
> reads git's output, it doesn't know whether it needs to open a pager.
>
> i.e If the user calls `git log`, the log is printed through the pager.
> However, if a user
> calls the exact same command through Fugitive. Fugitive needs to know
> ahead if the
> resulting command is going to need a pager (AKA a new window in vim) and prepare
> accordingly. However, since git does not tell us when it's going to
> open a page, we
> have to book-keep commands and whether they are going to use the pager or not.

Having read this thread it feels as though there's a missing description
between the 2nd and 3rd paragraphs of your original mail.

I.e. you never really explicitly said what you're going to use this for,
and why it's needed, but I think I get it now.

The "straighforward" thing to do would be to just capture stderr/stdout,
which is what e.g. Magit does, which I gather is a similar thing to this
"Fugitive" thing, except for Emacs (I use Magit heavily, I've only spent
~5m browsing the Fugitive page for the first time, skipping through a
couple of screencasts).

But what you're doing in this editor plugin is assigning semantic
importance to if and when git uses a pager. If it would you open a
dedicated window with the output, if it doesn't you presumably show it
differently, just the exit code, in some "raw git output" buffer or
something?

> 1. https://github.com/tpope/vim-fugitive/blob/79e2bd381ad6e7f3d70ce4a8ec9f3f107b40f053/autoload/fugitive.vim#L2951-L2956
>
> This is the current proposal:
>
> --- a/pager.c
> +++ b/pager.c
> @@ -45,7 +45,8 @@ const char *git_pager(int stdout_is_tty)
>  {
>         const char *pager;
>
> -       if (!stdout_is_tty)
> +
> +       if (!stdout_is_tty && !(is_terminal_dumb() &&
> git_env_bool("GIT_FORCE_TTY_DUMB", 0)))
>                 return NULL;
>
>         pager = getenv("GIT_PAGER");

So yes, I could see how this would be useful, and FWIW I've got nothing
against such a thing, it just took me a while to understand what you
were going for, and maybe I still don't get it.

I will say that I think this approach is /probably/ not a very fruitful
one in the longer term for an editor plugin.

E.g. in Magit everything we'd invoke a pager for such as "log", "blame"
etc. is a dedicated top-level command, Magit heavily modifies the UI, so
e.g. for the "log" equivalent you can "click" on commits, fold them and
their diffs etc.

So "does it use a pager?" nonwithstanding I'd think any sufficiently
mature editor plugin would end up having to maintain its own list of
"this is an interesting command, and this is how I make use of its
anyway. So the equivalent of piping what we'd send to the pager to
another FD (or setting a flag or whatever) wouldn't be very useful in
the end.

But hey, if it works for you why not :)

What do you do about things like "git status" etc. that produce a lot of
output, but don't invoke the pager even then (but perhaps should), ditto
for all the plumbing or plumbing-like such as "ls-files", "ls-tree" etc?

Re: Forcing git to use a pager without a tty

[Felipe Contreras]

Avishay Matayev wrote:
> On Wed, 23 Jun 2021 at 04:57, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:

> > Seems like a completely sensible request to me, except I would do it a
> > slightly different place:
> >
> > --- a/pager.c
> > +++ b/pager.c
> > @@ -101,7 +101,7 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)
> >
> >  void setup_pager(void)
> >  {
> > -       const char *pager = git_pager(isatty(1));
> > +       const char *pager = git_pager(git_env_bool("GIT_PAGER_FORCE", isatty(1)));
> >
> >         if (!pager)
> >                 return;
> >
> > I'm Cc'ing Jonathan Nieder who seems to have toched these lines.

> 
> Well I'm glad to hear you find it sensible, though your patch suggests
> that `GIT_PAGER_FORCE` will be a boolean, while I expect it to be an
> actual pager, which would make more sense IMO.

I just copied the idea from the GitHub issue, I find it simple and easy
to implement in the code this way. A GIT_PAGER_FORCE as you suggest
would be a bit more complicated.

Either way feel free to provide a patch with whatever approach you think
is best.

However, for a proper patch you need to add documentation too (see
Documentation/git-var.txt), and preferably some tests.

Cheers.

-- 
Felipe Contreras

Re: Forcing git to use a pager without a tty

[Avishay Matayev]

On Fri, 25 Jun 2021 at 01:31, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
>
>
> On Thu, Jun 24 2021, Avishay Matayev wrote:
>
> It's interesting that your original mail didn't end up in our main
> archive:
> https://lore.kernel.org/git/CAJ-0OswsrnAuCwU6U=S2i1qKkg=66U-8RHSGqD2kh9T_30Yw9w@mail.gmail.com/,
> but e.g. marc.info has it (and it's in my mailbox):
> https://marc.info/?l=git&m=162440160200930
>
I guess the empty subject tripped it somehow, oh well..
> > On Wed, 23 Jun 2021 at 12:29, Phillip Wood <phillip.wood123@gmail.com> wrote:
> >>
> >> On 23/06/2021 10:05, Phillip Wood wrote:
> >> > Hi Avishay
> >> >
> >> > On 22/06/2021 23:40, Avishay Matayev wrote:
> >> >> Hi there!
> >> >>
> >> >> Fugitive[1] is a vim plugin that wraps git and many of its commands
> >> >> into the editor in a really awesome way, I won't meddle into it too
> >> >> much as you can read about it in its README, but as you understand, it
> >> >> uses git, a lot.
> >> >>
> >> >> Some git commands use a pager, which is usually a program that needs a
> >> >> pty to function properly (`less`, for example). Fugitive can't really
> >> >> use a pty for the pager as vim runs its subprocesses without a pty.
> >> >> Therefore Fugitive just creates its own pager (which is a simple
> >> >> window in vim) and pastes the git command output there.
> >> >
> >> > If I understand correctly fugitive is reading the output of the git
> >> > command over a pipe and putting it into a vim buffer?
> >> >
> > AFAIK, no. It could read the output through vim's jobstart API (that calls a
> > callback when there's activity on stdout for example) or by
> > redirecting the output
> > to a temporary file and reading it.
>
> ...
I might have taken the word 'pipe' too literally :)
>
> >> >> The only problem left is that Fugitive can't reliably know when git
> >> >> decides to use the pager, for example `git reflog show` does raise the
> >> >> pager while `git reflog expire` does not. Fugitive currently maintains
> >> >> an (very possibly) incomplete list of commands that need a pager but
> >> >> maintaining it manually isn't ideal.
> >> >
> >> > I don't understand, if as you say above there isn't a pty then git wont
> >> > use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem
> >> > to,
> >>
> >> Sorry that is confused. GIT_PAGER_IN_USE only causes git to act as if a
> >> pager is being used (for example so it colors things as if it was
> >> outputting to a tty), it does not invoke the pager
> >>
> >>   so I'm not sure what you mean by 'Fugitive can't reliably know when
> >> > git decides to use the pager'
> >>
> >> I'm still confused by this - if there is no tty git wont use a pager.
> >>
> > That's the problem, git doesn't tell whoever calls it if it is going
> > to use the pager,
> > and that information is useful for Fugitive. Why? Because even if
> > fugitive just naively
> > reads git's output, it doesn't know whether it needs to open a pager.
> >
> > i.e If the user calls `git log`, the log is printed through the pager.
> > However, if a user
> > calls the exact same command through Fugitive. Fugitive needs to know
> > ahead if the
> > resulting command is going to need a pager (AKA a new window in vim) and prepare
> > accordingly. However, since git does not tell us when it's going to
> > open a page, we
> > have to book-keep commands and whether they are going to use the pager or not.
>
> Having read this thread it feels as though there's a missing description
> between the 2nd and 3rd paragraphs of your original mail.
>
> I.e. you never really explicitly said what you're going to use this for,
> and why it's needed, but I think I get it now.
>
> The "straighforward" thing to do would be to just capture stderr/stdout,
> which is what e.g. Magit does, which I gather is a similar thing to this
> "Fugitive" thing, except for Emacs (I use Magit heavily, I've only spent
> ~5m browsing the Fugitive page for the first time, skipping through a
> couple of screencasts).
>
> But what you're doing in this editor plugin is assigning semantic
> importance to if and when git uses a pager. If it would you open a
> dedicated window with the output, if it doesn't you presumably show it
> differently, just the exit code, in some "raw git output" buffer or
> something?
>
No, there's no semantic importance. If a command has semantic
importance, Fugitive will act appropriately. We want to know whether
git is going to use a pager to provide *correct* behavior for the most
simple cases, which is either open a pager with the output, or don't.
> > 1. https://github.com/tpope/vim-fugitive/blob/79e2bd381ad6e7f3d70ce4a8ec9f3f107b40f053/autoload/fugitive.vim#L2951-L2956
> >
> > This is the current proposal:
> >
> > --- a/pager.c
> > +++ b/pager.c
> > @@ -45,7 +45,8 @@ const char *git_pager(int stdout_is_tty)
> >  {
> >         const char *pager;
> >
> > -       if (!stdout_is_tty)
> > +
> > +       if (!stdout_is_tty && !(is_terminal_dumb() &&
> > git_env_bool("GIT_FORCE_TTY_DUMB", 0)))
> >                 return NULL;
> >
> >         pager = getenv("GIT_PAGER");
>
> So yes, I could see how this would be useful, and FWIW I've got nothing
> against such a thing, it just took me a while to understand what you
> were going for, and maybe I still don't get it.
>
> I will say that I think this approach is /probably/ not a very fruitful
> one in the longer term for an editor plugin.
>
The main problem this change solves is overriding git's decisiveness
on how to act in certain situations. e.g. If no pty is present, git won't
open the pager, i.e git will never invoke the command that is inside
the PAGER environment variable. If no pty is present, git won't report
its progress on a `git push` through a progress bar.

Forcing git to always act like it has a pty will allow fugitive to present
these 'advanced' features to its users.
> E.g. in Magit everything we'd invoke a pager for such as "log", "blame"
> etc. is a dedicated top-level command, Magit heavily modifies the UI, so
> e.g. for the "log" equivalent you can "click" on commits, fold them and
> their diffs etc.
>
> So "does it use a pager?" nonwithstanding I'd think any sufficiently
> mature editor plugin would end up having to maintain its own list of
> "this is an interesting command, and this is how I make use of its
> anyway. So the equivalent of piping what we'd send to the pager to
> another FD (or setting a flag or whatever) wouldn't be very useful in
> the end.
>
Yes, but not every command is an interesting one. You don't need a
special window for every command, and some commands don't even
have any output besides their return value.
> But hey, if it works for you why not :)
>
> What do you do about things like "git status" etc. that produce a lot of
> output, but don't invoke the pager even then (but perhaps should), ditto
> for all the plumbing or plumbing-like such as "ls-files", "ls-tree" etc?
>
`Git status` (or just `Git`) is special because it opens the fugitive status
buffer (just like the one Magit has).

`git ls-files | ls-tree` just create an output window that shows the output
of the command, that window disappears on any keystroke.


I'd also like to add that I am just a regular user of the plugin, I've never
committed anything to it and I just wanted to solve an issue that
bothered me, so what I write here's from the perspective of a user (and
someone who can read code), not from the view of a maintainer or
the designer of Fugitive.

Avishay

Re: Forcing git to use a pager without a tty

[Avishay Matayev]

On Fri, 25 Jun 2021 at 20:42, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Either way feel free to provide a patch with whatever approach you think
> is best.
>
> However, for a proper patch you need to add documentation too (see
> Documentation/git-var.txt), and preferably some tests.
>
> Cheers.
>
> --
> Felipe Contreras
I will do so, thank you.

Avishay

Re: Forcing git to use a pager without a tty

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> What do you do about things like "git status" etc. that produce a lot of
> output, but don't invoke the pager even then (but perhaps should), ditto
> for all the plumbing or plumbing-like such as "ls-files", "ls-tree" etc?

I am puzzled.  Doesn't "git -p status" force spawning the pager?

    ... goes and checks ...

Ahh, OK, -p does force us to try to see if pager is warranted, and
then we decide it is not because the output is not sent to the tty.

This behaviour makes sense if we are truly driving a pager via
GIT_PAGER, but I agree with you that it probably is inconvenient if
you are abusing the GIT_PAGER mechanism to drive something that is
not about paging output but about grabbing the output and
post-processing it.