* (no subject) @ 2021-06-22 22:40 Avishay Matayev 2021-06-23 1:56 ` Felipe Contreras ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Avishay Matayev @ 2021-06-22 22:40 UTC (permalink / raw) To: git; +Cc: gitster 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* (no subject) 2021-06-22 22:40 Avishay Matayev @ 2021-06-23 1:56 ` Felipe Contreras 2021-06-23 8:25 ` Forcing git to use a pager without a tty Avishay Matayev 2021-06-23 8:12 ` Why empty subject? (was Re: ) Bagas Sanjaya 2021-06-23 9:05 ` Forcing git to use a pager without a tty Phillip Wood 2 siblings, 1 reply; 13+ messages in thread From: Felipe Contreras @ 2021-06-23 1:56 UTC (permalink / raw) To: Avishay Matayev, git; +Cc: gitster, Jonathan Nieder 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-23 1:56 ` Felipe Contreras @ 2021-06-23 8:25 ` Avishay Matayev 2021-06-25 17:42 ` Felipe Contreras 0 siblings, 1 reply; 13+ messages in thread From: Avishay Matayev @ 2021-06-23 8:25 UTC (permalink / raw) To: Felipe Contreras; +Cc: git, gitster, Jonathan Nieder 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-23 8:25 ` Forcing git to use a pager without a tty Avishay Matayev @ 2021-06-25 17:42 ` Felipe Contreras 2021-06-26 10:28 ` Avishay Matayev 0 siblings, 1 reply; 13+ messages in thread From: Felipe Contreras @ 2021-06-25 17:42 UTC (permalink / raw) To: Avishay Matayev, Felipe Contreras; +Cc: git, gitster, Jonathan Nieder 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-25 17:42 ` Felipe Contreras @ 2021-06-26 10:28 ` Avishay Matayev 0 siblings, 0 replies; 13+ messages in thread From: Avishay Matayev @ 2021-06-26 10:28 UTC (permalink / raw) To: Felipe Contreras; +Cc: git, gitster, Jonathan Nieder 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Why empty subject? (was Re: ) 2021-06-22 22:40 Avishay Matayev 2021-06-23 1:56 ` Felipe Contreras @ 2021-06-23 8:12 ` Bagas Sanjaya 2021-06-23 8:21 ` Avishay Matayev 2021-06-23 9:05 ` Forcing git to use a pager without a tty Phillip Wood 2 siblings, 1 reply; 13+ messages in thread From: Bagas Sanjaya @ 2021-06-23 8:12 UTC (permalink / raw) To: Avishay Matayev, git; +Cc: gitster 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Why empty subject? (was Re: ) 2021-06-23 8:12 ` Why empty subject? (was Re: ) Bagas Sanjaya @ 2021-06-23 8:21 ` Avishay Matayev 0 siblings, 0 replies; 13+ messages in thread From: Avishay Matayev @ 2021-06-23 8:21 UTC (permalink / raw) To: Bagas Sanjaya; +Cc: git, gitster 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-22 22:40 Avishay Matayev 2021-06-23 1:56 ` Felipe Contreras 2021-06-23 8:12 ` Why empty subject? (was Re: ) Bagas Sanjaya @ 2021-06-23 9:05 ` Phillip Wood 2021-06-23 9:29 ` Phillip Wood 2 siblings, 1 reply; 13+ messages in thread From: Phillip Wood @ 2021-06-23 9:05 UTC (permalink / raw) To: Avishay Matayev, git; +Cc: gitster 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-23 9:05 ` Forcing git to use a pager without a tty Phillip Wood @ 2021-06-23 9:29 ` Phillip Wood 2021-06-24 20:25 ` Avishay Matayev 0 siblings, 1 reply; 13+ messages in thread From: Phillip Wood @ 2021-06-23 9:29 UTC (permalink / raw) To: Avishay Matayev, git; +Cc: gitster 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 >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-23 9:29 ` Phillip Wood @ 2021-06-24 20:25 ` Avishay Matayev 2021-06-24 22:10 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 13+ messages in thread From: Avishay Matayev @ 2021-06-24 20:25 UTC (permalink / raw) To: phillip.wood; +Cc: git, gitster 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-24 20:25 ` Avishay Matayev @ 2021-06-24 22:10 ` Ævar Arnfjörð Bjarmason 2021-06-26 10:26 ` Avishay Matayev 2021-06-29 6:09 ` Junio C Hamano 0 siblings, 2 replies; 13+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2021-06-24 22:10 UTC (permalink / raw) To: Avishay Matayev; +Cc: phillip.wood, git, gitster 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? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-24 22:10 ` Ævar Arnfjörð Bjarmason @ 2021-06-26 10:26 ` Avishay Matayev 2021-06-29 6:09 ` Junio C Hamano 1 sibling, 0 replies; 13+ messages in thread From: Avishay Matayev @ 2021-06-26 10:26 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: phillip.wood, git, gitster 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Forcing git to use a pager without a tty 2021-06-24 22:10 ` Ævar Arnfjörð Bjarmason 2021-06-26 10:26 ` Avishay Matayev @ 2021-06-29 6:09 ` Junio C Hamano 1 sibling, 0 replies; 13+ messages in thread From: Junio C Hamano @ 2021-06-29 6:09 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason; +Cc: Avishay Matayev, phillip.wood, git Æ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. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-06-29 6:09 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-22 22:40 Avishay Matayev 2021-06-23 1:56 ` Felipe Contreras 2021-06-23 8:25 ` Forcing git to use a pager without a tty Avishay Matayev 2021-06-25 17:42 ` Felipe Contreras 2021-06-26 10:28 ` Avishay Matayev 2021-06-23 8:12 ` Why empty subject? (was Re: ) Bagas Sanjaya 2021-06-23 8:21 ` Avishay Matayev 2021-06-23 9:05 ` Forcing git to use a pager without a tty Phillip Wood 2021-06-23 9:29 ` Phillip Wood 2021-06-24 20:25 ` Avishay Matayev 2021-06-24 22:10 ` Ævar Arnfjörð Bjarmason 2021-06-26 10:26 ` Avishay Matayev 2021-06-29 6:09 ` Junio C Hamano
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.