git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Avishay Matayev <me@avishay.dev>
Cc: phillip.wood@dunelm.org.uk, git@vger.kernel.org, gitster@pobox.com
Subject: Re: Forcing git to use a pager without a tty
Date: Fri, 25 Jun 2021 00:10:49 +0200	[thread overview]
Message-ID: <87pmwazygf.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CAJ-0Osy9JhGD0=6eF3jgZuoHJEzymksCWZZZC+A4FtHxzOrdhA@mail.gmail.com>


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?


  reply	other threads:[~2021-06-24 22:31 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2021-06-26 10:26         ` Avishay Matayev
2021-06-29  6:09         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87pmwazygf.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@avishay.dev \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).