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

khanchi97 (Sushil khanchi) phabricator at mercurial-scm.org
Sun Aug 26 23:06:55 UTC 2018


khanchi97 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Before this patch we were priting every action msg before actually
  performing that action and that would result in inconsistencies like
  if user decided to not revert changes in a file foo, while he is
  running --interactive mode, still there will be a msg on console
  saying "reverting foo".
  
  To fix this issue, I have made some changes to show an action msg only
  when we are just about to perform that action (when user decided to
  make some changes in interactive mode).
  
  There are also some changes in test-revert.t because of the change in
  order of messages, review this file because there may be some changes
  that should not be there.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4380

AFFECTED FILES
  mercurial/cmdutil.py
  tests/test-revert-interactive.t
  tests/test-revert.t

CHANGE DETAILS

diff --git a/tests/test-revert.t b/tests/test-revert.t
--- a/tests/test-revert.t
+++ b/tests/test-revert.t
@@ -41,20 +41,23 @@
 revert removal of a file
 
   $ hg revert a
+  undeleting a
   $ hg status
   M c
   A b
 
 revert addition of a file
 
   $ hg revert b
+  forgetting b
   $ hg status
   M c
   ? b
 
 revert modification of a file (--no-backup)
 
   $ hg revert --no-backup c
+  reverting c
   $ hg status
   ? b
 
@@ -129,9 +132,9 @@
 ----------------------------------
 
   $ hg revert --all -r0
-  adding a
+  forgetting z
   removing d
-  forgetting z
+  adding a
 
 revert explicitly to parent (--rev)
 -----------------------------------
@@ -147,6 +150,7 @@
 exact match are more silent
 
   $ hg revert -r0 a
+  adding a
   $ hg st a
   A a
   $ hg rm d
@@ -214,6 +218,7 @@
   > fakedirstatewritetime = $TESTDIR/fakedirstatewritetime.py
   > EOF
   $ hg revert -r 0 e
+  reverting e
   $ cat >> .hg/hgrc <<EOF
   > [extensions]
   > fakedirstatewritetime = !
@@ -283,8 +288,8 @@
   $ 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
@@ -295,13 +300,17 @@
 
   $ hg mv a newa
   $ hg revert newa
+  forgetting newa
+  undeleting a
   $ hg st a newa
   ? newa
 
 Also true for move overwriting an existing file
 
   $ hg mv --force a b/b
   $ hg revert b/b
+  reverting b/b
+  undeleting a
   $ hg status a b/b
 
   $ cd ..
@@ -349,6 +358,8 @@
 --------------------------------------
 
   $ hg revert --no-backup ignored removed
+  reverting ignored
+  undeleting removed
   $ hg st -mardi
 
 Reverting copy (issue3920)
@@ -368,9 +379,9 @@
   $ 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 +801,28 @@
 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 +858,28 @@
 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.
 
@@ -911,66 +922,88 @@
   no changes needed to content1_content1_content1-tracked
   
   ### revert for: content1_content1_content1-untracked
+  undeleting content1_content1_content1-untracked
   
   ### revert for: content1_content1_content3-tracked
+  reverting content1_content1_content3-tracked
   
   ### revert for: content1_content1_content3-untracked
+  undeleting content1_content1_content3-untracked
   
   ### revert for: content1_content1_missing-tracked
+  reverting content1_content1_missing-tracked
   
   ### revert for: content1_content1_missing-untracked
+  undeleting content1_content1_missing-untracked
   
   ### revert for: content1_content2_content1-tracked
+  reverting content1_content2_content1-tracked
   
   ### revert for: content1_content2_content1-untracked
+  undeleting content1_content2_content1-untracked
   
   ### revert for: content1_content2_content2-tracked
   no changes needed to content1_content2_content2-tracked
   
   ### revert for: content1_content2_content2-untracked
+  undeleting content1_content2_content2-untracked
   
   ### revert for: content1_content2_content3-tracked
+  reverting content1_content2_content3-tracked
   
   ### revert for: content1_content2_content3-untracked
+  undeleting content1_content2_content3-untracked
   
   ### revert for: content1_content2_missing-tracked
+  reverting content1_content2_missing-tracked
   
   ### revert for: content1_content2_missing-untracked
+  undeleting content1_content2_missing-untracked
   
   ### revert for: content1_missing_content1-tracked
+  forgetting content1_missing_content1-tracked
   
   ### revert for: content1_missing_content1-untracked
   file not managed: content1_missing_content1-untracked
   
   ### revert for: content1_missing_content3-tracked
