Patchwork [9,of,9] subrepo: drop the 'ui' parameter to revert()

login
register
mail settings
Submitter Matt Harbison
Date Dec. 15, 2014, 1:12 a.m.
Message ID <cd72135f7096f311371a.1418605966@Envy>
Download mbox | patch
Permalink /patch/7110/
State Accepted
Headers show

Comments

Matt Harbison - Dec. 15, 2014, 1:12 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1418517895 18000
#      Sat Dec 13 19:44:55 2014 -0500
# Node ID cd72135f7096f311371a86fab038d0a27c3fea58
# Parent  b58f5dcfe069ccf5154788c05348cc328b4f13f2
subrepo: drop the 'ui' parameter to revert()

This no longer needs to be explicitly passed because the subrepo object tracks
the 'ui' reference since fcbc66b5da6a.  See the change to 'archive' for details
about the differences between the output level in the root repo and subrepo 'ui'
object.

The only use for 'ui' in revert is to emit status and warning messages, and to
check the verbose flag prior to printing the action to be performed on a file.

The local repo's ui was already being used to print a warning message in
wctx.forget() and for 'ui.slash' when walking dirstate in the repo.status()
call.  Unlike other methods where the matcher is passed along and narrowed, a
new matcher is created in each repo, and therefore the bad() method already used
the local repo's ui.
Augie Fackler - Dec. 15, 2014, 4:29 p.m.
On Sun, Dec 14, 2014 at 08:12:46PM -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1418517895 18000
> #      Sat Dec 13 19:44:55 2014 -0500
> # Node ID cd72135f7096f311371a86fab038d0a27c3fea58
> # Parent  b58f5dcfe069ccf5154788c05348cc328b4f13f2
> subrepo: drop the 'ui' parameter to revert()

Series sounds reasonable. Queued, will push once I've read the diffs
one more time and run the tests. Thanks!

