On Sat, 25 Feb 2017 23:32:12 -0500, Augie Fackler <r...@durin42.com> wrote:


On Feb 25, 2017, at 11:22 PM, Matt Harbison <mharbiso...@gmail.com> wrote:

On Thu, 16 Feb 2017 11:59:10 -0500, Augie Fackler <r...@durin42.com> wrote:

# HG changeset patch
# User Augie Fackler <au...@google.com>
# Date 1487198871 18000
#      Wed Feb 15 17:47:51 2017 -0500
# Node ID cd19a77477bcd18d1f7ab4fa73ee0dbf3c7a4e46
# Parent  791b4e846a7b9a0783440b9504585438777fe2d2
pager: move pager-initiating code into core

No functionality change.

A previous version of this API had a category argument on
ui.pager(). As I migrated the commands in core, I couldn't come up
with good enough consistency in any categorization scheme so I just
scrapped the whole idea. It may be worth revisiting in the future.

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
+        # TODO: add a "system defaults" config section so this default
+        # of more(1) can be easily replaced with a global
+        # configuration file. For example, on OS X the sane default is
+        # less(1), not more(1), and on debian it's
+        # sensible-pager(1). We should probably also give the system
+        # default editor command similar treatment.
+        envpager = encoding.environ.get('PAGER', 'more')
+        pagercmd = self.config('pager', 'pager', envpager)

Any thoughts on what this looks like? It seems like it might be distantly(?) related to something I tried a couple years ago [1], and the discussions around it.

It’s pretty different from the previous round in my mind: it’d be a config section, only advertised for sysadmins and packagers, specifically intended for setting more-reasonable defaults, e.g.:

(debian)
[systemdefaults]
editor = sensible-editor
pager = sensible-pager

(macOS)
[systemdefaults]
editor = nano
pager = less

(FreeBSD)
[systemdefaults]
editor = vi
pager = less

etc.

Does it seem reasonable to move the above to a method, similar to ui.editor()? I'd like to detect MSYS and default to `less`. IDK if we can pull that off with just a global config file. (The only way I see to detect MSYS is to look for 'MSYSTEM' in env.)

In line with the above, I’d expect an mays-backed package to set systemdefaults.pager to less (maybe that’s not a thing? I dunno.)

I'm not thinking of an msys package, rather my personal usage. If I'm doing anything of any substance on the command line, I'll use msys. But TortoiseHg launches cmd.exe with a simple Ctrl + Shift + T, and sometimes that's good enough. Therefore, I can't just set a pager value in %HOME%/mercurial.ini. The hg.exe built from source and the one bundled with TortoiseHg are each visible to msys and cmd.exe, depending on what directory I'm in. So even if TortoiseHg picked one (and we ignore hg built from the dev repo), it may or may not use the best pager available without a little help.

I’m extremely hesitant to add any kind of conditional logic in hgrc.

Agreed. The conditional logic I'm thinking of is in python, similar to how plan9 sets its last-ditch editor to 'E' in ui.geteditor(). Based on the vision you've described, I don't see a list as useful. And because I can't think of any other OS with such a bad pager (and a better one so tantalizingly close), maybe a one-off python conditional is OK?

Do you know of any other systems that use this kind of configuration
approach but don’t have a scripting language as their config language?

IDK what this means, so I'm gonna say no. :-)

Note that systemdefaults (which is a name I came up with in the process of writing this mail, and probably isn’t what we should keep) would *not* overwrite environment variables, so if the user has EDITOR or PAGER set, that takes precedence over systemdefaults - this new config section is explicitly about helping packagers get the best possible “out of the box” experience without having to patch hg (which the debian package currently has to for sensible-editor).

Makes sense. This would get rid of the special case editor for plan9, for packages anyway. It wouldn't for hg built from source, but that could presumably be set in ~/.hgrc.

In theory, the `less` command in GnuWin32 can be installed and made visible to cmd.exe too. I wonder if (on some platforms anyway), there should be a list of default choices, and the first one found in $PATH used. ui.editor() doesn't make sure the editor is valid, so I figured keeping that kind of check out of ui was purposeful. A bad editor will produce an error that tries to indicate the problem (though I missed it the first time in the OS error output):

   $ HGEDITOR=xzy ../hg ci
   'xzy' is not recognized as an internal or external command,
   operable program or batch file.
   abort: edit failed: xzy exited with status 1

But not so much for the pager:

   hg>set PAGER=less
   hg>hg help filesets
   'less' is not recognized as an internal or external command,
   operable program or batch file.

Could you please file a bug for this? It reproduces on OS X too, with equally mystifying results (I’m on a slow connection right now, so email is tolerable but https is not).

Done.

I'm not sure if the right thing here is to error out, or to treat the pager as disabled. Especially if it is on by default, with an env knob that isn't as obvious as the example above. So it seems like we might need some sort of check, whether or not we try a list of defaults.

git disables the pager, with a warning that it couldn’t exec the pager. We probably should do something similar, I can’t think of anything else reasonable to do.

Seems reasonable. Not sure if the warning should come before or after the wall of text, but it would be an improvement either way.

[1] https://www.mercurial-scm.org/pipermail/mercurial-devel/2015-March/067563.html
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to