Patchwork D7993: merge: use check_incompatible_arguments() for --abort

login
register
mail settings
Submitter phabricator
Date Jan. 25, 2020, 12:17 a.m.
Message ID <differential-rev-PHID-DREV-2mtbfltjotxg2yughr43-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/44642/
State Superseded
Headers show

Comments

phabricator - Jan. 25, 2020, 12:17 a.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - Jan. 27, 2020, 10:28 a.m.
> +    cmdutil.check_incompatible_arguments(opts, b'abort', b'rev', b'preview')

It's a bit late, but I feel the arguments of `check_incompatible_arguments()`
is confusing since `b'abort'` isn't the same kind of arguments as the others.
I think `(opts, b'abort', [b'rev', b'preview'])` is more explicit.
phabricator - Jan. 27, 2020, 10:32 a.m.
yuja added a comment.


  > +    cmdutil.check_incompatible_arguments(opts, b'abort', b'rev', b'preview')
  
  It's a bit late, but I feel the arguments of `check_incompatible_arguments()`
  is confusing since `b'abort'` isn't the same kind of arguments as the others.
  I think `(opts, b'abort', [b'rev', b'preview'])` is more explicit.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7993/new/

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

To: martinvonz, #hg-reviewers, pulkit
Cc: yuja, mercurial-devel
phabricator - Jan. 27, 2020, 5:42 p.m.
martinvonz added a comment.


  In D7993#118162 <https://phab.mercurial-scm.org/D7993#118162>, @yuja wrote:
  
  >> +    cmdutil.check_incompatible_arguments(opts, b'abort', b'rev', b'preview')
  >
  > It's a bit late, but I feel the arguments of `check_incompatible_arguments()`
  > is confusing since `b'abort'` isn't the same kind of arguments as the others.
  > I think `(opts, b'abort', [b'rev', b'preview'])` is more explicit.
  
  Good point. I've sent D8018 <https://phab.mercurial-scm.org/D8018>.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7993/new/

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

To: martinvonz, #hg-reviewers, pulkit
Cc: yuja, mercurial-devel

Patch

diff --git a/tests/test-commit-unresolved.t b/tests/test-commit-unresolved.t
--- a/tests/test-commit-unresolved.t
+++ b/tests/test-commit-unresolved.t
@@ -60,7 +60,7 @@ 
   abort: cannot specify a node with --abort
   [255]
   $ hg merge --abort --rev e4501
-  abort: cannot specify both --rev and --abort
+  abort: cannot specify both --abort and --rev
   [255]
 
 #if abortcommand
@@ -144,7 +144,7 @@ 
   (branch merge, don't forget to commit)
 
   $ hg merge --preview --abort
-  abort: cannot specify --preview with --abort
+  abort: cannot specify both --abort and --preview
   [255]
 
   $ hg abort
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4847,6 +4847,7 @@ 
     abort = opts.get(b'abort')
     if abort and repo.dirstate.p2() == nullid:
         cmdutil.wrongtooltocontinue(repo, _(b'merge'))
+    cmdutil.check_incompatible_arguments(opts, b'abort', b'rev', b'preview')
     if abort:
         state = cmdutil.getunfinishedstate(repo)
         if state and state._opname != b'merge':
@@ -4856,10 +4857,6 @@ 
             )
         if node:
             raise error.Abort(_(b"cannot specify a node with --abort"))
-        if opts.get(b'rev'):
-            raise error.Abort(_(b"cannot specify both --rev and --abort"))
-        if opts.get(b'preview'):
-            raise error.Abort(_(b"cannot specify --preview with --abort"))
     if opts.get(b'rev') and node:
         raise error.Abort(_(b"please specify just one revision"))
     if not node: