{"thread":{"id":"55992","subject":"Why empty subject? (was Re: )","startedAt":"2021-06-22T22:40:19Z","lastAt":"2021-06-29T06:09:29Z","messageCount":13,"participants":["Bagas Sanjaya","Avishay Matayev","Phillip Wood","Ævar Arnfjörð Bjarmason","Felipe Contreras","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"428605","messageId":"CAJ-0OswsrnAuCwU6U=S2i1qKkg=66U-8RHSGqD2kh9T_30Yw9w@mail.gmail.com","threadId":"55992","inReplyTo":null,"subject":"","fromName":"Avishay Matayev","fromEmail":"me@avishay.dev","sentAt":"2021-06-22T22:40:06Z","receivedAt":"2021-06-22T22:40:19Z","isPatch":false,"sender":{"key":"me@avishay.dev","avatar":null},"body":"Hi there!\n\nFugitive[1] is a vim plugin that wraps git and many of its commands\ninto the editor in a really awesome way, I won't meddle into it too\nmuch as you can read about it in its README, but as you understand, it\nuses git, a lot.\n\nSome git commands use a pager, which is usually a program that needs a\npty to function properly (`less`, for example). Fugitive can't really\nuse a pty for the pager as vim runs its subprocesses without a pty.\nTherefore Fugitive just creates its own pager (which is a simple\nwindow in vim) and pastes the git command output there.\n\nThe only problem left is that Fugitive can't reliably know when git\ndecides to use the pager, for example `git reflog show` does raise the\npager while `git reflog expire` does not. Fugitive currently maintains\nan (very possibly) incomplete list of commands that need a pager but\nmaintaining it manually isn't ideal.\n\nI started discussing this on an issue in Fugitive's github page[2] and\nTim Pope (the creator and maintainer of Fugitive, thank you!)\nexplained that `git` doesn't use a pager if there is no pty so it's\nimpossible to override its behavior.\n\nWe had some ideas how to make this feasible (as you can read on the\nthread) but for brevity's sake I'll present the best (IMO) idea:\nEssentially, at `pager.c`, don't short-circuit in `git_pager` (or\n`setup_pager`?) due to pty absence if a new environment variable is\npresent, perhaps something like `GIT_PAGER_FORCE` which will override\nthe `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to\napply custom logic through to pager to know if one exists and present\nthe window in vim.\n\nI will appreciate any written thoughts on the matter, thank you :)\n\nP.S. I am a complete newbie in regards to mailing lists etiquette,\npardon me if I've done anything incorrect\nP.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all\nchanges to `pager.c`, sorry if that was wrong of me (You probably got\nthis mail twice because of a misconfiguration, oops)\n\n\n1. https://github.com/tpope/vim-fugitive\n2. https://github.com/tpope/vim-fugitive/issues/1772\n"},{"id":"428606","messageId":"60d294eb984ba_3122081@natae.notmuch","threadId":"55992","inReplyTo":"CAJ-0OswsrnAuCwU6U=S2i1qKkg=66U-8RHSGqD2kh9T_30Yw9w@mail.gmail.com","subject":"","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-23T01:56:59Z","receivedAt":"2021-06-23T01:57:04Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Avishay Matayev wrote:\n\n> Some git commands use a pager, which is usually a program that needs a\n> pty to function properly (`less`, for example). Fugitive can't really\n> use a pty for the pager as vim runs its subprocesses without a pty.\n> Therefore Fugitive just creates its own pager (which is a simple\n> window in vim) and pastes the git command output there.\n\nThat's not necessarily true; vim uses a bunch of convoluted logic to\nsometimes use a pty, but that depends on the mode used (graphical vs.\nconsole), and a bunch of other things.\n\nEven more complicated when you throw nvim into the mix.\n\nBut yeah, for the sake of simplication let's say that's true.\n\n> I started discussing this on an issue in Fugitive's github page[2] and\n> Tim Pope (the creator and maintainer of Fugitive, thank you!)\n> explained that `git` doesn't use a pager if there is no pty so it's\n> impossible to override its behavior.\n\nThat is true.\n\n> We had some ideas how to make this feasible (as you can read on the\n> thread) but for brevity's sake I'll present the best (IMO) idea:\n> Essentially, at `pager.c`, don't short-circuit in `git_pager` (or\n> `setup_pager`?) due to pty absence if a new environment variable is\n> present, perhaps something like `GIT_PAGER_FORCE` which will override\n> the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to\n> apply custom logic through to pager to know if one exists and present\n> the window in vim.\n\nSeems like a completely sensible request to me, except I would do it a\nslightly different place:\n\n--- a/pager.c\n+++ b/pager.c\n@@ -101,7 +101,7 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)\n \n void setup_pager(void)\n {\n-       const char *pager = git_pager(isatty(1));\n+       const char *pager = git_pager(git_env_bool(\"GIT_PAGER_FORCE\", isatty(1)));\n \n        if (!pager)\n                return;\n\nI'm Cc'ing Jonathan Nieder who seems to have toched these lines.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"428261","messageId":"2c2cfbc6-c428-772e-6ccf-66261cf0e331@gmail.com","threadId":"55992","inReplyTo":"CAJ-0OswsrnAuCwU6U=S2i1qKkg=66U-8RHSGqD2kh9T_30Yw9w@mail.gmail.com","subject":"Why empty subject? (was Re: )","fromName":"Bagas Sanjaya","fromEmail":"bagasdotme@gmail.com","sentAt":"2021-06-23T08:12:45Z","receivedAt":"2021-06-23T08:13:08Z","isPatch":false,"sender":{"key":"bagasdotme@gmail.com","avatar":"https://avatars.githubusercontent.com/u/40219486?v=4"},"body":"Hi,\n\nOn 23/06/21 05.40, Avishay Matayev wrote:\n> P.S. I am a complete newbie in regards to mailing lists etiquette,\n> pardon me if I've done anything incorrect\n> P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all\n> changes to `pager.c`, sorry if that was wrong of me (You probably got\n> this mail twice because of a misconfiguration, oops)\n\nWhy did you send the thread with empty subject?\n\n-- \nAn old man doll... just what I always wanted! - Clara\n"},{"id":"428262","messageId":"CAJ-0OsyXuNoG04j6rPioMuegp2jtJT4gHE-m_9seQUix5Z3_LQ@mail.gmail.com","threadId":"55992","inReplyTo":"2c2cfbc6-c428-772e-6ccf-66261cf0e331@gmail.com","subject":"Re: Why empty subject? (was Re: )","fromName":"Avishay Matayev","fromEmail":"me@avishay.dev","sentAt":"2021-06-23T08:21:18Z","receivedAt":"2021-06-23T08:21:35Z","isPatch":false,"sender":{"key":"me@avishay.dev","avatar":null},"body":"On Wed, 23 Jun 2021, 11:13 am Bagas Sanjaya, <bagasdotme@gmail.com> wrote:\n>\n> Hi,\n>\n> On 23/06/21 05.40, Avishay Matayev wrote:\n> > P.S. I am a complete newbie in regards to mailing lists etiquette,\n> > pardon me if I've done anything incorrect\n> > P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all\n> > changes to `pager.c`, sorry if that was wrong of me (You probably got\n> > this mail twice because of a misconfiguration, oops)\n>\n> Why did you send the thread with empty subject?\n>\n> --\n> An old man doll... just what I always wanted! - Clara\n\nOh, my bad.\n\nIt was \"Forcing git to use a pager without a tty\" and I sent it with\nmy initial mail that got rejected because I didn't send it as plain\ntext. I copied the mail and fixed it but forgot to reuse the subject,\noops!\n\nI'll change the subject in the other reply, sorry about that.\n\nAvishay\n"},{"id":"428263","messageId":"CAJ-0Oswry0Rry_0ZqZzzVGuzfU-apZnv95LcxQEhm7GVA=uOxA@mail.gmail.com","threadId":"55992","inReplyTo":"60d294eb984ba_3122081@natae.notmuch","subject":"Re: Forcing git to use a pager without a tty","fromName":"Avishay Matayev","fromEmail":"me@avishay.dev","sentAt":"2021-06-23T08:25:00Z","receivedAt":"2021-06-23T08:25:15Z","isPatch":false,"sender":{"key":"me@avishay.dev","avatar":null},"body":"On Wed, 23 Jun 2021 at 04:57, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n>\n> Avishay Matayev wrote:\n>\n> > Some git commands use a pager, which is usually a program that needs a\n> > pty to function properly (`less`, for example). Fugitive can't really\n> > use a pty for the pager as vim runs its subprocesses without a pty.\n> > Therefore Fugitive just creates its own pager (which is a simple\n> > window in vim) and pastes the git command output there.\n>\n> That's not necessarily true; vim uses a bunch of convoluted logic to\n> sometimes use a pty, but that depends on the mode used (graphical vs.\n> console), and a bunch of other things.\n>\n> Even more complicated when you throw nvim into the mix.\n>\n> But yeah, for the sake of simplication let's say that's true.\n\nTrue, I did oversimplify.\n\n> > We had some ideas how to make this feasible (as you can read on the\n> > thread) but for brevity's sake I'll present the best (IMO) idea:\n> > Essentially, at `pager.c`, don't short-circuit in `git_pager` (or\n> > `setup_pager`?) due to pty absence if a new environment variable is\n> > present, perhaps something like `GIT_PAGER_FORCE` which will override\n> > the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to\n> > apply custom logic through to pager to know if one exists and present\n> > the window in vim.\n>\n> Seems like a completely sensible request to me, except I would do it a\n> slightly different place:\n>\n> --- a/pager.c\n> +++ b/pager.c\n> @@ -101,7 +101,7 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)\n>\n>  void setup_pager(void)\n>  {\n> -       const char *pager = git_pager(isatty(1));\n> +       const char *pager = git_pager(git_env_bool(\"GIT_PAGER_FORCE\", isatty(1)));\n>\n>         if (!pager)\n>                 return;\n>\n> I'm Cc'ing Jonathan Nieder who seems to have toched these lines.\n>\n> Cheers.\n>\n> --\n> Felipe Contreras\n\nWell I'm glad to hear you find it sensible, though your patch suggests\nthat `GIT_PAGER_FORCE` will be a boolean, while I expect it to be an\nactual pager, which would make more sense IMO.\n\nAvishay\n"},{"id":"428264","messageId":"077d147c-c22f-6463-6545-3aa991e7868b@gmail.com","threadId":"55992","inReplyTo":"CAJ-0OswsrnAuCwU6U=S2i1qKkg=66U-8RHSGqD2kh9T_30Yw9w@mail.gmail.com","subject":"Re: Forcing git to use a pager without a tty","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-06-23T09:05:22Z","receivedAt":"2021-06-23T09:05:41Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Avishay\n\nOn 22/06/2021 23:40, Avishay Matayev wrote:\n> Hi there!\n> \n> Fugitive[1] is a vim plugin that wraps git and many of its commands\n> into the editor in a really awesome way, I won't meddle into it too\n> much as you can read about it in its README, but as you understand, it\n> uses git, a lot.\n> \n> Some git commands use a pager, which is usually a program that needs a\n> pty to function properly (`less`, for example). Fugitive can't really\n> use a pty for the pager as vim runs its subprocesses without a pty.\n> Therefore Fugitive just creates its own pager (which is a simple\n> window in vim) and pastes the git command output there.\n\nIf I understand correctly fugitive is reading the output of the git \ncommand over a pipe and putting it into a vim buffer?\n\n> The only problem left is that Fugitive can't reliably know when git\n> decides to use the pager, for example `git reflog show` does raise the\n> pager while `git reflog expire` does not. Fugitive currently maintains\n> an (very possibly) incomplete list of commands that need a pager but\n> maintaining it manually isn't ideal.\n\nI don't understand, if as you say above there isn't a pty then git wont \nuse a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem \nto, so I'm not sure what you mean by 'Fugitive can't reliably know when \ngit decides to use the pager'\n\n> I started discussing this on an issue in Fugitive's github page[2] and\n> Tim Pope (the creator and maintainer of Fugitive, thank you!)\n> explained that `git` doesn't use a pager if there is no pty so it's\n> impossible to override its behavior.\n> \n> We had some ideas how to make this feasible (as you can read on the\n> thread) but for brevity's sake I'll present the best (IMO) idea:\n> Essentially, at `pager.c`, don't short-circuit in `git_pager` (or\n> `setup_pager`?) due to pty absence if a new environment variable is\n> present, perhaps something like `GIT_PAGER_FORCE` which will override\n> the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to\n> apply custom logic through to pager to know if one exists and present\n> the window in vim.\n\nI note that the latest comment [1] on the github issue talks about a \ndifferent solution that would cause all git commands to behave as if \nthere was a pty present.\n\n[1] https://github.com/tpope/vim-fugitive/issues/1772#issuecomment-866401942\n\nBest Wishes\n\nPhillip\n\n> I will appreciate any written thoughts on the matter, thank you :)\n> \n> P.S. I am a complete newbie in regards to mailing lists etiquette,\n> pardon me if I've done anything incorrect\n> P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all\n> changes to `pager.c`, sorry if that was wrong of me (You probably got\n> this mail twice because of a misconfiguration, oops)\n> \n> \n> 1. https://github.com/tpope/vim-fugitive\n> 2. https://github.com/tpope/vim-fugitive/issues/1772\n> \n"},{"id":"428265","messageId":"b5462f20-5d9d-4775-ad5c-c8de686167af@gmail.com","threadId":"55992","inReplyTo":"077d147c-c22f-6463-6545-3aa991e7868b@gmail.com","subject":"Re: Forcing git to use a pager without a tty","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-06-23T09:29:57Z","receivedAt":"2021-06-23T09:30:02Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 23/06/2021 10:05, Phillip Wood wrote:\n> Hi Avishay\n> \n> On 22/06/2021 23:40, Avishay Matayev wrote:\n>> Hi there!\n>>\n>> Fugitive[1] is a vim plugin that wraps git and many of its commands\n>> into the editor in a really awesome way, I won't meddle into it too\n>> much as you can read about it in its README, but as you understand, it\n>> uses git, a lot.\n>>\n>> Some git commands use a pager, which is usually a program that needs a\n>> pty to function properly (`less`, for example). Fugitive can't really\n>> use a pty for the pager as vim runs its subprocesses without a pty.\n>> Therefore Fugitive just creates its own pager (which is a simple\n>> window in vim) and pastes the git command output there.\n> \n> If I understand correctly fugitive is reading the output of the git \n> command over a pipe and putting it into a vim buffer?\n> \n>> The only problem left is that Fugitive can't reliably know when git\n>> decides to use the pager, for example `git reflog show` does raise the\n>> pager while `git reflog expire` does not. Fugitive currently maintains\n>> an (very possibly) incomplete list of commands that need a pager but\n>> maintaining it manually isn't ideal.\n> \n> I don't understand, if as you say above there isn't a pty then git wont \n> use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem \n> to,\n\nSorry that is confused. GIT_PAGER_IN_USE only causes git to act as if a \npager is being used (for example so it colors things as if it was \noutputting to a tty), it does not invoke the pager\n\n  so I'm not sure what you mean by 'Fugitive can't reliably know when\n> git decides to use the pager'\n\nI'm still confused by this - if there is no tty git wont use a pager.\n\nBest Wishes\n\nPhillip\n\n>> I started discussing this on an issue in Fugitive's github page[2] and\n>> Tim Pope (the creator and maintainer of Fugitive, thank you!)\n>> explained that `git` doesn't use a pager if there is no pty so it's\n>> impossible to override its behavior.\n>>\n>> We had some ideas how to make this feasible (as you can read on the\n>> thread) but for brevity's sake I'll present the best (IMO) idea:\n>> Essentially, at `pager.c`, don't short-circuit in `git_pager` (or\n>> `setup_pager`?) due to pty absence if a new environment variable is\n>> present, perhaps something like `GIT_PAGER_FORCE` which will override\n>> the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to\n>> apply custom logic through to pager to know if one exists and present\n>> the window in vim.\n> \n> I note that the latest comment [1] on the github issue talks about a \n> different solution that would cause all git commands to behave as if \n> there was a pty present.\n> \n> [1] \n> https://github.com/tpope/vim-fugitive/issues/1772#issuecomment-866401942\n> \n> Best Wishes\n> \n> Phillip\n> \n>> I will appreciate any written thoughts on the matter, thank you :)\n>>\n>> P.S. I am a complete newbie in regards to mailing lists etiquette,\n>> pardon me if I've done anything incorrect\n>> P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all\n>> changes to `pager.c`, sorry if that was wrong of me (You probably got\n>> this mail twice because of a misconfiguration, oops)\n>>\n>>\n>> 1. https://github.com/tpope/vim-fugitive\n>> 2. https://github.com/tpope/vim-fugitive/issues/1772\n>>\n\n"},{"id":"428431","messageId":"CAJ-0Osy9JhGD0=6eF3jgZuoHJEzymksCWZZZC+A4FtHxzOrdhA@mail.gmail.com","threadId":"55992","inReplyTo":"b5462f20-5d9d-4775-ad5c-c8de686167af@gmail.com","subject":"Re: Forcing git to use a pager without a tty","fromName":"Avishay Matayev","fromEmail":"me@avishay.dev","sentAt":"2021-06-24T20:25:35Z","receivedAt":"2021-06-24T20:25:50Z","isPatch":false,"sender":{"key":"me@avishay.dev","avatar":null},"body":"Hello Phillip!\n\n\nOn Wed, 23 Jun 2021 at 12:29, Phillip Wood <phillip.wood123@gmail.com> wrote:\n>\n> On 23/06/2021 10:05, Phillip Wood wrote:\n> > Hi Avishay\n> >\n> > On 22/06/2021 23:40, Avishay Matayev wrote:\n> >> Hi there!\n> >>\n> >> Fugitive[1] is a vim plugin that wraps git and many of its commands\n> >> into the editor in a really awesome way, I won't meddle into it too\n> >> much as you can read about it in its README, but as you understand, it\n> >> uses git, a lot.\n> >>\n> >> Some git commands use a pager, which is usually a program that needs a\n> >> pty to function properly (`less`, for example). Fugitive can't really\n> >> use a pty for the pager as vim runs its subprocesses without a pty.\n> >> Therefore Fugitive just creates its own pager (which is a simple\n> >> window in vim) and pastes the git command output there.\n> >\n> > If I understand correctly fugitive is reading the output of the git\n> > command over a pipe and putting it into a vim buffer?\n> >\nAFAIK, no. It could read the output through vim's jobstart API (that calls a\ncallback when there's activity on stdout for example) or by\nredirecting the output\nto a temporary file and reading it.\n\n> >> The only problem left is that Fugitive can't reliably know when git\n> >> decides to use the pager, for example `git reflog show` does raise the\n> >> pager while `git reflog expire` does not. Fugitive currently maintains\n> >> an (very possibly) incomplete list of commands that need a pager but\n> >> maintaining it manually isn't ideal.\n> >\n> > I don't understand, if as you say above there isn't a pty then git wont\n> > use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem\n> > to,\n>\n> Sorry that is confused. GIT_PAGER_IN_USE only causes git to act as if a\n> pager is being used (for example so it colors things as if it was\n> outputting to a tty), it does not invoke the pager\n>\n>   so I'm not sure what you mean by 'Fugitive can't reliably know when\n> > git decides to use the pager'\n>\n> I'm still confused by this - if there is no tty git wont use a pager.\n>\nThat's the problem, git doesn't tell whoever calls it if it is going\nto use the pager,\nand that information is useful for Fugitive. Why? Because even if\nfugitive just naively\nreads git's output, it doesn't know whether it needs to open a pager.\n\ni.e If the user calls `git log`, the log is printed through the pager.\nHowever, if a user\ncalls the exact same command through Fugitive. Fugitive needs to know\nahead if the\nresulting command is going to need a pager (AKA a new window in vim) and prepare\naccordingly. However, since git does not tell us when it's going to\nopen a page, we\nhave to book-keep commands and whether they are going to use the pager or not.\n\n> Best Wishes\n>\n> Phillip\n>\n> >> I started discussing this on an issue in Fugitive's github page[2] and\n> >> Tim Pope (the creator and maintainer of Fugitive, thank you!)\n> >> explained that `git` doesn't use a pager if there is no pty so it's\n> >> impossible to override its behavior.\n> >>\n> >> We had some ideas how to make this feasible (as you can read on the\n> >> thread) but for brevity's sake I'll present the best (IMO) idea:\n> >> Essentially, at `pager.c`, don't short-circuit in `git_pager` (or\n> >> `setup_pager`?) due to pty absence if a new environment variable is\n> >> present, perhaps something like `GIT_PAGER_FORCE` which will override\n> >> the `PAGER` and `GIT_PAGER` variables. This will allow Fugitive to\n> >> apply custom logic through to pager to know if one exists and present\n> >> the window in vim.\n> >\n> > I note that the latest comment [1] on the github issue talks about a\n> > different solution that would cause all git commands to behave as if\n> > there was a pty present.\n> >\nI hadn't seen that comment when I submitted the initial mail, and in fact - this\nmight actually be a better solution for this problem (and several others). If we\ncan force git to think that it has a pty stdout, even though it\ndoesn't. We can just\nread its output and convert it to the way we want it displayed in vim.\n\n> > [1]\n> > https://github.com/tpope/vim-fugitive/issues/1772#issuecomment-866401942\n> >\n> > Best Wishes\n> >\n> > Phillip\n> >\n> >> I will appreciate any written thoughts on the matter, thank you :)\n> >>\n> >> P.S. I am a complete newbie in regards to mailing lists etiquette,\n> >> pardon me if I've done anything incorrect\n> >> P.P.S. I CC'd Junio C Hamano because he signed off on (almost?) all\n> >> changes to `pager.c`, sorry if that was wrong of me (You probably got\n> >> this mail twice because of a misconfiguration, oops)\n> >>\n> >>\n> >> 1. https://github.com/tpope/vim-fugitive\n> >> 2. https://github.com/tpope/vim-fugitive/issues/1772\n> >>\n>\n\n1. https://github.com/tpope/vim-fugitive/blob/79e2bd381ad6e7f3d70ce4a8ec9f3f107b40f053/autoload/fugitive.vim#L2951-L2956\n\nThis is the current proposal:\n\n--- a/pager.c\n+++ b/pager.c\n@@ -45,7 +45,8 @@ const char *git_pager(int stdout_is_tty)\n {\n        const char *pager;\n\n-       if (!stdout_is_tty)\n+\n+       if (!stdout_is_tty && !(is_terminal_dumb() &&\ngit_env_bool(\"GIT_FORCE_TTY_DUMB\", 0)))\n                return NULL;\n\n        pager = getenv(\"GIT_PAGER\");\n\n\nAvishay\n"},{"id":"428435","messageId":"87pmwazygf.fsf@evledraar.gmail.com","threadId":"55992","inReplyTo":"CAJ-0Osy9JhGD0=6eF3jgZuoHJEzymksCWZZZC+A4FtHxzOrdhA@mail.gmail.com","subject":"Re: Forcing git to use a pager without a tty","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-24T22:10:49Z","receivedAt":"2021-06-24T22:31:51Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jun 24 2021, Avishay Matayev wrote:\n\nIt's interesting that your original mail didn't end up in our main\narchive:\nhttps://lore.kernel.org/git/CAJ-0OswsrnAuCwU6U=S2i1qKkg=66U-8RHSGqD2kh9T_30Yw9w@mail.gmail.com/,\nbut e.g. marc.info has it (and it's in my mailbox):\nhttps://marc.info/?l=git&m=162440160200930\n\n> On Wed, 23 Jun 2021 at 12:29, Phillip Wood <phillip.wood123@gmail.com> wrote:\n>>\n>> On 23/06/2021 10:05, Phillip Wood wrote:\n>> > Hi Avishay\n>> >\n>> > On 22/06/2021 23:40, Avishay Matayev wrote:\n>> >> Hi there!\n>> >>\n>> >> Fugitive[1] is a vim plugin that wraps git and many of its commands\n>> >> into the editor in a really awesome way, I won't meddle into it too\n>> >> much as you can read about it in its README, but as you understand, it\n>> >> uses git, a lot.\n>> >>\n>> >> Some git commands use a pager, which is usually a program that needs a\n>> >> pty to function properly (`less`, for example). Fugitive can't really\n>> >> use a pty for the pager as vim runs its subprocesses without a pty.\n>> >> Therefore Fugitive just creates its own pager (which is a simple\n>> >> window in vim) and pastes the git command output there.\n>> >\n>> > If I understand correctly fugitive is reading the output of the git\n>> > command over a pipe and putting it into a vim buffer?\n>> >\n> AFAIK, no. It could read the output through vim's jobstart API (that calls a\n> callback when there's activity on stdout for example) or by\n> redirecting the output\n> to a temporary file and reading it.\n\n...\n\n>> >> The only problem left is that Fugitive can't reliably know when git\n>> >> decides to use the pager, for example `git reflog show` does raise the\n>> >> pager while `git reflog expire` does not. Fugitive currently maintains\n>> >> an (very possibly) incomplete list of commands that need a pager but\n>> >> maintaining it manually isn't ideal.\n>> >\n>> > I don't understand, if as you say above there isn't a pty then git wont\n>> > use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem\n>> > to,\n>>\n>> Sorry that is confused. GIT_PAGER_IN_USE only causes git to act as if a\n>> pager is being used (for example so it colors things as if it was\n>> outputting to a tty), it does not invoke the pager\n>>\n>>   so I'm not sure what you mean by 'Fugitive can't reliably know when\n>> > git decides to use the pager'\n>>\n>> I'm still confused by this - if there is no tty git wont use a pager.\n>>\n> That's the problem, git doesn't tell whoever calls it if it is going\n> to use the pager,\n> and that information is useful for Fugitive. Why? Because even if\n> fugitive just naively\n> reads git's output, it doesn't know whether it needs to open a pager.\n>\n> i.e If the user calls `git log`, the log is printed through the pager.\n> However, if a user\n> calls the exact same command through Fugitive. Fugitive needs to know\n> ahead if the\n> resulting command is going to need a pager (AKA a new window in vim) and prepare\n> accordingly. However, since git does not tell us when it's going to\n> open a page, we\n> have to book-keep commands and whether they are going to use the pager or not.\n\nHaving read this thread it feels as though there's a missing description\nbetween the 2nd and 3rd paragraphs of your original mail.\n\nI.e. you never really explicitly said what you're going to use this for,\nand why it's needed, but I think I get it now.\n\nThe \"straighforward\" thing to do would be to just capture stderr/stdout,\nwhich is what e.g. Magit does, which I gather is a similar thing to this\n\"Fugitive\" thing, except for Emacs (I use Magit heavily, I've only spent\n~5m browsing the Fugitive page for the first time, skipping through a\ncouple of screencasts).\n\nBut what you're doing in this editor plugin is assigning semantic\nimportance to if and when git uses a pager. If it would you open a\ndedicated window with the output, if it doesn't you presumably show it\ndifferently, just the exit code, in some \"raw git output\" buffer or\nsomething?\n\n> 1. https://github.com/tpope/vim-fugitive/blob/79e2bd381ad6e7f3d70ce4a8ec9f3f107b40f053/autoload/fugitive.vim#L2951-L2956\n>\n> This is the current proposal:\n>\n> --- a/pager.c\n> +++ b/pager.c\n> @@ -45,7 +45,8 @@ const char *git_pager(int stdout_is_tty)\n>  {\n>         const char *pager;\n>\n> -       if (!stdout_is_tty)\n> +\n> +       if (!stdout_is_tty && !(is_terminal_dumb() &&\n> git_env_bool(\"GIT_FORCE_TTY_DUMB\", 0)))\n>                 return NULL;\n>\n>         pager = getenv(\"GIT_PAGER\");\n\nSo yes, I could see how this would be useful, and FWIW I've got nothing\nagainst such a thing, it just took me a while to understand what you\nwere going for, and maybe I still don't get it.\n\nI will say that I think this approach is /probably/ not a very fruitful\none in the longer term for an editor plugin.\n\nE.g. in Magit everything we'd invoke a pager for such as \"log\", \"blame\"\netc. is a dedicated top-level command, Magit heavily modifies the UI, so\ne.g. for the \"log\" equivalent you can \"click\" on commits, fold them and\ntheir diffs etc.\n\nSo \"does it use a pager?\" nonwithstanding I'd think any sufficiently\nmature editor plugin would end up having to maintain its own list of\n\"this is an interesting command, and this is how I make use of its\nanyway. So the equivalent of piping what we'd send to the pager to\nanother FD (or setting a flag or whatever) wouldn't be very useful in\nthe end.\n\nBut hey, if it works for you why not :)\n\nWhat do you do about things like \"git status\" etc. that produce a lot of\noutput, but don't invoke the pager even then (but perhaps should), ditto\nfor all the plumbing or plumbing-like such as \"ls-files\", \"ls-tree\" etc?\n\n"},{"id":"428490","messageId":"60d6158fcef74_cc8d20834@natae.notmuch","threadId":"55992","inReplyTo":"CAJ-0Oswry0Rry_0ZqZzzVGuzfU-apZnv95LcxQEhm7GVA=uOxA@mail.gmail.com","subject":"Re: Forcing git to use a pager without a tty","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-25T17:42:39Z","receivedAt":"2021-06-25T17:42:44Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Avishay Matayev wrote:\n> On Wed, 23 Jun 2021 at 04:57, Felipe Contreras\n> <felipe.contreras@gmail.com> wrote:\n\n> > Seems like a completely sensible request to me, except I would do it a\n> > slightly different place:\n> >\n> > --- a/pager.c\n> > +++ b/pager.c\n> > @@ -101,7 +101,7 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)\n> >\n> >  void setup_pager(void)\n> >  {\n> > -       const char *pager = git_pager(isatty(1));\n> > +       const char *pager = git_pager(git_env_bool(\"GIT_PAGER_FORCE\", isatty(1)));\n> >\n> >         if (!pager)\n> >                 return;\n> >\n> > I'm Cc'ing Jonathan Nieder who seems to have toched these lines.\n\n> \n> Well I'm glad to hear you find it sensible, though your patch suggests\n> that `GIT_PAGER_FORCE` will be a boolean, while I expect it to be an\n> actual pager, which would make more sense IMO.\n\nI just copied the idea from the GitHub issue, I find it simple and easy\nto implement in the code this way. A GIT_PAGER_FORCE as you suggest\nwould be a bit more complicated.\n\nEither way feel free to provide a patch with whatever approach you think\nis best.\n\nHowever, for a proper patch you need to add documentation too (see\nDocumentation/git-var.txt), and preferably some tests.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"428517","messageId":"CAJ-0Osz-1PPQR-_k3_Qf8e0hJ8c43XbK3tzS=rVsvgFfk83ptA@mail.gmail.com","threadId":"55992","inReplyTo":"87pmwazygf.fsf@evledraar.gmail.com","subject":"Re: Forcing git to use a pager without a tty","fromName":"Avishay Matayev","fromEmail":"me@avishay.dev","sentAt":"2021-06-26T10:26:35Z","receivedAt":"2021-06-26T10:27:08Z","isPatch":false,"sender":{"key":"me@avishay.dev","avatar":null},"body":"On Fri, 25 Jun 2021 at 01:31, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>\n>\n> On Thu, Jun 24 2021, Avishay Matayev wrote:\n>\n> It's interesting that your original mail didn't end up in our main\n> archive:\n> https://lore.kernel.org/git/CAJ-0OswsrnAuCwU6U=S2i1qKkg=66U-8RHSGqD2kh9T_30Yw9w@mail.gmail.com/,\n> but e.g. marc.info has it (and it's in my mailbox):\n> https://marc.info/?l=git&m=162440160200930\n>\nI guess the empty subject tripped it somehow, oh well..\n> > On Wed, 23 Jun 2021 at 12:29, Phillip Wood <phillip.wood123@gmail.com> wrote:\n> >>\n> >> On 23/06/2021 10:05, Phillip Wood wrote:\n> >> > Hi Avishay\n> >> >\n> >> > On 22/06/2021 23:40, Avishay Matayev wrote:\n> >> >> Hi there!\n> >> >>\n> >> >> Fugitive[1] is a vim plugin that wraps git and many of its commands\n> >> >> into the editor in a really awesome way, I won't meddle into it too\n> >> >> much as you can read about it in its README, but as you understand, it\n> >> >> uses git, a lot.\n> >> >>\n> >> >> Some git commands use a pager, which is usually a program that needs a\n> >> >> pty to function properly (`less`, for example). Fugitive can't really\n> >> >> use a pty for the pager as vim runs its subprocesses without a pty.\n> >> >> Therefore Fugitive just creates its own pager (which is a simple\n> >> >> window in vim) and pastes the git command output there.\n> >> >\n> >> > If I understand correctly fugitive is reading the output of the git\n> >> > command over a pipe and putting it into a vim buffer?\n> >> >\n> > AFAIK, no. It could read the output through vim's jobstart API (that calls a\n> > callback when there's activity on stdout for example) or by\n> > redirecting the output\n> > to a temporary file and reading it.\n>\n> ...\nI might have taken the word 'pipe' too literally :)\n>\n> >> >> The only problem left is that Fugitive can't reliably know when git\n> >> >> decides to use the pager, for example `git reflog show` does raise the\n> >> >> pager while `git reflog expire` does not. Fugitive currently maintains\n> >> >> an (very possibly) incomplete list of commands that need a pager but\n> >> >> maintaining it manually isn't ideal.\n> >> >\n> >> > I don't understand, if as you say above there isn't a pty then git wont\n> >> > use a pager unless GIT_PAGER_IN_USE is set which Fugitive does not seem\n> >> > to,\n> >>\n> >> Sorry that is confused. GIT_PAGER_IN_USE only causes git to act as if a\n> >> pager is being used (for example so it colors things as if it was\n> >> outputting to a tty), it does not invoke the pager\n> >>\n> >>   so I'm not sure what you mean by 'Fugitive can't reliably know when\n> >> > git decides to use the pager'\n> >>\n> >> I'm still confused by this - if there is no tty git wont use a pager.\n> >>\n> > That's the problem, git doesn't tell whoever calls it if it is going\n> > to use the pager,\n> > and that information is useful for Fugitive. Why? Because even if\n> > fugitive just naively\n> > reads git's output, it doesn't know whether it needs to open a pager.\n> >\n> > i.e If the user calls `git log`, the log is printed through the pager.\n> > However, if a user\n> > calls the exact same command through Fugitive. Fugitive needs to know\n> > ahead if the\n> > resulting command is going to need a pager (AKA a new window in vim) and prepare\n> > accordingly. However, since git does not tell us when it's going to\n> > open a page, we\n> > have to book-keep commands and whether they are going to use the pager or not.\n>\n> Having read this thread it feels as though there's a missing description\n> between the 2nd and 3rd paragraphs of your original mail.\n>\n> I.e. you never really explicitly said what you're going to use this for,\n> and why it's needed, but I think I get it now.\n>\n> The \"straighforward\" thing to do would be to just capture stderr/stdout,\n> which is what e.g. Magit does, which I gather is a similar thing to this\n> \"Fugitive\" thing, except for Emacs (I use Magit heavily, I've only spent\n> ~5m browsing the Fugitive page for the first time, skipping through a\n> couple of screencasts).\n>\n> But what you're doing in this editor plugin is assigning semantic\n> importance to if and when git uses a pager. If it would you open a\n> dedicated window with the output, if it doesn't you presumably show it\n> differently, just the exit code, in some \"raw git output\" buffer or\n> something?\n>\nNo, there's no semantic importance. If a command has semantic\nimportance, Fugitive will act appropriately. We want to know whether\ngit is going to use a pager to provide *correct* behavior for the most\nsimple cases, which is either open a pager with the output, or don't.\n> > 1. https://github.com/tpope/vim-fugitive/blob/79e2bd381ad6e7f3d70ce4a8ec9f3f107b40f053/autoload/fugitive.vim#L2951-L2956\n> >\n> > This is the current proposal:\n> >\n> > --- a/pager.c\n> > +++ b/pager.c\n> > @@ -45,7 +45,8 @@ const char *git_pager(int stdout_is_tty)\n> >  {\n> >         const char *pager;\n> >\n> > -       if (!stdout_is_tty)\n> > +\n> > +       if (!stdout_is_tty && !(is_terminal_dumb() &&\n> > git_env_bool(\"GIT_FORCE_TTY_DUMB\", 0)))\n> >                 return NULL;\n> >\n> >         pager = getenv(\"GIT_PAGER\");\n>\n> So yes, I could see how this would be useful, and FWIW I've got nothing\n> against such a thing, it just took me a while to understand what you\n> were going for, and maybe I still don't get it.\n>\n> I will say that I think this approach is /probably/ not a very fruitful\n> one in the longer term for an editor plugin.\n>\nThe main problem this change solves is overriding git's decisiveness\non how to act in certain situations. e.g. If no pty is present, git won't\nopen the pager, i.e git will never invoke the command that is inside\nthe PAGER environment variable. If no pty is present, git won't report\nits progress on a `git push` through a progress bar.\n\nForcing git to always act like it has a pty will allow fugitive to present\nthese 'advanced' features to its users.\n> E.g. in Magit everything we'd invoke a pager for such as \"log\", \"blame\"\n> etc. is a dedicated top-level command, Magit heavily modifies the UI, so\n> e.g. for the \"log\" equivalent you can \"click\" on commits, fold them and\n> their diffs etc.\n>\n> So \"does it use a pager?\" nonwithstanding I'd think any sufficiently\n> mature editor plugin would end up having to maintain its own list of\n> \"this is an interesting command, and this is how I make use of its\n> anyway. So the equivalent of piping what we'd send to the pager to\n> another FD (or setting a flag or whatever) wouldn't be very useful in\n> the end.\n>\nYes, but not every command is an interesting one. You don't need a\nspecial window for every command, and some commands don't even\nhave any output besides their return value.\n> But hey, if it works for you why not :)\n>\n> What do you do about things like \"git status\" etc. that produce a lot of\n> output, but don't invoke the pager even then (but perhaps should), ditto\n> for all the plumbing or plumbing-like such as \"ls-files\", \"ls-tree\" etc?\n>\n`Git status` (or just `Git`) is special because it opens the fugitive status\nbuffer (just like the one Magit has).\n\n`git ls-files | ls-tree` just create an output window that shows the output\nof the command, that window disappears on any keystroke.\n\n\nI'd also like to add that I am just a regular user of the plugin, I've never\ncommitted anything to it and I just wanted to solve an issue that\nbothered me, so what I write here's from the perspective of a user (and\nsomeone who can read code), not from the view of a maintainer or\nthe designer of Fugitive.\n\nAvishay\n"},{"id":"428518","messageId":"CAJ-0OszRbZYe6RbgKmEG=38FzrcfOezpYKtYRSLT0bokaLRZPA@mail.gmail.com","threadId":"55992","inReplyTo":"60d6158fcef74_cc8d20834@natae.notmuch","subject":"Re: Forcing git to use a pager without a tty","fromName":"Avishay Matayev","fromEmail":"me@avishay.dev","sentAt":"2021-06-26T10:28:05Z","receivedAt":"2021-06-26T10:28:21Z","isPatch":false,"sender":{"key":"me@avishay.dev","avatar":null},"body":"On Fri, 25 Jun 2021 at 20:42, Felipe Contreras\n<felipe.contreras@gmail.com> wrote:\n> Either way feel free to provide a patch with whatever approach you think\n> is best.\n>\n> However, for a proper patch you need to add documentation too (see\n> Documentation/git-var.txt), and preferably some tests.\n>\n> Cheers.\n>\n> --\n> Felipe Contreras\nI will do so, thank you.\n\nAvishay\n"},{"id":"428716","messageId":"xmqqlf6tyzfy.fsf@gitster.g","threadId":"55992","inReplyTo":"87pmwazygf.fsf@evledraar.gmail.com","subject":"Re: Forcing git to use a pager without a tty","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-29T06:09:21Z","receivedAt":"2021-06-29T06:09:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> What do you do about things like \"git status\" etc. that produce a lot of\n> output, but don't invoke the pager even then (but perhaps should), ditto\n> for all the plumbing or plumbing-like such as \"ls-files\", \"ls-tree\" etc?\n\nI am puzzled.  Doesn't \"git -p status\" force spawning the pager?\n\n    ... goes and checks ...\n\nAhh, OK, -p does force us to try to see if pager is warranted, and\nthen we decide it is not because the output is not sent to the tty.\n\nThis behaviour makes sense if we are truly driving a pager via\nGIT_PAGER, but I agree with you that it probably is inconvenient if\nyou are abusing the GIT_PAGER mechanism to drive something that is\nnot about paging output but about grabbing the output and\npost-processing it.\n"}]}