+  forgetting content1_missing_content3-tracked
   
   ### revert for: content1_missing_content3-untracked
   file not managed: content1_missing_content3-untracked
   
   ### revert for: content1_missing_missing-tracked
+  forgetting content1_missing_missing-tracked
   
   ### revert for: content1_missing_missing-untracked
   content1_missing_missing-untracked: no such file in rev * (glob)
   
   ### revert for: missing_content2_content2-tracked
   no changes needed to missing_content2_content2-tracked
   
   ### revert for: missing_content2_content2-untracked
+  undeleting missing_content2_content2-untracked
   
   ### revert for: missing_content2_content3-tracked
+  reverting missing_content2_content3-tracked
   
   ### revert for: missing_content2_content3-untracked
+  undeleting missing_content2_content3-untracked
   
   ### revert for: missing_content2_missing-tracked
+  reverting missing_content2_missing-tracked
   
   ### revert for: missing_content2_missing-untracked
+  undeleting missing_content2_missing-untracked
   
   ### revert for: missing_missing_content3-tracked
+  forgetting missing_missing_content3-tracked
   
   ### revert for: missing_missing_content3-untracked
   file not managed: missing_missing_content3-untracked
   
   ### revert for: missing_missing_missing-tracked
+  forgetting missing_missing_missing-tracked
   
   ### revert for: missing_missing_missing-untracked
   missing_missing_missing-untracked: no such file in rev * (glob)
@@ -1004,66 +1037,88 @@
   no changes needed to content1_content1_content1-tracked
   
   ### revert for: content1_content1_content1-untracked
+  undeleting content1_content1_content1-untracked
   
   ### revert for: content1_content1_content3-tracked
+  reverting content1_content1_content3-tracked
   
   ### revert for: content1_content1_content3-untracked
+  undeleting content1_content1_content3-untracked
   
   ### revert for: content1_content1_missing-tracked
+  reverting content1_content1_missing-tracked
   
   ### revert for: content1_content1_missing-untracked
+  undeleting content1_content1_missing-untracked
   
   ### revert for: content1_content2_content1-tracked
   no changes needed to content1_content2_content1-tracked
   
   ### revert for: content1_content2_content1-untracked
+  undeleting content1_content2_content1-untracked
   
   ### revert for: content1_content2_content2-tracked
+  reverting content1_content2_content2-tracked
   
   ### revert for: content1_content2_content2-untracked
+  undeleting content1_content2_content2-untracked
   
   ### revert for: content1_content2_content3-tracked
+  reverting content1_content2_content3-tracked
   
   ### revert for: content1_content2_content3-untracked
+  undeleting content1_content2_content3-untracked
   
   ### revert for: content1_content2_missing-tracked
+  reverting content1_content2_missing-tracked
   
   ### revert for: content1_content2_missing-untracked
+  undeleting content1_content2_missing-untracked
   
   ### revert for: content1_missing_content1-tracked
   no changes needed to content1_missing_content1-tracked
   
   ### revert for: content1_missing_content1-untracked
+  adding content1_missing_content1-untracked
   
   ### revert for: content1_missing_content3-tracked
+  reverting content1_missing_content3-tracked
   
   ### revert for: content1_missing_content3-untracked
+  adding content1_missing_content3-untracked
   
   ### revert for: content1_missing_missing-tracked
+  reverting content1_missing_missing-tracked
   
   ### revert for: content1_missing_missing-untracked
+  adding content1_missing_missing-untracked
   
   ### revert for: missing_content2_content2-tracked
+  removing missing_content2_content2-tracked
   
   ### revert for: missing_content2_content2-untracked
   no changes needed to missing_content2_content2-untracked
   
   ### revert for: missing_content2_content3-tracked
+  removing missing_content2_content3-tracked
   
   ### revert for: missing_content2_content3-untracked
   no changes needed to missing_content2_content3-untracked
   
   ### revert for: missing_content2_missing-tracked
+  removing missing_content2_missing-tracked
   
   ### revert for: missing_content2_missing-untracked
   no changes needed to missing_content2_missing-untracked
   
   ### revert for: missing_missing_content3-tracked
+  forgetting missing_missing_content3-tracked
   
   ### revert for: missing_missing_content3-untracked
   file not managed: missing_missing_content3-untracked
   
   ### revert for: missing_missing_missing-tracked
+  forgetting missing_missing_missing-tracked
   
   ### revert for: missing_missing_missing-untracked
   missing_missing_missing-untracked: no such file in rev * (glob)
@@ -1120,8 +1175,8 @@
   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 +1195,8 @@
   M A
   A B
   $ hg revert --rev 1 --all
+  removing B
   reverting A
-  removing B
   $ hg status --rev 1
 
   $ cd ..
