threads / discuss / 55992

Why empty subject? (was Re: )

Subject: Why empty subject? (was Re: )

## tl;dr

13 messages between Jun 22, 2021 and Jun 29, 2021.

replies: 12people: 6as markdown or json

Avishay Matayev· Jun 22, 2021, 22:40 UTC · lore

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
Felipe Contreras· Jun 23, 2021, 01:56 UTC · re: Avishay Matayev · lore

Avishay Matayev wrote:
Show 5 quoted lines
> 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.
Show 8 quoted lines
> 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
Avishay Matayev· Jun 23, 2021, 08:25 UTC · re: Felipe Contreras · lore

Re: Forcing git to use a pager without a tty

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

Show 16 quoted lines
>
> 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.
Show 30 quoted lines
> > 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
Felipe Contreras· Jun 25, 2021, 17:42 UTC · re: Avishay Matayev · lore

Re: Forcing git to use a pager without a tty

Avishay Matayev wrote:
> On Wed, 23 Jun 2021 at 04:57, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
Show 16 quoted lines
> > 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
Avishay Matayev· Jun 26, 2021, 10:28 UTC · re: Felipe Contreras · lore

Re: Forcing git to use a pager without a tty

On Fri, 25 Jun 2021 at 20:42, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 10 quoted lines
> 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
Bagas Sanjaya· Jun 23, 2021, 08:12 UTC · re: Avishay Matayev · lore
Hi,
On 23/06/21 05.40, Avishay Matayev wrote:
Show 5 quoted lines
> 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
Avishay Matayev· Jun 23, 2021, 08:21 UTC · re: Bagas Sanjaya · lore

Re: Why empty subject? (was Re: )

On Wed, 23 Jun 2021, 11:13 am Bagas Sanjaya, <bagasdotme@gmail.com> wrote:
Show 14 quoted lines
>
> 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
Phillip Wood· Jun 23, 2021, 09:05 UTC · re: Avishay Matayev · lore

Re: Forcing git to use a pager without a tty

Hi Avishay
On 22/06/2021 23:40, Avishay Matayev wrote:
Show 12 quoted lines
> 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?

Show 5 quoted lines
> 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'

Show 13 quoted lines
> 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
Show 12 quoted lines
> 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
> 
Phillip Wood· Jun 23, 2021, 09:29 UTC · re: Phillip Wood · lore

Re: Forcing git to use a pager without a tty

On 23/06/2021 10:05, Phillip Wood wrote:
Show 28 quoted lines
> 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
Show 37 quoted lines
>> 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
>>
Avishay Matayev· Jun 24, 2021, 20:25 UTC · re: Phillip Wood · lore

Re: Forcing git to use a pager without a tty

Hello Phillip!
On Wed, 23 Jun 2021 at 12:29, Phillip Wood <phillip.wood123@gmail.com> wrote:
Show 21 quoted lines
>
> 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.

Show 19 quoted lines
> >> 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.

Show 22 quoted lines
> 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.

Show 20 quoted lines
> > [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
Ævar Arnfjörð Bjarmason· Jun 24, 2021, 22:10 UTC · re: Avishay Matayev · lore

Re: Forcing git to use a pager without a tty

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

Show 26 quoted lines
> 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.
...
Show 33 quoted lines
>> >> 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?

Show 17 quoted lines
> 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?

Avishay Matayev· Jun 26, 2021, 10:26 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: Forcing git to use a pager without a tty

On Fri, 25 Jun 2021 at 01:31, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 10 quoted lines
>
>
> 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..
Show 28 quoted lines
> > 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 :)
Show 53 quoted lines
>
> >> >> 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.

Show 25 quoted lines
> > 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.

Show 12 quoted lines
> 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.

Show 6 quoted lines
> 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
Junio C Hamano· Jun 29, 2021, 06:09 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: Forcing git to use a pager without a tty

Æ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.

← back to recent threads