Patchwork D3209: amend: exit 0 if there are no changes

login
register
mail settings
Submitter phabricator
Date April 9, 2018, 10:10 p.m.
Message ID <differential-rev-PHID-DREV-lbe2j4vjvnews3vtlues-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/30614/
State New
Headers show

Comments

phabricator - April 9, 2018, 10:10 p.m.
spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  It's useful for writing some scripts or shell aliases if `hg amend` doesn't
  abort the series of commands if there's nothing to amend.  Users can then have
  something like "alias amendevo='hg amend && hg evolve -a'" to amend and evolve
  in one step.
  
  To do this otherwise would require something like this:
  
    alias amendevo='[[ -z "$(hg status -mar)" ]] && hg amend && hg evolve -a'
  
  While that's not too onerous, it's not immediately obvious to most that this is
  what would have to be done, so it's a small hurdle that users either have to
  spend time to figure out on their own or ask about.
  
  A similar change was made to `hg evolve` recently, for similar reasons.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/commands.py
  tests/test-amend.t
  tests/test-commit-amend.t

CHANGE DETAILS




To: spectral, #hg-reviewers
Cc: mercurial-devel
phabricator - April 9, 2018, 11:37 p.m.
quark added a comment.


  Returning 1 is actually more consistent with other core commands like pull, push, commit. See https://www.mercurial-scm.org/pipermail/mercurial-devel/2012-January/037711.html. Scripts should be updated to use `$?` explicitly.

REPOSITORY
  rHG Mercurial

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

To: spectral, #hg-reviewers
Cc: quark, mercurial-devel
phabricator - April 10, 2018, 12:20 a.m.
spectral added a comment.


  In https://phab.mercurial-scm.org/D3209#51512, @quark wrote:
  
  > Returning 1 is actually more consistent with other core commands like pull, push, commit. See https://www.mercurial-scm.org/pipermail/mercurial-devel/2012-January/037711.html. Scripts should be updated to use `$?` explicitly.
  
  
  I'm not sure how scripts can be updated to use $? explicitly.  Mercurial exits 1 in many situations:
  
  - nothing found
  - hg verify, if there's any problems
  - unresolved conflicts when updating/rebasing/whatever
  - histedit 'edit' seems to dump you back into a terminal with exit status 1
  - MQ seems to use it a lot
  - I think a few others?
  
  Yes, none of these apply to `hg amend`, so doing something like `hg amend; [[ $? -eq 1 ]] && hg evolve -a` is feasible, but again, feels really weird and non-discoverable. :/
  
  I also see a few cases that return 2, mostly in MQ stuff (though there is one case in test-bookmarks-pushpull.t that's a little odd).  There's obviously the "255" return whenever there's a major problem.
  
  I'm not too tied to the patch and can abandon it if people feel it's incorrect.

REPOSITORY
  rHG Mercurial

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

To: spectral, #hg-reviewers
Cc: quark, mercurial-devel
phabricator - April 10, 2018, 1:51 a.m.
quark added a comment.


  I think it depends on what scripts want to do. I guess mpm's original point is, suppose you have a build script, or something that should do nothing if nothing changed, then `hg amend && build_script` just works as expected. If amend returns 0, then it'd be more complex to detect "nothing changed" case.

REPOSITORY
  rHG Mercurial

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

To: spectral, #hg-reviewers
Cc: quark, mercurial-devel

Patch

diff --git a/tests/test-commit-amend.t b/tests/test-commit-amend.t
--- a/tests/test-commit-amend.t
+++ b/tests/test-commit-amend.t
@@ -22,7 +22,6 @@ 
 
   $ hg ci --amend -m 'base1'
   nothing changed
-  [1]
 
   $ cat >> $HGRCPATH <<EOF
   > [hooks]
diff --git a/tests/test-amend.t b/tests/test-amend.t
--- a/tests/test-amend.t
+++ b/tests/test-amend.t
@@ -83,15 +83,12 @@ 
 
   $ hg amend
   nothing changed
-  [1]
 
   $ hg amend -d "0 0"
   nothing changed
-  [1]
 
   $ hg amend -d "Thu Jan 01 00:00:00 1970 UTC"
   nothing changed
-  [1]
 
 Matcher and metadata options
 
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1592,7 +1592,9 @@ 
         node = cmdutil.amend(ui, repo, old, extra, pats, opts)
         if node == old.node():
             ui.status(_("nothing changed\n"))
-            return 1
+            # Returning 0 because this is not technically an error condition,
+            # and it makes some scripts a little easier to write.
+            return 0
     else:
         def commitfunc(ui, repo, message, match, opts):
             overrides = {}
@@ -1622,6 +1624,9 @@ 
                             "'hg status')\n") % len(stat[3]))
             else:
                 ui.status(_("nothing changed\n"))
+            # "amend" not doing something in a script might be expected;
+            # "commit" not doing something in a script is generally a bug
+            # somewhere, so we return non-zero here.
             return 1
 
     cmdutil.commitstatus(repo, node, branch, bheads, opts)