Since git 1.5.6, `git status` always invokes a pager, which is really annoying when the output is less than the number of terminal rows available. Can I turn that off somehow or do I need to send a reverting patch?
Jan
threads / discuss / 14075
Subject: about c8af1de9 (git status uses pager)
Since git 1.5.6, `git status` always invokes a pager, which is really annoying when the output is less than the number of terminal rows available. Can I turn that off somehow or do I need to send a reverting patch?
Jan
Re: about c8af1de9 (git status uses pager)
On Sat, Jun 21, 2008 at 11:21 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
> > Since git 1.5.6, `git status` always invokes a pager, which is really > annoying when the output is less than the number of terminal rows > available. Can I turn that off somehow or do I need to send a reverting > patch?
I think it would work to set PAGER="less -F" (a.k.a. --quit-if-one-screen)?
There's also GIT_PAGER variable, core.pager git setting, etc.
Vegard
-- "The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Re: about c8af1de9 (git status uses pager)
"Vegard Nossum" <vegard.nossum@gmail.com> writes:
> On Sat, Jun 21, 2008 at 11:21 PM, Jan Engelhardt <jengelh@medozas.de> wrote: >> >> Since git 1.5.6, `git status` always invokes a pager, which is really >> annoying when the output is less than the number of terminal rows >> available. Can I turn that off somehow or do I need to send a reverting >> patch? > > I think it would work to set PAGER="less -F" (a.k.a. --quit-if-one-screen)?
Probably better with LESS=FRSX, which is what git uses as a sane default if nothing is set.
Re: about c8af1de9 (git status uses pager)
On Saturday 2008-06-21 23:42, Junio C Hamano wrote:
>"Vegard Nossum" <vegard.nossum@gmail.com> writes: > >> On Sat, Jun 21, 2008 at 11:21 PM, Jan Engelhardt <jengelh@medozas.de> wrote: >>> >>> Since git 1.5.6, `git status` always invokes a pager, which is really >>> annoying when the output is less than the number of terminal rows >>> available. Can I turn that off somehow or do I need to send a reverting >>> patch? >> >> I think it would work to set PAGER="less -F" (a.k.a. --quit-if-one-screen)? > >Probably better with LESS=FRSX, which is what git uses as a sane default >if nothing is set. >
I went with Vegard's suggestion to change the pager command in ~/.gitconfig, since I have the $LESS environment variable already defined as "-MSi", and I do not want to change that; because if I am going to run less (often at the end of a pipe), I certainly do not want it to just quit on me. So -F in $LESS is a no-no.
Since I need "-MRSi" for git anyhow, tweaking ~/.gitconfig was easy.
Re: about c8af1de9 (git status uses pager)
On Saturday 2008-06-21 23:45, Jan Engelhardt wrote:
>On Saturday 2008-06-21 23:42, Junio C Hamano wrote: >>"Vegard Nossum" <vegard.nossum@gmail.com> writes: >> >>> On Sat, Jun 21, 2008 at 11:21 PM, Jan Engelhardt <jengelh@medozas.de> wrote: >>>> >>>> Since git 1.5.6, `git status` always invokes a pager, which is really >>>> annoying when the output is less than the number of terminal rows >>>> available. Can I turn that off somehow or do I need to send a reverting >>>> patch? >>> >>> I think it would work to set PAGER="less -F" (a.k.a. --quit-if-one-screen)? >> >>Probably better with LESS=FRSX, which is what git uses as a sane default >>if nothing is set. >> >I went with Vegard's suggestion to change the pager command in >~/.gitconfig, since I have the $LESS environment variable already >defined as "-MSi", and I do not want to change that; because if I am >going to run less (often at the end of a pipe), I certainly do not >want it to just quit on me. So -F in $LESS is a no-no. > >Since I need "-MRSi" for git anyhow, tweaking ~/.gitconfig was easy. >
I've played with `less -F` for a while now, and there is an inherent problem with that. Or should I say, xterm. When less quits, the xterm "screen" blits back to the shell output that was previously visible, so git status with less -F as a pager displays practically *nothing*. I am aware that this is a less / xterm issue, but I wanted to let you know.
Re: about c8af1de9 (git status uses pager)
Jan Engelhardt <jengelh@medozas.de> writes:
> I am aware that this is a less / xterm issue, but I wanted to let > you know.
Do you mean "me" by "let you know"? Well, I have already been aware of it, thanks.
Re: about c8af1de9 (git status uses pager)
Junio C Hamano <gitster@pobox.com> writes:
> Jan Engelhardt <jengelh@medozas.de> writes: > >> I am aware that this is a less / xterm issue, but I wanted to let >> you know. > > Do you mean "me" by "let you know"? Well, I have already been aware of > it, thanks.
Having said all that, I have to say I am regretting to have accepted that patch to enable pager on status, not because it bothers me personally (it doesn't primarily because I practically never run git-status because I consider the command useless and living almost always in Emacs helps), but because in principle changing anything that existing users are used to is bad.
Jeff had a patch to allow boolean configuration variable "pager.<command>" to override the built-in pager settings during 1.5.6 cycle, and I think it was a reasonable approach to take. People who want to page output from git-status can then set "pager.status = true" in their configuration (and then we can revert c8af1de (make git-status use a pager, 2008-04-23)). Alternatively we could keep the current status-quo for the default, and people can say "pager.status = false" in their configuration.
Re: about c8af1de9 (git status uses pager)
On Sun, Jun 22, 2008 at 03:01:07AM -0700, Junio C Hamano wrote:
> Jeff had a patch to allow boolean configuration variable "pager.<command>" > to override the built-in pager settings during 1.5.6 cycle, and I think it > was a reasonable approach to take. People who want to page output from > git-status can then set "pager.status = true" in their configuration (and > then we can revert c8af1de (make git-status use a pager, 2008-04-23)). > Alternatively we could keep the current status-quo for the default, and > people can say "pager.status = false" in their configuration.
I have been running with the patch for a month or two, and it works fine for controlling the pager. Unfortunately, there is a nasty interaction in the git wrapper with reading the config file early, and we end up not calculating the GIT_DIR and worktree in the same way. I think this is part of a larger problem which needs solving, but everytime I look at it, my eyes start bleeding (and I have to admit, since the patch does work, I have forgotten how annoyed I was at the paging behavior in the first place, and I don't have as much motivation to work on it).
I think this is deeply related to the "git config alias.st status && cd .git && git st" problem, which is also on my long-term todo. So I'll see if I can do something about it during this release cycle.
-Peff
Re: about c8af1de9 (git status uses pager)
Junio C Hamano(gitster@pobox.com)@220608-03:01:
> Having said all that, I have to say I am regretting to have accepted that > patch to enable pager on status, not because it bothers me personally (it > doesn't primarily because I practically never run git-status because I > consider the command useless and living almost always in Emacs helps), but > because in principle changing anything that existing users are used to is > bad. > > Jeff had a patch to allow boolean configuration variable "pager.<command>" > to override the built-in pager settings during 1.5.6 cycle, and I think it > was a reasonable approach to take. People who want to page output from > git-status can then set "pager.status = true" in their configuration (and > then we can revert c8af1de (make git-status use a pager, 2008-04-23)). > Alternatively we could keep the current status-quo for the default, and > people can say "pager.status = false" in their configuration.
I'd really like to see this. Setting core.pager to `less -FSRX` or similar is not useful for me - I *want* to have -X for eg. `git diff`, but I don't want paging at all for status.
This was quite a nasty change to sneak on people I think.
Tim
-- Tim Stoakes
Re: about c8af1de9 (git status uses pager)
On Thu, Jul 03, 2008 at 11:45:41AM +0930, Tim Stoakes wrote:
> > Jeff had a patch to allow boolean configuration variable "pager.<command>" > > to override the built-in pager settings during 1.5.6 cycle, and I think it > > was a reasonable approach to take. People who want to page output from > > git-status can then set "pager.status = true" in their configuration (and > > then we can revert c8af1de (make git-status use a pager, 2008-04-23)). > > Alternatively we could keep the current status-quo for the default, and > > people can say "pager.status = false" in their configuration. > > I'd really like to see this. Setting core.pager to `less -FSRX` or > similar is not useful for me - I *want* to have -X for eg. `git diff`, > but I don't want paging at all for status.
OK, here is a revised patch that actually passes the tests. I'm not incredibly happy with it (see the caveats below), but I think this is the best we can do without major surgery on the git_dir setup (there seems to be some nasty interaction between setup_git_env and setup_git_directory, but several attempts at obvious fixes have left me pulling my hair out).
-- >8 -- allow per-command pager config
There is great debate over whether some commands should set up a pager automatically. This patch allows individuals to set their own pager preferences for each command, overriding the default. For example, to disable the pager for git status:
git config pager.status false
If "--pager" or "--no-pager" is specified on the command line, it takes precedence over the config option.
There are two caveats:
- you can turn on the pager for plumbing commands.
Combined with "core.pager = always", this will probably
break a lot of things. Don't do it. - This only works for builtin commands. The reason is
somewhat complex: Calling git_config before we do setup_git_directory
has bad side effects, because it wants to know where
the git_dir is to find ".git/config". Unfortunately,
we cannot call setup_git_directory indiscriminately,
because some builtins (like "init") break if we do. For builtins, this is OK, since we can just wait until
after we call setup_git_directory. But for aliases, we
don't know until we expand (recursively) which command
we're doing. This should not be a huge problem for
aliases, which can simply use "--pager" or "--no-pager"
in the alias as appropriate. For external commands, however, we don't know we even
have an external command until we exec it, and by then
it is too late to check the config. An alternative approach would be to have a config mode
where we don't bother looking at .git/config, but only
at the user and system config files. This would make the
behavior consistent across builtins, aliases, and
external commands, at the cost of not allowing per-repo
pager config for at all.
---
git.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 47 insertions(+), 4 deletions(-)diff --git a/git.c b/git.c index 22ac522..426a17f 100644 --- a/git.c +++ b/git.c @@ -9,6 +9,43 @@ const char git_usage_string[] = const char git_more_info_string[] = "See 'git help COMMAND' for more information on a specific command."; +static int use_pager = -1; +struct pager_config { + const char *cmd; + int val; +}; + +static int pager_command_config(const char *var, const char *value, void *data) +{ + struct pager_config *c = data; + if (!prefixcmp(var, "pager.") && !strcmp(var + 6, c->cmd)) + c->val = git_config_bool(var, value); + return 0; +} + +/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */ +int check_pager_config(const char *cmd) +{ + struct pager_config c; + c.cmd = cmd; + c.val = -1; + git_config(pager_command_config, &c); + return c.val; +} + +static void commit_pager_choice(void) { + switch (use_pager) { + case 0: + setenv("GIT_PAGER", "cat", 1); + break; + case 1: + setup_pager(); + break; + default: + break; + } +} + static int handle_options(const char*** argv, int* argc, int* envchanged) { int handled = 0; @@ -38,9 +75,9 @@ static int handle_options(const char*** argv, int* argc, int* envchanged) exit(0); } } else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) { - setup_pager(); + use_pager = 1; } else if (!strcmp(cmd, "--no-pager")) { - setenv("GIT_PAGER", "cat", 1); + use_pager = 0; if (envchanged) *envchanged = 1; } else if (!strcmp(cmd, "--git-dir")) { @@ -242,8 +279,13 @@ static int run_command(struct cmd_struct *p, int argc, const char **argv) prefix = NULL; if (p->option & RUN_SETUP) prefix = setup_git_directory(); - if (p->option & USE_PAGER) - setup_pager(); + + if (use_pager == -1 && p->option & RUN_SETUP) + use_pager = check_pager_config(p->cmd); + if (use_pager == -1 && p->option & USE_PAGER) + use_pager = 1; + commit_pager_choice(); + if (p->option & NEED_WORK_TREE) setup_work_tree(); @@ -453,6 +495,7 @@ int main(int argc, const char **argv) argv++; argc--; handle_options(&argv, &argc, NULL); + commit_pager_choice(); if (argc > 0) { if (!prefixcmp(argv[0], "--")) argv[0] += 2;
-- 1.5.6.1.158.g8cb5.dirty
Re: about c8af1de9 (git status uses pager)
Hi,
On Thu, 3 Jul 2008, Tim Stoakes wrote:
> This [changing git status to use the pager automatically] was quite a > nasty change to sneak on people I think.
Well, I think that a command producing pages and pages of output without stopping is useless. Therefore, _I_ maintain that it makes tons of sense.
Of course, if you find enough support for a patch reverting it, I might have to accept the fact that "git status" becomes less useful for me.
Ciao, Dscho
Re: about c8af1de9 (git status uses pager)
El 3/7/2008, a las 14:11, Johannes Schindelin escribió:
> Hi, > > On Thu, 3 Jul 2008, Tim Stoakes wrote: > >> This [changing git status to use the pager automatically] was quite a >> nasty change to sneak on people I think. > > Well, I think that a command producing pages and pages of output > without > stopping is useless. Therefore, _I_ maintain that it makes tons of > sense.
Ditto. For me, the change was very welcome. Sure, usually the status output is short, but on those occasions where it isn't (big file reorganizations etc) I was quite sick of doing the "git status, oops that output was too long let's try again, git -p status" dance. It was also incongruous with my very first experiences with Git, where I had been pleasantly surprised that "git log" automatically invoked the pager whereas "svn log" just spewed output into my console until ^C.
Cheers, Wincent
Re: about c8af1de9 (git status uses pager)
On Thu, Jul 03, 2008 at 03:37:30PM +0200, Wincent Colaiuta wrote:
>>> This [changing git status to use the pager automatically] was quite a >>> nasty change to sneak on people I think. >> >> Well, I think that a command producing pages and pages of output >> without stopping is useless. Therefore, _I_ maintain that it makes >> tons of sense. > > Ditto. For me, the change was very welcome. Sure, usually the status > output is short, but on those occasions where it isn't (big file > reorganizations etc) I was quite sick of doing the "git status, oops > that output was too long let's try again, git -p status" dance. It was > also incongruous with my very first experiences with Git, where I had > been pleasantly surprised that "git log" automatically invoked the > pager whereas "svn log" just spewed output into my console until ^C.
I think there is not much point in discussing whether "git status" should use a pager. It is obvious at this point that some people love it, and some people despise it, and it seems to be a matter of personal preference. The only sane way forward seems to be configurable paging. So the next steps are:
- getting a configurable paging patch that is good enough; I just
posted another trial, but there are still a few caveats. I would
love to hear commentary on whether people find it acceptable. - once there is a patch, there is still the matter of "on by default,
make people configure off" versus "off by default, make people
configure on". And then you can make arguments about how the default
changes the new user experience, but hopefully there will be a
little less vehemence on both sides, since it is "here's what new
users should see" and not "here's what I am forced to see every
day".-Peff
Re: about c8af1de9 (git status uses pager)
El 3/7/2008, a las 21:08, Jeff King escribió:
> On Thu, Jul 03, 2008 at 03:37:30PM +0200, Wincent Colaiuta wrote: > >>>> This [changing git status to use the pager automatically] was >>>> quite a >>>> nasty change to sneak on people I think. >>> >>> Well, I think that a command producing pages and pages of output >>> without stopping is useless. Therefore, _I_ maintain that it makes >>> tons of sense. >> >> Ditto. For me, the change was very welcome. Sure, usually the status >> output is short, but on those occasions where it isn't (big file >> reorganizations etc) I was quite sick of doing the "git status, oops >> that output was too long let's try again, git -p status" dance. It >> was >> also incongruous with my very first experiences with Git, where I had >> been pleasantly surprised that "git log" automatically invoked the >> pager whereas "svn log" just spewed output into my console until ^C. > > I think there is not much point in discussing whether "git status" > should use a pager. It is obvious at this point that some people love > it, and some people despise it, and it seems to be a matter of > personal > preference. The only sane way forward seems to be configurable paging.
I completely agree. I just don't like it when people use words like "nasty" and "sneak" to describe a change that was discussed in one short and one long thread prior to being incorporated. But you're right; there's no sense in discussing the "whether" any more.
Cheers, Wincent
Re: about c8af1de9 (git status uses pager)
On Sun, Jun 22, 2008 at 11:09 AM, Jan Engelhardt <jengelh@medozas.de> wrote:
> I've played with `less -F` for a while now, and there is an inherent > problem with that. Or should I say, xterm. > When less quits, the xterm "screen" blits back to the shell output > that was previously visible, so > git status > with less -F as a pager displays practically *nothing*. > I am aware that this is a less / xterm issue, but I wanted to let > you know. >
Option -X should do the trick :-)
Vegard
-- "The animistic metaphor of the bug that maliciously sneaked in while the programmer was not looking is intellectually dishonest as it disguises that the error is the programmer's own creation." -- E. W. Dijkstra, EWD1036
Re: about c8af1de9 (git status uses pager)
On 21/06/08 23:21, Jan Engelhardt wrote:
> > Since git 1.5.6, `git status` always invokes a pager, which is really > annoying when the output is less than the number of terminal rows > available. Can I turn that off somehow or do I need to send a reverting > patch? >
Wow, I just noticed it myself. Why was that changed? I don't know about your status lines, but I for one find it really annoying. Anything that's in a pager isn't visible in my console afterwards. What's next? git branch in a pager too?
Regards, Jojo
-- Johannes Gilger <heipei@hackvalue.de> http://hackvalue.de/heipei/ GPG-Key: 0x42F6DE81 GPG-Fingerprint: BB49 F967 775E BB52 3A81 882C 58EE B178 42F6 DE81
Re: about c8af1de9 (git status uses pager)
On 21/06/08 23:42, Johannes Gilger wrote:
> Wow, I just noticed it myself. Why was that changed? I don't know about > your status lines, but I for one find it really annoying. Anything > that's in a pager isn't visible in my console afterwards. What's next? > git branch in a pager too?
Seems like I got ahead of myself there. After setting the core.pager to less -FRSX everything works fine ;)
Regards, Jojo
-- Johannes Gilger <heipei@hackvalue.de> http://hackvalue.de/heipei/ GPG-Key: 0x42F6DE81 GPG-Fingerprint: BB49 F967 775E BB52 3A81 882C 58EE B178 42F6 DE81
Re: about c8af1de9 (git status uses pager)
On 06/22/2008 05:01 AM, Junio C Hamano wrote:
>> Having said all that, I have to say I am regretting to have accepted that >> patch to enable pager on status, not because it bothers me personally (it >> doesn't primarily because I practically never run git-status because I >> consider the command useless and living almost always in Emacs helps), but >> because in principle changing anything that existing users are used to is >> bad.
I'm a bit late on the uptake here, but I wanted to throw my two cents in- this was one of the biggest usability flubs I've seen with git in a long time. I suspect (actually, I *know*) there were many users like I that used git-status all the time in the console to get a handle on what their work tree was looking like. That became impossible with 1.5.6 out of the box, and I have to spend an hour reading mailing list posts trying to restore the former non-broken behavior.
Why did this patch get pulled in with so little discussion? Didn't someone think that there must be a reason git-status didn't use a pager before?
On 06/22/2008 02:24 AM, Johannes Gilger wrote:
> On 21/06/08 23:42, Johannes Gilger wrote: >> Wow, I just noticed it myself. Why was that changed? I don't know about >> your status lines, but I for one find it really annoying. Anything >> that's in a pager isn't visible in my console afterwards. What's next? >> git branch in a pager too? > > Seems like I got ahead of myself there. After setting the core.pager to > less -FRSX everything works fine ;)
This is a terrible thing to force users to have to do. Not only that, but the interaction between GIT_PAGER, core.pager, and everything else just makes this change even more hard to swallow.
I probably came across as flame/troll here, but I felt I needed to speak up for those of us that never knew this change was coming. I would go so far as to say this should be reverted for a maint release.
-Dan
Re: about c8af1de9 (git status uses pager)
Dan McGee <dpmcgee@gmail.com> writes:
> That became impossible with 1.5.6 out of the box,
Actually, by default it does. It does not for you because you've set $LESS. That said, I argee that a config option to enable/disable the pager per-command would have been better that the change we're talking about.
-- Matthieu
Re: about c8af1de9 (git status uses pager)
El 26/6/2008, a las 5:53, Dan McGee escribió:
> Why did this patch get pulled in with so little discussion? Didn't > someone think that there must be a reason git-status didn't use a > pager before?
I believe it was discussed in at least a couple of threads back then:
Original patch in late April, 3 messages:
http://article.gmane.org/gmane.comp.version-control.git/80279/
Follow-up discussion in early May, 31 messages:
http://article.gmane.org/gmane.comp.version-control.git/80957/
Wincent
Re: about c8af1de9 (git status uses pager)
On Thu, Jun 26, 2008 at 12:17:53PM +0200, Wincent Colaiuta wrote:
> Follow-up discussion in early May, 31 messages: > > http://article.gmane.org/gmane.comp.version-control.git/80957/
And I posted several patches in that thread to make paging configuration more fine-grained, but they got caught up in some setup_git_* bermuda triangle. Dan (and others who dislike the new change), I would be very happy if you wanted to take a look at them and see if you can address some of the issues I raised.
-Peff