>
> This no longer needs to be explicitly passed because the subrepo object tracks
> the 'ui' reference since fcbc66b5da6a.  See the change to 'archive' for details
> about the differences between the output level in the root repo and subrepo 'ui'
> object.
>
> The only use for 'ui' in revert is to emit status and warning messages, and to
> check the verbose flag prior to printing the action to be performed on a file.
>
> The local repo's ui was already being used to print a warning message in
> wctx.forget() and for 'ui.slash' when walking dirstate in the repo.status()
> call.  Unlike other methods where the matcher is passed along and narrowed, a
> new matcher is created in each repo, and therefore the bad() method already used
> the local repo's ui.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2801,7 +2801,7 @@
>              if targetsubs:
>                  # Revert the subrepos on the revert list
>                  for sub in targetsubs:
> -                    ctx.sub(sub).revert(ui, ctx.substate[sub], *pats, **opts)
> +                    ctx.sub(sub).revert(ctx.substate[sub], *pats, **opts)
>      finally:
>          wlock.release()
>
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -506,8 +506,8 @@
>          """
>          return 1
>
> -    def revert(self, ui, substate, *pats, **opts):
> -        ui.warn('%s: reverting %s subrepos is unsupported\n' \
> +    def revert(self, substate, *pats, **opts):
> +        self.ui.warn('%s: reverting %s subrepos is unsupported\n' \
>              % (substate[0], substate[2]))
>          return []
>
> @@ -861,13 +861,13 @@
>                                subrepos)
>
>      @annotatesubrepoerror
> -    def revert(self, ui, substate, *pats, **opts):
> +    def revert(self, substate, *pats, **opts):
>          # reverting a subrepo is a 2 step process:
>          # 1. if the no_backup is not set, revert all modified
>          #    files inside the subrepo
>          # 2. update the subrepo to the revision specified in
>          #    the corresponding substate dictionary
> -        ui.status(_('reverting subrepo %s\n') % substate[0])
> +        self.ui.status(_('reverting subrepo %s\n') % substate[0])
>          if not opts.get('no_backup'):
>              # Revert all files on the subrepo, creating backups
>              # Note that this will not recursively revert subrepos
> @@ -879,19 +879,19 @@
>              pats = []
>              if not opts.get('all'):
>                  pats = ['set:modified()']
> -            self.filerevert(ui, *pats, **opts)
> +            self.filerevert(*pats, **opts)
>
>          # Update the repo to the revision specified in the given substate
>          self.get(substate, overwrite=True)
>
> -    def filerevert(self, ui, *pats, **opts):
> +    def filerevert(self, *pats, **opts):
>          ctx = self._repo[opts['rev']]
>          parents = self._repo.dirstate.parents()
>          if opts.get('all'):
>              pats = ['set:modified()']
>          else:
>              pats = []
> -        cmdutil.revert(ui, self._repo, ctx, parents, *pats, **opts)
> +        cmdutil.revert(self.ui, self._repo, ctx, parents, *pats, **opts)
>
>      def shortid(self, revid):
>          return revid[:12]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Matt Mackall - Dec. 18, 2014, 6:01 p.m.
On Sun, 2014-12-14 at 20:12 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1418517895 18000
> #      Sat Dec 13 19:44:55 2014 -0500
> # Node ID cd72135f7096f311371a86fab038d0a27c3fea58
> # Parent  b58f5dcfe069ccf5154788c05348cc328b4f13f2
> subrepo: drop the 'ui' parameter to revert()

FYI, this broke test-subrepo-git.t. I've fixed it in flight.
Matt Harbison - Dec. 18, 2014, 6:17 p.m.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2801,7 +2801,7 @@ 
             if targetsubs:
                 # Revert the subrepos on the revert list
                 for sub in targetsubs:
-                    ctx.sub(sub).revert(ui, ctx.substate[sub], *pats, **opts)
+                    ctx.sub(sub).revert(ctx.substate[sub], *pats, **opts)
     finally:
         wlock.release()
 
diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -506,8 +506,8 @@ 
         """
         return 1
 
-    def revert(self, ui, substate, *pats, **opts):
-        ui.warn('%s: reverting %s subrepos is unsupported\n' \
+    def revert(self, substate, *pats, **opts):
+        self.ui.warn('%s: reverting %s subrepos is unsupported\n' \
             % (substate[0], substate[2]))
         return []
 
@@ -861,13 +861,13 @@ 
                               subrepos)
 
     @annotatesubrepoerror
-    def revert(self, ui, substate, *pats, **opts):
+    def revert(self, substate, *pats, **opts):
         # reverting a subrepo is a 2 step process:
         # 1. if the no_backup is not set, revert all modified
         #    files inside the subrepo
         # 2. update the subrepo to the revision specified in
         #    the corresponding substate dictionary
-        ui.status(_('reverting subrepo %s\n') % substate[0])
+        self.ui.status(_('reverting subrepo %s\n') % substate[0])
         if not opts.get('no_backup'):
             # Revert all files on the subrepo, creating backups
             # Note that this will not recursively revert subrepos
@@ -879,19 +879,19 @@ 
             pats = []
             if not opts.get('all'):
                 pats = ['set:modified()']
-            self.filerevert(ui, *pats, **opts)
+            self.filerevert(*pats, **opts)
 
         # Update the repo to the revision specified in the given substate
         self.get(substate, overwrite=True)
 
-    def filerevert(self, ui, *pats, **opts):
+    def filerevert(self, *pats, **opts):
         ctx = self._repo[opts['rev']]
         parents = self._repo.dirstate.parents()
         if opts.get('all'):
             pats = ['set:modified()']
         else:
             pats = []
-        cmdutil.revert(ui, self._repo, ctx, parents, *pats, **opts)
+        cmdutil.revert(self.ui, self._repo, ctx, parents, *pats, **opts)
 
     def shortid(self, revid):
         return revid[:12]