Patchwork [6,of,6,STABLE] pager: expand partial commands in pager.attend-% (issue5527)

login
register
mail settings
Submitter Gregory Szorc
Date April 25, 2017, 7:22 a.m.
Message ID <167dcc29e8d47edb7dbd.1493104974@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/20295/
State Deferred
Headers show

Comments

Gregory Szorc - April 25, 2017, 7:22 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1493104878 25200
#      Tue Apr 25 00:21:18 2017 -0700
# Branch stable
# Node ID 167dcc29e8d47edb7dbd44c00f3da2927e82471b
# Parent  3dd8a2d6584eb02016a9db1b06eb69ff8f054e2d
pager: expand partial commands in pager.attend-% (issue5527)

Previously, if a pager.attend-% config option is using a partial
command name, the option has no effect because we only look for
config options having exact values of the primary command name and
its full aliases. I think this is a poor end-user experience, as
many users may e.g. type `hg st` and want "pager.attend-st" to
"just work."

This commit implements support for expanding partial command names
and aliases in pager.attend-% config options.

There is a more concise way to implement this feature: just set
pager.attend-<full> when iterating pager.attend-% options. However,
I'm not comfortable setting those options because I'm not sure of
the implications.

We should probably also expand pager.attend and pager.ignore
similarly. I'm starting with this patch to test the waters.
Yuya Nishihara - April 25, 2017, 2:26 p.m.
On Tue, 25 Apr 2017 00:22:54 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1493104878 25200
> #      Tue Apr 25 00:21:18 2017 -0700
> # Branch stable
> # Node ID 167dcc29e8d47edb7dbd44c00f3da2927e82471b
> # Parent  3dd8a2d6584eb02016a9db1b06eb69ff8f054e2d
> pager: expand partial commands in pager.attend-% (issue5527)

A new feature including code refactoring. I don't think this is suitable
for stable.

BTW, when will we stop improving the deprecated pager extension?
Gregory Szorc - April 25, 2017, 5:40 p.m.
> On Apr 25, 2017, at 07:26, Yuya Nishihara <yuya@tcha.org> wrote:
> 
>> On Tue, 25 Apr 2017 00:22:54 -0700, Gregory Szorc wrote:
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1493104878 25200
>> #      Tue Apr 25 00:21:18 2017 -0700
>> # Branch stable
>> # Node ID 167dcc29e8d47edb7dbd44c00f3da2927e82471b
>> # Parent  3dd8a2d6584eb02016a9db1b06eb69ff8f054e2d
>> pager: expand partial commands in pager.attend-% (issue5527)
> 
> A new feature including code refactoring. I don't think this is suitable
> for stable.

Fair enough. Could you please consider taking the tests though so the test coverage is there?

> 
> BTW, when will we stop improving the deprecated pager extension?

No clue. Ask Pierre-Yves.
Pierre-Yves David - April 25, 2017, 7:01 p.m.
On 04/25/2017 07:40 PM, Gregory Szorc wrote:
>
>
>> On Apr 25, 2017, at 07:26, Yuya Nishihara <yuya@tcha.org> wrote:
>>
>>> On Tue, 25 Apr 2017 00:22:54 -0700, Gregory Szorc wrote:
>>> # HG changeset patch
>>> # User Gregory Szorc <gregory.szorc@gmail.com>
>>> # Date 1493104878 25200
>>> #      Tue Apr 25 00:21:18 2017 -0700
>>> # Branch stable
>>> # Node ID 167dcc29e8d47edb7dbd44c00f3da2927e82471b
>>> # Parent  3dd8a2d6584eb02016a9db1b06eb69ff8f054e2d
>>> pager: expand partial commands in pager.attend-% (issue5527)
>>
>> A new feature including code refactoring. I don't think this is suitable
>> for stable.
>
> Fair enough. Could you please consider taking the tests though so the test coverage is there?
>
>>
>> BTW, when will we stop improving the deprecated pager extension?
>
> No clue. Ask Pierre-Yves.

hu? what is the context of the question and why does it falls to me?
Gregory Szorc - April 25, 2017, 11:06 p.m.
On Tue, Apr 25, 2017 at 12:01 PM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 04/25/2017 07:40 PM, Gregory Szorc wrote:
>
>>
>>
>> On Apr 25, 2017, at 07:26, Yuya Nishihara <yuya@tcha.org> wrote:
>>>
>>> On Tue, 25 Apr 2017 00:22:54 -0700, Gregory Szorc wrote:
>>>> # HG changeset patch
>>>> # User Gregory Szorc <gregory.szorc@gmail.com>
>>>> # Date 1493104878 25200
>>>> #      Tue Apr 25 00:21:18 2017 -0700
>>>> # Branch stable
>>>> # Node ID 167dcc29e8d47edb7dbd44c00f3da2927e82471b
>>>> # Parent  3dd8a2d6584eb02016a9db1b06eb69ff8f054e2d
>>>> pager: expand partial commands in pager.attend-% (issue5527)
>>>>
>>>
>>> A new feature including code refactoring. I don't think this is suitable
>>> for stable.
>>>
>>
>> Fair enough. Could you please consider taking the tests though so the
>> test coverage is there?
>>
>>
>>> BTW, when will we stop improving the deprecated pager extension?
>>>
>>
>> No clue. Ask Pierre-Yves.
>>
>
> hu? what is the context of the question and why does it falls to me?


I confused the relationship between you and Augie and color and pager.
Augie was the one moving pager to core, not you.
Yuya Nishihara - April 26, 2017, 11:51 a.m.
On Tue, 25 Apr 2017 10:40:37 -0700, Gregory Szorc wrote:
> > On Apr 25, 2017, at 07:26, Yuya Nishihara <yuya@tcha.org> wrote:
> >> On Tue, 25 Apr 2017 00:22:54 -0700, Gregory Szorc wrote:
> >> # HG changeset patch
> >> # User Gregory Szorc <gregory.szorc@gmail.com>
> >> # Date 1493104878 25200
> >> #      Tue Apr 25 00:21:18 2017 -0700
> >> # Branch stable
> >> # Node ID 167dcc29e8d47edb7dbd44c00f3da2927e82471b
> >> # Parent  3dd8a2d6584eb02016a9db1b06eb69ff8f054e2d
> >> pager: expand partial commands in pager.attend-% (issue5527)
> > 
> > A new feature including code refactoring. I don't think this is suitable
> > for stable.
> 
> Fair enough. Could you please consider taking the tests though so the test coverage is there?

Sure. Queued 3-5, thanks.

This patch would introduce inconsistency between deprecated and modern pager.
The modern pager doesn't work in dispatch layer, so you can't disable it by
an abbreviated command name. I think that's also true for new commands.* config
knob.

Patch

diff --git a/hgext/pager.py b/hgext/pager.py
--- a/hgext/pager.py
+++ b/hgext/pager.py
@@ -40,6 +40,28 @@  def _pagerruncommand(orig, ui, options, 
     if options['pager'] != 'auto' or ui.pageractive:
         return orig(ui, options, cmd, cmdfunc)
 
+    # Expand partial commands in pager.attend-% options.
+    reverseoptions = {}
+    for key, value in ui.configitems('pager'):
+        if not key.startswith('attend-'):
+            continue
+
+        command = key[len('attend-'):]
+        cmds, _ = cmdutil.findpossible(command, commands.table)
+
+        # Exact match. Config entry is fine.
+        if command in cmds:
+            continue
+
+        # Maps to an ambiguous command. No clear action to take.
+        if len(cmds) > 1:
+            continue
+
+        # Else, the name maps to another command. Set the config option.
+        if cmds:
+            actualcommand = cmds.keys()[0]
+            reverseoptions.setdefault(actualcommand, set()).add(command)
+
     usepager = False
     attend = ui.configlist('pager', 'attend', attended)
     ignore = ui.configlist('pager', 'ignore')
@@ -50,6 +72,18 @@  def _pagerruncommand(orig, ui, options, 
         if ui.config('pager', var):
             usepager = ui.configbool('pager', var)
             break
+
+        havereverse = False
+        for reversename in reverseoptions.get(cmd, []):
+            var = 'attend-%s' % reversename
+            if ui.config('pager', var):
+                usepager = ui.configbool('pager', var)
+                havereverse = True
+                break
+
+        if havereverse:
+            break
+
         if (cmd in attend or
                 (cmd not in ignore and not attend)):
             usepager = True
diff --git a/tests/test-pager.t b/tests/test-pager.t
--- a/tests/test-pager.t
+++ b/tests/test-pager.t
@@ -94,13 +94,14 @@  Abbreviated command alias should also be
   paged! 'summary:     modify a 10\n'
   paged! '\n'
 
-Attend for an abbreviated command does not work
+Attend for an abbreviated command works
+(TODO broken without pager extension enabled)
 
   $ hg --config pager.attend-ident=true ident
   46106edeeb38 tip
 
   $ hg --config extensions.pager= --config pager.attend-ident=true ident
-  46106edeeb38 tip
+  paged! '46106edeeb38 tip\n'
 
 Pager should not start if stdout is not a tty.