revert: fix the inconsistency of status msgs in --interactive mode

Before this patch we were priting every action msg before actually
performing that action and that was resulting in inconsistencies;
like in --interactive session if user decided to not revert any
changes in a file foo, still there will be a msg on console saying
"reverting foo".

To fix this, I have made some changes to print status msg just
before the action it is going to perform, no matter if --interactive
or not.

Changes made in test-revert-interactive.t reflect the changed behavior.
There are also some changes in test-revert.t because of change in the
order of messages.

Differential Revision: https://phab.mercurial-scm.org/D4380

mercurial/cmdutil.py

@@ -3019,8 +3019,9 b' def revert(ui, repo, ctx, parents, *pats'
                                     util.copyfile(target, bakname)
                                 else:
                                     util.rename(target, bakname)
-                    if ui.verbose or not exact:
-                        ui.status(msg % rel)
+                    if opts.get('dry_run'):
+                        if ui.verbose or not exact:
+                            ui.status(msg % rel)
                 elif exact:
                     ui.warn(msg % rel)
                 break
@@ -3033,7 +3034,8 b' def revert(ui, repo, ctx, parents, *pats'
             prefetch(repo, [ctx.rev()],
                      matchfiles(repo,
                                 [f for sublist in oplist for f in sublist]))
-            _performrevert(repo, parents, ctx, actions, interactive, tobackup)
+            _performrevert(repo, parents, ctx, names, actions, interactive,
+                           tobackup)
 
         if targetsubs:
             # Revert the subrepos on the revert list
@@ -3045,7 +3047,7 b' def revert(ui, repo, ctx, parents, *pats'
                     raise error.Abort("subrepository '%s' does not exist in %s!"
                                       % (sub, short(ctx.node())))
 
