Mercurial should not enable pager by default

[jdemeyer]

The Mercurial spkg (#10594) created a file $SAGE_LOCAL/etc/mercurial/hgrc enabling the "pager" extension. This is very bad for scripts using Mercurial and should be disabled.

[kini]

Please explain what is very bad about this. Any problems caused by the pager extension being enabled in $SAGE_LOCAL/etc/mercurial/hgrc would equally be caused by the pager extension being enabled in $HOME/.hgrc, which you will agree is not under our control. Furthermore we do not actually set a pager, so by default the pager extension doesn't even do anything, other than enable the command line option --pager=. Even if we did set a pager, it is set to only apply to the commands annotate, cat, diff, log, glog, and qdiff. I question why one would need to use these in a script in the first place, as they are by nature interactive commands. And even if for some reason you must use them, you can always run them with --pager=off (and indeed you should do this anyway).

This was introduced by #11121, not #10594 - please see the comments there for more discussion about this. See also #11142.

[kini]

As Volker pointed out on sage-devel, an even better way to avoid this is to set the HGPLAIN environment variable.

[jdemeyer]

Replying to kini:

Please explain what is very bad about this. Any problems caused by the pager extension being enabled in $SAGE_LOCAL/etc/mercurial/hgrc would equally be caused by the pager extension being enabled in $HOME/.hgrc, which you will agree is not under our control.

True, but $SAGE_LOCAL/etc/mercurial/hgrc is not under my control. I personally do not want a pager to be enabled, so I do not enable the pager extension in my .hgrc. It's true that I have the environment variable PAGER set, but that is used for many things (like e.g. EDITOR), so I don't want to disable it just for Sage.

Even if we did set a pager, it is set to only apply to the commands annotate, cat, diff, log, glog, and qdiff. I question why one would need to use these in a script in the first place, as they are by nature interactive commands.

There are 6 occurences of hg diff in the $SAGE_ROOT/local/bin/* scripts, so I disagree.

This was introduced by #11121, not #10594

Wrong, this was introduced in #10594. It is the Mercurial spkg which writes $SAGE_LOCAL/etc/mercurial/hgrc. On #11121 I have made a comment that I did not want the pager extension to be enabled by default.

As Volker pointed out on sage-devel, an even better way to avoid this is to set the HGPLAIN environment variable.

True, but I think that is an orthogonal issue. Even if we do use HGPLAIN in scripts, I think we still should not enable the pager extension by default.

[kini]

Replying to jdemeyer:

Even if we did set a pager, it is set to only apply to the commands annotate, cat, diff, log, glog, and qdiff. I question why one would need to use these in a script in the first place, as they are by nature interactive commands.

There are 6 occurences of hg diff in the $SAGE_ROOT/local/bin/* scripts, so I disagree.

Yes, and all six of them are mysterious and seem unnecessary to me.

This was introduced by #11121, not #10594

Wrong, this was introduced in #10594. It is the Mercurial spkg which writes $SAGE_LOCAL/etc/mercurial/hgrc. On #11121 I have made a comment that I did not want the pager extension to be enabled by default.

The spkg on #10594 fixes #10594, #11121, and #11120 in three separate commits to the mercurial repository in the spkg. There is zero discussion of pagers on the trac ticket for #10594, whereas there is substantial discussion of it on #11121. So I think it's fair to say that this was introduced by #11121, and not by #10594. But this is immaterial I suppose.

As Volker pointed out on sage-devel, an even better way to avoid this is to set the HGPLAIN environment variable.

True, but I think that is an orthogonal issue. Even if we do use HGPLAIN in scripts, I think we still should not enable the pager extension by default.

Do you propose to also revert #11142, which explicitly uses the pager extension?

True, but $SAGE_LOCAL/etc/mercurial/hgrc is not under my control. I personally do not want a pager to be enabled, so I do not enable the pager extension in my .hgrc. It's true that I have the environment variable PAGER set, but that is used for many things (like e.g. EDITOR), so I don't want to disable it just for Sage.

Just to be clear, I think it was a bad idea, from an overall software design perspective, to entangle Mercurial with Sage, create wrappers for it in the Sage library, etc. in the first place. Maybe it was even a mistake to bundle Mercurial at all. But one of the main goals behind the way Sage is set up is to lower the barrier of entry and make things easier for beginner developers, and I think part of that goal involves making sage -hg as user-friendly as possible. The functionality implemented by jhpalmieri at #11142 is part of this and requires the pager extension. So too is making sage -hg understand your $PAGER variable and show things with ANSI colors.

In short, it seems to me that sage -hg is meant to be an easy tool for those who don't care about fine tuning their Mercurial. To my mind, Sage is a mathematical software distribution, and the real reason that Mercurial is distributed with it is not that it gives the user a Mercurial installation, but rather that it 1) allows Sage to use Mercurial to do various things with its scripts; and 2) allows you to use Mercurial from the command line or from the Sage prompt to do some minimal amount of actions that are necessary to contribute to Sage development. For more advanced things, or if you desire more control, you are free to use your own system Mercurial in whatever way you like - I certainly don't use sage -hg myself in interactive shells. And if we are using $HGPLAIN in scripts, the only objection remaining, as far as I can see, is the lack of control over the behavior of sage -hg on the command line, which I don't think is a legitimate issue, assuming the above is true.

Also, I don't think this even really needs mentioning, but if there is really someone out there who cares so much, they can manually turn off the pager extension in their ~/.hgrc by setting pager = !. But I gather you are not one of these people, as you say on sage-devel that you don't use sage -hg anyway.

---

Now, maybe you are of the opinion that all what I said above is totally wrong, and that Sage is supposed to be shipping Mercurial as a standalone application, just like it ships GAP, Python, etc., and that you should be able to use sage -hg just like you would use hg. Honestly, I could agree with that. That would make more sense with Burcin's prefix system, for example, which I hope one day Sage will eventually ship with.

But in that case most of #11121 doesn't make any sense anyway and most of $SAGE_LOCAL/etc/mercurial/hgrc should be deleted (except the first two lines). What do you think? I am fine with that, if that's what people generally agree upon. In that case I would request that you start rejecting patches that are not in git format, though, as that was the original (and important) purpose of #11121. Luckily the patches on #11121 (i.e. not the commit in the spkg) mostly solve that problem.

[jdemeyer]

Replying to kini:

Replying to jdemeyer:

Even if we did set a pager, it is set to only apply to the commands annotate, cat, diff, log, glog, and qdiff. I question why one would need to use these in a script in the first place, as they are by nature interactive commands.

There are 6 occurences of hg diff in the $SAGE_ROOT/local/bin/* scripts, so I disagree.

Yes, and all six of them are mysterious and seem unnecessary to me.

Maybe you are right, but I do think that hg diff can be useful in scripts sometimes, and hg log also.

True, but I think that is an orthogonal issue. Even if we do use HGPLAIN in scripts, I think we still should not enable the pager extension by default.

Do you propose to also revert #11142, which explicitly uses the pager extension?

No, I think using the pager within Sage is fine, analogously to how "is_prime?" runs a pager. In Sage, there is no simple way to explicitly run a pager (in the shell, it's easy to type "hg diff |less")

Just to be clear, I think it was a bad idea, from an overall software design perspective, to entangle Mercurial with Sage, create wrappers for it in the Sage library, etc. in the first place. Maybe it was even a mistake to bundle Mercurial at all.

I don't think it's a bad idea to bundle Mercurial. I actually like how the Sage scripts like sage-sdist use Mercurial.

But one of the main goals behind the way Sage is set up is to lower the barrier of entry and make things easier for beginner developers, and I think part of that goal involves making sage -hg as user-friendly as possible.

Okay, agreed.

But in that case most of #11121 doesn't make any sense anyway and most of $SAGE_LOCAL/etc/mercurial/hgrc should be deleted (except the first two lines). What do you think? I am fine with that, if that's what people generally agree upon. In that case I would request that you start rejecting patches that are not in git format, though, as that was the original (and important) purpose of #11121. Luckily the patches on #11121 (i.e. not the commit in the spkg) mostly solve that problem.

Well, I think the "git" default is good to have, also the encoding being utf-8 (through HGENCODING set in sage-env). I never argued about that, only about the pager.

[jdemeyer]

I changed my mind about HGPLAIN: maybe it is the best solution.

[vbraun]

If you call mercurial from within sage ('hg-sage' and friends) to diff then we pass --config pager.pager=... in sage/misc/hg.py. So whether or not the pager is enabled by default in the hgrc is just a matter of taste.

My vote would be:

• Use HGPLAIN in scripts if necessary.
• Keep the pager extension to intentionally break scripts that forget to set HGPLAIN.

[kini]

Replying to vbraun:

If you call mercurial from within sage ('hg-sage' and friends) to diff then we pass --config pager.pager=... in sage/misc/hg.py. So whether or not the pager is enabled by default in the hgrc is just a matter of taste.

The point is that --config pager.pager=... won't work unless the pager extension is enabled. As far as I can tell there is no way to enable an extension on the fly from a command line call to Mercurial, which makes sense since extensions can have a rather deep effect on Mercurial's operation. In the hgrc, no pager program is defined, which is a different matter (and is already what I think we all agree is better than defining a pager program there).

My vote would be:

• Use HGPLAIN in scripts if necessary.
• Keep the pager extension to intentionally break scripts that forget to set HGPLAIN.

Sounds great to me. Keep in mind that the pager extension will not break scripts that forget to set $HGPLAIN unless $PAGER is also set while the script is run, or pager.pager is set to something in some hgrc somewhere.

[jdemeyer]

Proof-of-concept patch using HGPLAIN. Note that I replaced sage -hg by hg in all scripts, hoping that SAGE_ROOT/local/bin is in the path.

[vbraun]

Looks good to me!

[kini]

Replying to jdemeyer:

Note that I replaced sage -hg by hg in all scripts, hoping that SAGE_ROOT/local/bin is in the path.

This is a good idea. If it doesn't work then something is broken and needs to be fixed.

[kini]

Replying to jdemeyer:

Well, I think the "git" default is good to have

Sure, me too, but it also takes control away from the user, just like turning the pager extension on. The patches on #11121 which are not in the spkg cause sage to put .hg/.hgrc files in the Sage mercurial repositories themselves. This means that sage -hg will produce git-style patches when operating on a Sage repository, but not by default when operating on any other random repository on your system. So it's a more "vanilla" approach to leave those patches from #11121 intact and remove the git stuff from $SAGE_LOCAL/etc/mercurial/hgrc. Just saying, in case we eventually decide to roll back #11121 and rewrite #11142.

(The first two lines of $SAGE_LOCAL/etc/mercurial/hgrc are still necessary to make the previously mentioned .hgrc files be accepted by a Mercurial running under a different user from whoever installed the Sage repos - another indication that maybe version control should not be integrated into the installation of a package, but should rather be managed by the user in user-owned directories, but that's another story.)

[kcrisman]

BAD BAD BAD news for this on systems with nothing installed. Here is what I see with 4.7.2.

changeset:   16189:9e29a3d84c48
tag:         tip
user:        Jeroen Demeyer <jdemeyer@cage.ugent.be>
date:        Sat Oct 29 12:50:17 2011 +0000
summary:     4.7.2

changeset:   16188:0f6304794835
user:        Jeroen Demeyer <jdemeyer@cage.ugent.be>
date:        Sat Oct 29 09:36:26 2011 +0000
summary:     Added tag 4.7.2 for changeset 55f2efa29bab

changeset:   16187:55f2efa29bab
tag:         4.7.2
user:        Jeroen Demeyer <jdemeyer@cage.ugent.be>
date:        Sat Oct 29 09:36:25 2011 +0000
summary:     Added tag 4.7.2.rc1 for changeset acc90fc8ee5d

changeset:   16186:acc90fc8ee5d
tag:         4.7.2.rc1
user:        Jeroen Demeyer <jdemeyer@cage.ugent.be>
date:        Sat Oct 29 09:36:22 2011 +0000
summary:     Added tag 4.7.2.rc0 for changeset af169997d6c8

:

But with 4.8.alpha5 (i.e., after this patch):

changeset:   3:6ee487e4d475
user:        tornaria@math.utexas.edu
date:        Sat Feb 11 21:39:44 2006 +0000
summary:     [project @ patch to sage-1.0.0.1]

changeset:   2:a572d77184a3
user:        tornaria@math.utexas.edu
date:        Sat Feb 11 21:36:43 2006 +0000
summary:     [project @ patch to sage-1.0.0]

changeset:   1:bf5417bc8931
user:        tornaria@math.utexas.edu
date:        Sat Feb 11 21:33:33 2006 +0000
summary:     [project @ patch to sage-0.10.13]

changeset:   0:039f6310c6fe
user:        tornaria@math.utexas.edu
date:        Sat Feb 11 01:13:08 2006 +0000
summary:     [project @ original sage-0.10.12]

after a lot of scrolling. No surprise, perhaps.

The whole point of hg_sage is for it to be EASY for people to use. Now it is not. I'm not quite sure what happened in this ticket, and in general the changes of late with Mercurial and paging seemed ok, but this did something very different.

[jhpalmieri]

Oh, I think I see. See #12288 for a followup.

[jdemeyer]

I wouldn't want to revert this ticket, because it would imply reverting #10594. Sage could unset the environment variable HGPLAIN or explicitly run less. This would be a fairly easy fix. I don't have the energy to write or review such a patch myself though.

[kcrisman]

Replying to jdemeyer:

I wouldn't want to revert this ticket, because it would imply reverting #10594.

Not at all, just wanting to point this out and that it's fairly important. I'll comment further at #12288 if necessary. Thank goodness for long alpha (=beta, soon) cycles!