On Fri, 25 Jun 2021 at 01:31, Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> 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@xxxxxxxxxxxxxx/, > 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@xxxxxxxxx> 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