-def _performrevert(repo, parents, ctx, actions, interactive=False,
+def _performrevert(repo, parents, ctx, names, actions, interactive=False,
                    tobackup=None):
     """function that actually perform all the actions computed for revert
 
@@ -3070,16 +3072,23 b' def _performrevert(repo, parents, ctx, a'
             pass
         repo.dirstate.remove(f)
 
+    def prntstatusmsg(action, f):
+        rel, exact = names[f]
+        if repo.ui.verbose or not exact:
+            repo.ui.status(actions[action][1] % rel)
+
     audit_path = pathutil.pathauditor(repo.root, cached=True)
     for f in actions['forget'][0]:
         if interactive:
             choice = repo.ui.promptchoice(
                 _("forget added file %s (Yn)?$$ &Yes $$ &No") % f)
             if choice == 0:
+                prntstatusmsg('forget', f)
                 repo.dirstate.drop(f)
             else:
                 excluded_files.append(f)
         else:
+            prntstatusmsg('forget', f)
             repo.dirstate.drop(f)
     for f in actions['remove'][0]:
         audit_path(f)
@@ -3087,13 +3096,16 b' def _performrevert(repo, parents, ctx, a'
             choice = repo.ui.promptchoice(
                 _("remove added file %s (Yn)?$$ &Yes $$ &No") % f)
             if choice == 0:
+                prntstatusmsg('remove', f)
                 doremove(f)
             else:
                 excluded_files.append(f)
         else:
+            prntstatusmsg('remove', f)
             doremove(f)
     for f in actions['drop'][0]:
         audit_path(f)
+        prntstatusmsg('drop', f)
         repo.dirstate.remove(f)
 
     normal = None
@@ -3140,14 +3152,21 b' def _performrevert(repo, parents, ctx, a'
             tobackup = set()
         # Apply changes
         fp = stringio()
+        # `fnames` keeps track of filenames for which we have initiated changes,
+        # to make sure that we print status msg only once per file.
+        fnames = set()
         for c in chunks:
-            # Create a backup file only if this hunk should be backed up
-            if ishunk(c) and c.header.filename() in tobackup:
+            if ishunk(c):
                 abs = c.header.filename()
-                target = repo.wjoin(abs)
-                bakname = scmutil.origpath(repo.ui, repo, m.rel(abs))
-                util.copyfile(target, bakname)
-                tobackup.remove(abs)
+                if abs not in fnames:
+                    fnames.add(abs)
+                    prntstatusmsg('revert', abs)
+                # Create a backup file only if this hunk should be backed up
+                if c.header.filename() in tobackup:
+                    target = repo.wjoin(abs)
+                    bakname = scmutil.origpath(repo.ui, repo, m.rel(abs))
+                    util.copyfile(target, bakname)
+                    tobackup.remove(abs)
             c.write(fp)
         dopatch = fp.tell()
         fp.seek(0)
@@ -3159,6 +3178,7 b' def _performrevert(repo, parents, ctx, a'
         del fp
     else:
         for f in actions['revert'][0]:
+            prntstatusmsg('revert', f)
             checkout(f)
             if normal:
                 normal(f)
@@ -3166,6 +3186,7 b' def _performrevert(repo, parents, ctx, a'
     for f in actions['add'][0]:
         # Don't checkout modified files, they are already created by the diff
         if f not in newlyaddedandmodifiedfiles:
+            prntstatusmsg('add', f)
             checkout(f)
             repo.dirstate.add(f)
 
@@ -3173,6 +3194,7 b' def _performrevert(repo, parents, ctx, a'
     if node == parent and p2 == nullid:
         normal = repo.dirstate.normal
     for f in actions['undelete'][0]:
+        prntstatusmsg('undelete', f)
         checkout(f)
         normal(f)
 

tests/test-backout.t

@@ -147,8 +147,8 b' transaction: in-memory dirstate changes '
   $ hg debugstate --nodates
   n 644         12 set                 c
   $ hg backout -d '6 0' -m 'to be rollback-ed soon' -r .
+  removing c
   adding b
-  removing c
   changeset 6:4bfec048029d backs out changeset 5:fac0b729a654
   $ hg rollback -q
   $ hg status -A

tests/test-confused-revert.t

@@ -14,8 +14,8 b' Should show a removed and b added:'
   R a
 
   $ hg revert --all
+  forgetting b
   undeleting a
-  forgetting b
 
 Should show b unknown and a back to normal:
 
@@ -66,8 +66,8 b' Revert should fail:'
 Revert should be ok now:
 
   $ hg revert -r2 --all
+  forgetting b
   undeleting a
-  forgetting b
 
 Should show b unknown and a marked modified (merged):
 

tests/test-fileset-generated.t

@@ -187,11 +187,11 b' Test revert'
   undeleting missing_content2_missing-untracked
 
   $ hg revert 'set:deleted()'
+  forgetting content1_missing_missing-tracked
+  forgetting missing_missing_missing-tracked
   reverting content1_content1_missing-tracked
   reverting content1_content2_missing-tracked
-  forgetting content1_missing_missing-tracked
   reverting missing_content2_missing-tracked
-  forgetting missing_missing_missing-tracked
 
   $ hg revert 'set:unknown()'
 

tests/test-import-git.t

@@ -615,8 +615,8 b' Renames and strip'
 Prefix with strip, renames, creates etc
 
   $ hg revert -aC
+  forgetting b
   undeleting a
-  forgetting b
   $ rm b
   $ mkdir -p dir/dir2
   $ echo b > dir/dir2/b
@@ -715,10 +715,10 b' Renames, similarity and git diff'
 
   $ hg revert -aC
   forgetting dir/a
+  forgetting dir/dir2/b2
+  reverting dir/dir2/c
   undeleting dir/d
   undeleting dir/dir2/b
-  forgetting dir/dir2/b2
-  reverting dir/dir2/c
   $ rm dir/a dir/dir2/b2
   $ hg import --similarity 90 --no-commit - <<EOF
   > diff --git a/a b/b

tests/test-import.t

@@ -1014,8 +1014,8 b' test import with similarity and git and '
     a
   R a
   $ hg revert -a
+  forgetting b
   undeleting a
-  forgetting b
   $ cat b
   mod b
   $ rm b

tests/test-issue660.t

@@ -66,9 +66,9 b' Revert reintroducing shadow - should fai'
 Revert all - should succeed:
 
   $ hg revert --all
-  undeleting a
   forgetting a/a
   forgetting b
+  undeleting a
   undeleting b/b
 
   $ hg st

tests/test-largefiles-misc.t

@@ -1135,8 +1135,8 b' Move (and then undo) a directory move wi'
   ? large.orig
 
   $ hg revert --all
+  forgetting .hglf/dir/subdir2/large.bin
   undeleting .hglf/dir/subdir/large.bin
-  forgetting .hglf/dir/subdir2/large.bin
   reverting subrepo no-largefiles
 
   $ hg status -C
@@ -1214,8 +1214,8 b' Start from scratch, and rename something'
   large
 
   $ hg revert --all
+  forgetting .hglf/dir2/subdir/large.bin
   undeleting .hglf/dir/subdir/large.bin
-  forgetting .hglf/dir2/subdir/large.bin
   reverting subrepo no-largefiles
 
   $ hg status -C

tests/test-largefiles.t

@@ -1513,9 +1513,9 b' Test hg remove removes empty largefiles '
   $ cat sub/large4
   large4-modified
   $ hg revert -a --no-backup
-  undeleting .hglf/sub2/large6
   forgetting .hglf/sub2/large8
   reverting normal3
+  undeleting .hglf/sub2/large6
   $ hg status
   ? sub/large4.orig
   ? sub/normal4.orig

tests/test-lfs-test-server.t

@@ -694,10 +694,6 b' Revert will prefetch blobs in a group'
   $ rm *
   $ hg revert --all -r 1 --debug
   http auth: user foo, password ***
-  adding a
-  reverting b
-  reverting c
-  reverting d
   http auth: user foo, password ***
   Status: 200
   Content-Length: 905 (git-server !)
@@ -778,9 +774,13 b' Revert will prefetch blobs in a group'
   lfs: adding d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998 to the usercache
   lfs: processed: d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998
   lfs: downloaded 3 files (51 bytes)
+  reverting b
   lfs: found 31cf46fbc4ecd458a0943c5b4881f1f5a6dd36c53d6167d5b69ac45149b38e5b in the local lfs store
+  reverting c
   lfs: found d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998 in the local lfs store
+  reverting d
   lfs: found 37a65ab78d5ecda767e8622c248b5dbff1e68b1678ab0e730d5eb8601ec8ad19 in the local lfs store
+  adding a
   lfs: found 31cf46fbc4ecd458a0943c5b4881f1f5a6dd36c53d6167d5b69ac45149b38e5b in the local lfs store
 
 Check error message when the remote missed a blob:

tests/test-merge-remove.t

@@ -69,8 +69,8 b' Reverting foo1 and bar:'
 
   $ hg revert -vr . foo1 bar
   saving current version of bar as bar.orig
+  saving current version of foo1 as foo1.orig
   reverting bar
-  saving current version of foo1 as foo1.orig
   reverting foo1
 
   $ hg debugstate --nodates

tests/test-revert-interactive.t

@@ -51,11 +51,8 b' 10 run the same test than 8 from within '
   > n
   > n
   > EOF
-  reverting f
-  reverting folder1/g
+  remove added file folder1/i (Yn)? y
   removing folder1/i
-  reverting folder2/h
-  remove added file folder1/i (Yn)? y
   diff --git a/f b/f
   2 hunks, 2 lines changed
   examine changes to 'f'? [Ynesfdaq?] y
@@ -115,6 +112,8 b' 10 run the same test than 8 from within '
   2 hunks, 2 lines changed
   examine changes to 'folder2/h'? [Ynesfdaq?] n
   
+  reverting folder1/g
+  reverting f
   $ cat f
   1
   2
@@ -140,8 +139,6 b' 10 run the same test than 8 from within '
 Test that --interactive lift the need for --all
 
   $ echo q | hg revert -i -r 2
-  reverting folder1/g
-  reverting folder2/h
   diff --git a/folder1/g b/folder1/g
   1 hunks, 1 lines changed
   examine changes to 'folder1/g'? [Ynesfdaq?] q
@@ -197,10 +194,6 b' Test --no-backup'
   > n
   > n
   > EOF
-  reverting f
-  reverting folder1/g
-  removing folder1/i
-  reverting folder2/h
   remove added file folder1/i (Yn)? n
   diff --git a/f b/f
   2 hunks, 2 lines changed
@@ -250,6 +243,8 b' Test --no-backup'
   2 hunks, 2 lines changed
   examine changes to 'folder2/h'? [Ynesfdaq?] n
   
+  reverting folder1/g
+  reverting f
   $ cat f
   1
   2
@@ -354,7 +349,6 b' 3) Use interactive revert with editing ('
   > y
   > e
   > EOF
-  reverting k
   diff --git a/k b/k
   1 hunks, 2 lines changed
   examine changes to 'k'? [Ynesfdaq?] y
@@ -365,6 +359,7 b' 3) Use interactive revert with editing ('
   +2
   discard this change to 'k'? [Ynesfdaq?] e
   
+  reverting k
   $ cat k
   42
 
@@ -378,15 +373,14 b' 3) Use interactive revert with editing ('
   $ hg revert -i <<EOF
   > n
   > EOF
-  forgetting newfile
   forget added file newfile (Yn)? n
   $ hg status
   A newfile
   $ hg revert -i <<EOF
   > y
   > EOF
+  forget added file newfile (Yn)? y
   forgetting newfile
-  forget added file newfile (Yn)? y
   $ hg status
   ? newfile
 
@@ -406,7 +400,6 b' When a line without EOL is selected duri'
   > y
   > y
   > EOF
-  reverting a
   diff --git a/a b/a
   1 hunks, 1 lines changed
   examine changes to 'a'? [Ynesfdaq?] y
@@ -417,6 +410,7 b' When a line without EOL is selected duri'
   \ No newline at end of file
   apply this change to 'a'? [Ynesfdaq?] y
   
+  reverting a
   $ cat a
   0
 

tests/test-revert.t

@@ -129,9 +129,9 b' revert to another revision (--rev)'
 ----------------------------------
 
   $ hg revert --all -r0
-  adding a
+  forgetting z
   removing d
-  forgetting z
+  adding a
 
 revert explicitly to parent (--rev)
 -----------------------------------
@@ -283,8 +283,8 b' Issue332: confusing message when reverti'
   $ echo foo > newdir/newfile
   $ hg add newdir/newfile
   $ hg revert b newdir
+  forgetting newdir/newfile
   reverting b/b
-  forgetting newdir/newfile
   $ echo foobar > b/b
   $ hg revert .
   reverting b/b
@@ -368,9 +368,9 b' copies and renames, you have no chance t'
   $ hg update '.^'
   1 files updated, 0 files merged, 2 files removed, 0 files unresolved
   $ hg revert -rtip -a
+  removing ignored
   adding allyour
   adding base
-  removing ignored
   $ hg status -C
   A allyour
     ignored
@@ -790,28 +790,28 b' Test revert --all to parent content'
 check revert output
 
   $ hg revert --all
-  undeleting content1_content1_content1-untracked
-  reverting content1_content1_content3-tracked
-  undeleting content1_content1_content3-untracked
-  reverting content1_content1_missing-tracked
-  undeleting content1_content1_missing-untracked
-  reverting content1_content2_content1-tracked
-  undeleting content1_content2_content1-untracked
-  undeleting content1_content2_content2-untracked
-  reverting content1_content2_content3-tracked
-  undeleting content1_content2_content3-untracked
-  reverting content1_content2_missing-tracked
-  undeleting content1_content2_missing-untracked
   forgetting content1_missing_content1-tracked
   forgetting content1_missing_content3-tracked
   forgetting content1_missing_missing-tracked
-  undeleting missing_content2_content2-untracked
-  reverting missing_content2_content3-tracked
-  undeleting missing_content2_content3-untracked
-  reverting missing_content2_missing-tracked
-  undeleting missing_content2_missing-untracked
   forgetting missing_missing_content3-tracked
   forgetting missing_missing_missing-tracked
+  reverting content1_content1_content3-tracked
+  reverting content1_content1_missing-tracked
+  reverting content1_content2_content1-tracked
+  reverting content1_content2_content3-tracked
+  reverting content1_content2_missing-tracked
+  reverting missing_content2_content3-tracked
+  reverting missing_content2_missing-tracked
+  undeleting content1_content1_content1-untracked
+  undeleting content1_content1_content3-untracked
+  undeleting content1_content1_missing-untracked
+  undeleting content1_content2_content1-untracked
+  undeleting content1_content2_content2-untracked
+  undeleting content1_content2_content3-untracked
+  undeleting content1_content2_missing-untracked
+  undeleting missing_content2_content2-untracked
+  undeleting missing_content2_content3-untracked
+  undeleting missing_content2_missing-untracked
 
 Compare resulting directory with revert target.
 
@@ -847,28 +847,28 b' Test revert --all to "base" content'
 check revert output
 
   $ hg revert --all --rev 'desc(base)'
-  undeleting content1_content1_content1-untracked
-  reverting content1_content1_content3-tracked
-  undeleting content1_content1_content3-untracked
-  reverting content1_content1_missing-tracked
-  undeleting content1_content1_missing-untracked
-  undeleting content1_content2_content1-untracked
-  reverting content1_content2_content2-tracked
-  undeleting content1_content2_content2-untracked
-  reverting content1_content2_content3-tracked
-  undeleting content1_content2_content3-untracked
-  reverting content1_content2_missing-tracked
-  undeleting content1_content2_missing-untracked
-  adding content1_missing_content1-untracked
-  reverting content1_missing_content3-tracked
-  adding content1_missing_content3-untracked
-  reverting content1_missing_missing-tracked
-  adding content1_missing_missing-untracked
+  forgetting missing_missing_content3-tracked
+  forgetting missing_missing_missing-tracked
   removing missing_content2_content2-tracked
   removing missing_content2_content3-tracked
   removing missing_content2_missing-tracked
-  forgetting missing_missing_content3-tracked
-  forgetting missing_missing_missing-tracked
+  reverting content1_content1_content3-tracked
+  reverting content1_content1_missing-tracked
+  reverting content1_content2_content2-tracked
+  reverting content1_content2_content3-tracked
+  reverting content1_content2_missing-tracked
+  reverting content1_missing_content3-tracked
+  reverting content1_missing_missing-tracked
+  adding content1_missing_content1-untracked
+  adding content1_missing_content3-untracked
+  adding content1_missing_missing-untracked
+  undeleting content1_content1_content1-untracked
+  undeleting content1_content1_content3-untracked
+  undeleting content1_content1_missing-untracked
+  undeleting content1_content2_content1-untracked
+  undeleting content1_content2_content2-untracked
+  undeleting content1_content2_content3-untracked
+  undeleting content1_content2_missing-untracked
 
 Compare resulting directory with revert target.
 
@@ -1120,8 +1120,8 b' actual tests: reverting to something els'
   M A
   A B
   $ hg revert --rev 1 --all
+  removing B
   reverting A
-  removing B
   $ hg status --rev 1
 
 From the other parents
@@ -1140,8 +1140,8 b' From the other parents'
   M A
   A B
   $ hg revert --rev 1 --all
+  removing B
   reverting A
-  removing B
   $ hg status --rev 1
 
   $ cd ..