Augie Fackler <r...@durin42.com> writes: > (sending again because this seems to have gotten stuck somewhere...) > (+yuya explicitly for chg thoughts, +smf so someone else can forward to the > list if it's stuck again) > > On Fri, Feb 03, 2017 at 04:16:09PM -0800, Bryan O'Sullivan wrote: >> # HG changeset patch >> # User Bryan O'Sullivan <bry...@fb.com> >> # Date 1486160890 28800 >> # Fri Feb 03 14:28:10 2017 -0800 >> # Node ID 30ee18bf947b97eca3582555f63eb3b2441e9db8 >> # Parent abf029200e198878a4576a87e095bd8d77d9cea9 >> pager: migrate heavily-used extension into core >> >> No default behaviours were harmed during the making of this change. >> >> Note: this patch will break out-of-tree extensions that rely on the >> location of the old pager module's attend variable. It is now a >> static variable named pagedcommands on the ui class. > > I've got feedback on this approach, which I'd like to talk out a bit > before we either decide to land this or go on a v3 voyage. See below. > > If this is agreeable, I'll just push other things aside and do this, > since it's something that we've got pretty clear consensus on, and I'm > the one proposing a chunk of work to try and gild the lily. > > Sorry for how long this is - if you want, jump to the API sketch at > the bottom and start from there, and look at my paragraph of rationale > only if that looks bad? > >> >> diff --git a/mercurial/commands.py b/mercurial/commands.py >> --- a/mercurial/commands.py >> +++ b/mercurial/commands.py >> @@ -107,6 +107,8 @@ globalopts = [ >> ('', 'version', None, _('output version information and exit')), >> ('h', 'help', None, _('display help and exit')), >> ('', 'hidden', False, _('consider hidden changesets')), >> + ('', 'pager', 'auto', >> + _("when to paginate (boolean, always, auto, or never)"), _('TYPE')), >> ] >> >> dryrunopts = [('n', 'dry-run', None, >> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py >> --- a/mercurial/dispatch.py >> +++ b/mercurial/dispatch.py >> @@ -816,6 +816,39 @@ def _dispatch(req): >> def _runcommand(ui, options, cmd, cmdfunc): >> """Run a command function, possibly with profiling enabled.""" >> try: >> + p = ui.config("pager", "pager", encoding.environ.get("PAGER")) >> + usepager = ui.pageractive >> + always = util.parsebool(options['pager']) >> + auto = options['pager'] == 'auto' >> + >> + if not p or '--debugger' in sys.argv or not ui.formatted(): >> + pass >> + elif always: >> + usepager = True >> + elif not auto: >> + usepager = False >> + else: >> + attend = ui.configlist('pager', 'attend', ui.pagedcommands) >> + ignore = ui.configlist('pager', 'ignore') >> + cmds, _ = cmdutil.findcmd(cmd, commands.table) > > This bums me out as an approach, because it means that a command is > necessarily always paged or always not paged, which is not always > appropriate. The immediate example I can think of is shelve, which is > multi-mode: > > `shelve --list --patch` -> definitely wants to be paged, this is > likely a ton of output > > `shelve --interactive` -> is *broken* if a pager is in play. > > Rather than stick with the brute-force attend list in core, I'd like > to move in the direction of adding a ui.pager() call to the ui object, > which starts the pager. We'd then have a transitional period (a couple > of releases?) where programmatically setting the attend list was > supported in the old pager extension code, and then always support > setting pager.attend to forcibly page commands which don't think they > want pager love. This also plays reasonably well with chg, because it > means that there's (still) a trivial point for chg to be told "start > the pager now". > > My reason for wanting to move away from the attend list is the above, > but also that it's hopelessly buggy in certain circumstances, like > aliases, and it doesn't have a good affordance for new commands to > specify default behavior (other than poking themselves in a list, but > if the user has configured the attend list they no longer get the > benefits of the sane defaults people hopefully put thought into). > > How does that sound, overall? I'll tackle the refactoring today or > Wednesday to mail an RFC patch if that doesn't sound too much like the > perfect being the enemy of the good. > > (This design, btw, was inspired by the way git handles paging, where > it's largely up to the command to decide if it wants to invoke the > pager.)
If I'm understanding you correctly, this will get rid of the pager.attend variable? If that's true, then I fully support that. > As a sketch of where this is headed, API-wise: > > class ui: > def pager(self, command, category): > ""Starts the pager, if desired by the user. > > `command` specifies the current command the user is running, > something like `log` or `shelve`. It should be the whole original > command name, not the partial name or alias name. > > `category` specifies a broad category this pager invocation fits > into. Examples include `diff`, `log`, `status`, `help`. This > allows users to disable paging of entire types of commands easily. > """ > # pager starts, self.pageractive=true, etc > > @command > def shelve(ui, ...): > if action == 'list': > ui.pager('shelve', 'diff' if --patch else 'log') > ... > > @command > def summary(ui, ...): > ui.pager('summary', 'status') > ... Would this get rid of the need to set pager.pager=less? I think last time the pager was brought up (I believe the Munich sprint), there was a consensus on not relying on the existence of less / more / windows weirdness. The API looks pretty good to me. Nothing off the top of my head that I can add / question besides what I asked above. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel