Patchwork [1,of,2] dispatch: always load extensions before running shell aliases (issue5230)

login
register
mail settings
Submitter Jun Wu
Date May 7, 2016, 2:18 p.m.
Message ID <e4e2efae9f13369bd9e9.1462630705@x1c>
Download mbox | patch
Permalink /patch/14965/
State Accepted
Delegated to: Yuya Nishihara
Headers show

Comments

Jun Wu - May 7, 2016, 2:18 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1462626743 -3600
#      Sat May 07 14:12:23 2016 +0100
# Node ID e4e2efae9f13369bd9e9691edd48a7da6fd18fcb
# Parent  983353035cec51382257b7cbd599c7fa4da97847
dispatch: always load extensions before running shell aliases (issue5230)

Before this patch, we may or may not load extensions for shell aliases
depending on whether the command is abbreviated or not.

Loading extensions may have useful side effects to shell aliases. For example,
the pager extension does not work for shell aliases.

This patch removes the code checking shell aliases before loading extensions
to give the user a more consistent experience. It may hurt performance for
shell aliases a bit without chg but the correctness seems worth it. It will
also make the behavior consistent with chg since chg will always load all
extensions before running commands.
Yuya Nishihara - May 10, 2016, 12:53 p.m.
On Sat, 7 May 2016 15:18:25 +0100, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1462626743 -3600
> #      Sat May 07 14:12:23 2016 +0100
> # Node ID e4e2efae9f13369bd9e9691edd48a7da6fd18fcb
> # Parent  983353035cec51382257b7cbd599c7fa4da97847
> dispatch: always load extensions before running shell aliases (issue5230)

Nice cleanup. Pushed to the committed repo, thanks.

> This patch removes the code checking shell aliases before loading extensions
> to give the user a more consistent experience. It may hurt performance for
> shell aliases a bit

I've investigated how the shell alias handling was evolved, and my conclusion
is there was no practical reason to skip extensions.loadall().

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -664,12 +664,8 @@ 
 
     return path, lui
 
-def _checkshellalias(lui, ui, args, precheck=True):
-    """Return the function to run the shell alias, if it is required
-
-    'precheck' is whether this function is invoked before adding
-    aliases or not.
-    """
+def _checkshellalias(lui, ui, args):
+    """Return the function to run the shell alias, if it is required"""
     options = {}
 
     try:
@@ -680,16 +676,11 @@ 
     if not args:
         return
 
-    if precheck:
-        strict = True
-        cmdtable = commands.table.copy()
-        addaliases(lui, cmdtable)
-    else:
-        strict = False
-        cmdtable = commands.table
+    cmdtable = commands.table
 
     cmd = args[0]
     try:
+        strict = ui.configbool("ui", "strict")
         aliases, entry = cmdutil.findcmd(cmd, cmdtable, strict)
     except (error.AmbiguousCommand, error.UnknownCommand):
         return
@@ -739,12 +730,6 @@ 
     rpath = _earlygetopt(["-R", "--repository", "--repo"], args)
     path, lui = _getlocal(ui, rpath)
 
-    # Now that we're operating in the right directory/repository with
-    # the right config settings, check for shell aliases
-    shellaliasfn = _checkshellalias(lui, ui, args)
-    if shellaliasfn:
-        return shellaliasfn()
-
     # Configure extensions in phases: uisetup, extsetup, cmdtable, and
     # reposetup. Programs like TortoiseHg will call _dispatch several
     # times so we keep track of configured extensions in _loaded.
@@ -766,13 +751,11 @@ 
 
     addaliases(lui, commands.table)
 
-    if not lui.configbool("ui", "strict"):
-        # All aliases and commands are completely defined, now.
-        # Check abbreviation/ambiguity of shell alias again, because shell
-        # alias may cause failure of "_parse" (see issue4355)
-        shellaliasfn = _checkshellalias(lui, ui, args, precheck=False)
-        if shellaliasfn:
-            return shellaliasfn()
+    # All aliases and commands are completely defined, now.
+    # Check abbreviation/ambiguity of shell alias.
+    shellaliasfn = _checkshellalias(lui, ui, args)
+    if shellaliasfn:
+        return shellaliasfn()
 
     # check for fallback encoding
     fallback = lui.config('ui', 'fallbackencoding')
diff --git a/tests/test-pager.t b/tests/test-pager.t
--- a/tests/test-pager.t
+++ b/tests/test-pager.t
@@ -177,3 +177,15 @@ 
   paged! 'date:        Thu Jan 01 00:00:00 1970 +0000\n'
   paged! 'summary:     modify a 8\n'
   paged! '\n'
+
+Pager works with shell aliases.
+
+  $ cat >> $HGRCPATH <<EOF
+  > [alias]
+  > echoa = !echo a
+  > EOF
+
+  $ hg echoa
+  a
+  $ hg --config pager.attend-echoa=yes echoa
+  paged! 'a\n'