Patchwork [5,of,5] revert: simplify loop conditional

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 18, 2014, 10:30 p.m.
Message ID <cf55112fb55d83b77487.1408401055@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/5481/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 18, 2014, 10:30 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1406918516 25200
#      Fri Aug 01 11:41:56 2014 -0700
# Node ID cf55112fb55d83b77487e3b32235f6ab2b83cd5c
# Parent  e53ef14e9147149b9522ac32d603ef55d636ae4b
revert: simplify loop conditional

The two break can be joined into one. The code gain one level of ident.
Augie Fackler - Aug. 18, 2014, 11:11 p.m.
On Mon, Aug 18, 2014 at 03:30:55PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1406918516 25200
> #      Fri Aug 01 11:41:56 2014 -0700
> # Node ID cf55112fb55d83b77487e3b32235f6ab2b83cd5c
> # Parent  e53ef14e9147149b9522ac32d603ef55d636ae4b
> revert: simplify loop conditional

Queued, likely with an English tweak in one or two patches if I remember.

>
> The two break can be joined into one. The code gain one level of ident.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2495,26 +2495,25 @@ def revert(ui, repo, ctx, parents, *pats
>              # if the file is in any of this sets, it was touched in the working
>              # directory parent and we are sure it needs to be reverted.
>              for table, (xlist, msg), dobackup in disptable:
>                  if abs not in table:
>                      continue
> -                if xlist is None:
> -                    if exact:
> +                if xlist is not None:
> +                    xlist.append(abs)
> +                    if (dobackup and os.path.lexists(target) and
> +                        abs in ctx and repo[None][abs].cmp(ctx[abs])):
> +                        bakname = "%s.orig" % rel
> +                        ui.note(_('saving current version of %s as %s\n') %
> +                                (rel, bakname))
> +                        if not opts.get('dry_run'):
> +                            util.rename(target, bakname)
> +                    if ui.verbose or not exact:
> +                        if not isinstance(msg, basestring):
> +                            msg = msg(abs)
> +                        ui.status(msg % rel)
> +                elif exact:
>                          ui.warn(_('no changes needed to %s\n') % rel)
> -                    break
> -                xlist.append(abs)
> -                if (dobackup and os.path.lexists(target) and
> -                    abs in ctx and repo[None][abs].cmp(ctx[abs])):
> -                    bakname = "%s.orig" % rel
> -                    ui.note(_('saving current version of %s as %s\n') %
> -                            (rel, bakname))
> -                    if not opts.get('dry_run'):
> -                        util.rename(target, bakname)
> -                if ui.verbose or not exact:
> -                    if not isinstance(msg, basestring):
> -                        msg = msg(abs)
> -                    ui.status(msg % rel)
>                  break
>              else:
>                  # Not touched in current dirstate.
>
>                  # file is unknown in parent, restore older version or ignore.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2495,26 +2495,25 @@  def revert(ui, repo, ctx, parents, *pats
             # if the file is in any of this sets, it was touched in the working
             # directory parent and we are sure it needs to be reverted.
             for table, (xlist, msg), dobackup in disptable:
                 if abs not in table:
                     continue
-                if xlist is None:
-                    if exact:
+                if xlist is not None:
+                    xlist.append(abs)
+                    if (dobackup and os.path.lexists(target) and
+                        abs in ctx and repo[None][abs].cmp(ctx[abs])):
+                        bakname = "%s.orig" % rel
+                        ui.note(_('saving current version of %s as %s\n') %
+                                (rel, bakname))
+                        if not opts.get('dry_run'):
+                            util.rename(target, bakname)
+                    if ui.verbose or not exact:
+                        if not isinstance(msg, basestring):
+                            msg = msg(abs)
+                        ui.status(msg % rel)
+                elif exact:
                         ui.warn(_('no changes needed to %s\n') % rel)
-                    break
-                xlist.append(abs)
-                if (dobackup and os.path.lexists(target) and
-                    abs in ctx and repo[None][abs].cmp(ctx[abs])):
-                    bakname = "%s.orig" % rel
-                    ui.note(_('saving current version of %s as %s\n') %
-                            (rel, bakname))
-                    if not opts.get('dry_run'):
-                        util.rename(target, bakname)
-                if ui.verbose or not exact:
-                    if not isinstance(msg, basestring):
-                        msg = msg(abs)
-                    ui.status(msg % rel)
                 break
             else:
                 # Not touched in current dirstate.
 
                 # file is unknown in parent, restore older version or ignore.