Patchwork pager: honour aliases (issue3532)

# HG changeset patch
# User David Soria Parra <dsp@experimentalworks.net>
# Date 1381629094 25200
#      Sat Oct 12 18:51:34 2013 -0700
# Node ID 39084a836b0525cbcabc60b5200e83a5cd91577d
# Parent  1b2f9d36953e6ed384a044c1e73cb3a1aa072004
pager: honour aliases (issue3532)

If paging is configured for a command all it's aliases will be paged as
well. This will make attend=log cause 'hg history' to run the pager as
well as custom aliases defined with [alias].

On Sat, Oct 12, 2013 at 07:01:58PM -0700, David Soria Parra wrote:
> # HG changeset patch
> # User David Soria Parra <dsp@experimentalworks.net>
> # Date 1381629094 25200
> #      Sat Oct 12 18:51:34 2013 -0700
> # Node ID 39084a836b0525cbcabc60b5200e83a5cd91577d
> # Parent  1b2f9d36953e6ed384a044c1e73cb3a1aa072004
> pager: honour aliases (issue3532)

Hm, I could see going both ways on this. After all, using an alias is
one of the "normal" ways to bypass configuration on a given command,
isn't it?

I don't use pager though, so it won't matter for me one way or the other.

>
> If paging is configured for a command all it's aliases will be paged as
> well. This will make attend=log cause 'hg history' to run the pager as
> well as custom aliases defined with [alias].
>
> diff --git a/hgext/pager.py b/hgext/pager.py
> --- a/hgext/pager.py
> +++ b/hgext/pager.py
> @@ -48,7 +48,7 @@
>  '''
>
>  import atexit, sys, os, signal, subprocess, errno, shlex
> -from mercurial import commands, dispatch, util, extensions
> +from mercurial import commands, dispatch, util, extensions, cmdutil
>  from mercurial.i18n import _
>
>  testedwith = 'internal'
> @@ -121,14 +121,22 @@
>              attend = ui.configlist('pager', 'attend', attended)
>              auto = options['pager'] == 'auto'
>              always = util.parsebool(options['pager'])
> -            if (always or auto and
> -                (cmd in attend or
> -                 (cmd not in ui.configlist('pager', 'ignore') and not attend))):
> -                ui.setconfig('ui', 'formatted', ui.formatted())
> -                ui.setconfig('ui', 'interactive', False)
> -                if util.safehasattr(signal, "SIGPIPE"):
> -                    signal.signal(signal.SIGPIPE, signal.SIG_DFL)
> -                _runpager(ui, p)
> +
> +            cmds, (alias, _, _) = cmdutil.findcmd(cmd, commands.table)
> +            if util.safehasattr(alias, 'cmdname'):
> +                cmds.append(alias.cmdname)
> +
> +            ignore = ui.configlist('pager', 'ignore')
> +            for cmd in cmds:
> +                if (always or auto and
> +                    (cmd in attend or
> +                     (cmd not in ignore and not attend))):
> +                    ui.setconfig('ui', 'formatted', ui.formatted())
> +                    ui.setconfig('ui', 'interactive', False)
> +                    if util.safehasattr(signal, "SIGPIPE"):
> +                        signal.signal(signal.SIGPIPE, signal.SIG_DFL)
> +                    _runpager(ui, p)
> +                    break
>          return orig(ui, options, cmd, cmdfunc)
>
>      extensions.wrapfunction(dispatch, '_runcommand', pagecmd)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

On 10/16/2013 07:22 AM, Augie Fackler wrote:
> On Sat, Oct 12, 2013 at 07:01:58PM -0700, David Soria Parra wrote:
>> # HG changeset patch
>> # User David Soria Parra <dsp@experimentalworks.net>
>> # Date 1381629094 25200
>> #      Sat Oct 12 18:51:34 2013 -0700
>> # Node ID 39084a836b0525cbcabc60b5200e83a5cd91577d
>> # Parent  1b2f9d36953e6ed384a044c1e73cb3a1aa072004
>> pager: honour aliases (issue3532)
> 
> Hm, I could see going both ways on this. After all, using an alias is
> one of the "normal" ways to bypass configuration on a given command,
> isn't it?
> 
> I don't use pager though, so it won't matter for me one way or the other.
> 

Yes it's definatly argueable, but I think in most cases once you decide
that your alias is on the attend list you also want aliases. E.g i use
bm as an abbrev for bookmarks and have some shortcuts for log, so it's a
bit annoying to have an increasing attend list.

In case we decide against allowing aliases I'll followup with a patch
that at least takes fixed aliases such as history|log into account. In
that case I think it's clear, if you attend log you also want hg history
to page.

David

On 10/16/13 8:14 AM, "David Soria Parra" <dsp@experimentalworks.net> wrote:

>On 10/16/2013 07:22 AM, Augie Fackler wrote:
>> On Sat, Oct 12, 2013 at 07:01:58PM -0700, David Soria Parra wrote:
>>> # HG changeset patch
>>> # User David Soria Parra <dsp@experimentalworks.net>
>>> # Date 1381629094 25200
>>> #      Sat Oct 12 18:51:34 2013 -0700
>>> # Node ID 39084a836b0525cbcabc60b5200e83a5cd91577d
>>> # Parent  1b2f9d36953e6ed384a044c1e73cb3a1aa072004
>>> pager: honour aliases (issue3532)
>> 
>> Hm, I could see going both ways on this. After all, using an alias is
>> one of the "normal" ways to bypass configuration on a given command,
>> isn't it?
>> 
>> I don't use pager though, so it won't matter for me one way or the
>>other.
>> 
>
>Yes it's definatly argueable, but I think in most cases once you decide
>that your alias is on the attend list you also want aliases. E.g i use
>bm as an abbrev for bookmarks and have some shortcuts for log, so it's a
>bit annoying to have an increasing attend list.
>
>In case we decide against allowing aliases I'll followup with a patch
>that at least takes fixed aliases such as history|log into account. In
>that case I think it's clear, if you attend log you also want hg history
>to page.

I'm in favor of this change.  In our setup here we have an attend list set
in the system level hgrc on everyone's machine.  If someone adds a
personal alias for log that uses their own template, they are unable to
add it to the attend list without overwriting the system level one.  It
would be much more convenient if aliases for log were automatically paged
as well.

On Sat, 2013-10-12 at 19:01 -0700, David Soria Parra wrote:
> # HG changeset patch
> # User David Soria Parra <dsp@experimentalworks.net>
> # Date 1381629094 25200
> #      Sat Oct 12 18:51:34 2013 -0700
> # Node ID 39084a836b0525cbcabc60b5200e83a5cd91577d
> # Parent  1b2f9d36953e6ed384a044c1e73cb3a1aa072004
> pager: honour aliases (issue3532)
> 
> If paging is configured for a command all it's aliases will be paged as
> well. This will make attend=log cause 'hg history' to run the pager as
> well as custom aliases defined with [alias].

I think I have to say no to this one. People might be using aliases to
avoid/get paging today so this is a clear BC breakage... and offers
nothing users can't do for themselves in their config file (which
they're already editing), possibly by adding --pager. 

It's also possible with aliases to create things (for instance with
revsets and templates) that are radically different in usage than the
commands they alias, so that they will be thought of as new commands
rather than simply tweaked commands.

To Durham's complaint that there's no easy way to append to global
attend settings, I think the right fix is to add a new more granular
setting, perhaps:

[pager]
attend-fancylog = True
attend-export = False

On 10/20/2013 03:27 PM, Matt Mackall wrote:
> On Sat, 2013-10-12 at 19:01 -0700, David Soria Parra wrote:
>> # HG changeset patch
>> # User David Soria Parra <dsp@experimentalworks.net>
>> # Date 1381629094 25200
>> #      Sat Oct 12 18:51:34 2013 -0700
>> # Node ID 39084a836b0525cbcabc60b5200e83a5cd91577d
>> # Parent  1b2f9d36953e6ed384a044c1e73cb3a1aa072004
>> pager: honour aliases (issue3532)
>>
>> If paging is configured for a command all it's aliases will be paged as
>> well. This will make attend=log cause 'hg history' to run the pager as
>> well as custom aliases defined with [alias].
> 
> I think I have to say no to this one. People might be using aliases to
> avoid/get paging today so this is a clear BC breakage... and offers
> nothing users can't do for themselves in their config file (which
> they're already editing), possibly by adding --pager. 
> 

Fair enough, I still think that attend on log should also page
"history". So that predefined aliases in commands should be paged.

David

On Sun, 2013-10-20 at 15:30 -0700, David Soria Parra wrote:
> On 10/20/2013 03:27 PM, Matt Mackall wrote:
> > On Sat, 2013-10-12 at 19:01 -0700, David Soria Parra wrote:
> >> # HG changeset patch
> >> # User David Soria Parra <dsp@experimentalworks.net>
> >> # Date 1381629094 25200
> >> #      Sat Oct 12 18:51:34 2013 -0700
> >> # Node ID 39084a836b0525cbcabc60b5200e83a5cd91577d
> >> # Parent  1b2f9d36953e6ed384a044c1e73cb3a1aa072004
> >> pager: honour aliases (issue3532)
> >>
> >> If paging is configured for a command all it's aliases will be paged as
> >> well. This will make attend=log cause 'hg history' to run the pager as
> >> well as custom aliases defined with [alias].
> > 
> > I think I have to say no to this one. People might be using aliases to
> > avoid/get paging today so this is a clear BC breakage... and offers
> > nothing users can't do for themselves in their config file (which
> > they're already editing), possibly by adding --pager. 
> > 
> 
> Fair enough, I still think that attend on log should also page
> "history". So that predefined aliases in commands should be paged.

Agreed. I'd be surprised if anyone actually uses 'hg history' though.
Most of the other variants are prefixes (in vs incoming) or things you
probably don't care about paging (checkout vs update).