diff --git a/tests/test-revert-interactive.t b/tests/test-revert-interactive.t
--- a/tests/test-revert-interactive.t
+++ b/tests/test-revert-interactive.t
@@ -51,11 +51,8 @@
   > 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 @@
   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 @@
 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
@@ -180,6 +177,7 @@
   -d
   apply this change to 'folder1/g'? [Ynesfdaq?] y
   
+  reverting folder1/g
   $ ls folder1/
   g
   >>> open('folder1/g', 'wb').write(b"1\n2\n3\n4\n5\nd\n") and None
@@ -197,10 +195,6 @@
   > 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 +244,8 @@
   2 hunks, 2 lines changed
   examine changes to 'folder2/h'? [Ynesfdaq?] n
   
+  reverting folder1/g
+  reverting f
   $ cat f
   1
   2
@@ -314,6 +310,7 @@
   -b
   discard change 2/2 to 'f'? [Ynesfdaq?] n
   
+  reverting f
   $ hg st
   M f
   M folder1/g
@@ -354,7 +351,6 @@
   > y
   > e
   > EOF
-  reverting k
   diff --git a/k b/k
   1 hunks, 2 lines changed
   examine changes to 'k'? [Ynesfdaq?] y
@@ -365,6 +361,7 @@
   +2
   discard this change to 'k'? [Ynesfdaq?] e
   
+  reverting k
   $ cat k
   42
 
@@ -378,15 +375,14 @@
   $ 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 +402,6 @@
   > y
   > y
   > EOF
-  reverting a
   diff --git a/a b/a
   1 hunks, 1 lines changed
   examine changes to 'a'? [Ynesfdaq?] y
@@ -417,6 +412,7 @@
   \ No newline at end of file
   apply this change to 'a'? [Ynesfdaq?] y
   
+  reverting a
   $ cat a
   0
 
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -3021,7 +3021,8 @@
                     if ui.verbose or not exact:
                         if not isinstance(msg, bytes):
                             msg = msg(abs)
-                        ui.status(msg % rel)
+                        if opts.get('dry_run'):
+                            ui.status(msg % rel)
                 elif exact:
                     ui.warn(msg % rel)
                 break
@@ -3077,24 +3078,29 @@
             choice = repo.ui.promptchoice(
                 _("forget added file %s (Yn)?$$ &Yes $$ &No") % f)
             if choice == 0:
+                repo.ui.status(_("%s") % (actions['forget'][1] % f))
                 repo.dirstate.drop(f)
             else:
                 excluded_files.append(f)
         else:
+            repo.ui.status(_("%s") % (actions['forget'][1] % f))
             repo.dirstate.drop(f)
     for f in actions['remove'][0]:
         audit_path(f)
         if interactive:
             choice = repo.ui.promptchoice(
                 _("remove added file %s (Yn)?$$ &Yes $$ &No") % f)
             if choice == 0:
+                repo.ui.status(_("%s") % (actions['remove'][1] % f))
                 doremove(f)
             else:
                 excluded_files.append(f)
         else:
+            repo.ui.status(_("%s") % (actions['remove'][1] % f))
             doremove(f)
     for f in actions['drop'][0]:
         audit_path(f)
+        repo.ui.status(_("%s") % (actions['drop'][1] % f))
         repo.dirstate.remove(f)
 
     normal = None
@@ -3141,14 +3147,21 @@
             tobackup = set()
         # Apply changes
         fp = stringio()
+        # `fnames` keep track of filenames for which we have initiated changes,
+        # to make sure that we print status msg only once for a file.
+        fnames = []
         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.append(abs)
+                    repo.ui.status(_("%s") % (actions['revert'][1] % 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)
@@ -3160,21 +3173,24 @@
         del fp
     else:
         for f in actions['revert'][0]:
+            repo.ui.status(_("%s") % (actions['revert'][1] % f))
             checkout(f)
             if normal:
                 normal(f)
 
     for f in actions['add'][0]:
         # Don't checkout modified files, they are already created by the diff
         if f not in newlyaddedandmodifiedfiles:
             checkout(f)
+            repo.ui.status(_("%s") % (actions['add'][1] % f))
             repo.dirstate.add(f)
 
     normal = repo.dirstate.normallookup
     if node == parent and p2 == nullid:
         normal = repo.dirstate.normal
     for f in actions['undelete'][0]:
         checkout(f)
+        repo.ui.status(_("%s") % (actions['undelete'][1] % f))
         normal(f)
 
     copied = copies.pathcopies(repo[parent], ctx)



To: khanchi97, #hg-reviewers
Cc: mercurial-devel


More information about the Mercurial-devel mailing list