Submitter | phabricator |
---|---|
Date | Dec. 13, 2019, 7:56 a.m. |
Message ID | <differential-rev-PHID-DREV-7pst3ev7ep5ef5wnawyt-req@mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/43783/ |
State | Superseded |
Headers | show |
Comments
dlax added a comment. Useful refactoring! INLINE COMMENTS > cmdutil.py:263 > > +def check_unique_argument(opts, *args): > + """abort if more than one of the arguments are in opts""" The function name is a bit misleading; it looks like it would check that an option wasn't specified multiple times. Perhaps `check_exclusive_arguments`? REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7633/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7633 To: martinvonz, #hg-reviewers Cc: dlax, mercurial-devel
martinvonz added inline comments. INLINE COMMENTS > dlax wrote in cmdutil.py:263 > The function name is a bit misleading; it looks like it would check that an option wasn't specified multiple times. Perhaps `check_exclusive_arguments`? I don't think I'd be able to guess what a function by that name did either. `check_at_most_one_argument` seems clearer, but maybe too long. I'll wait a bit for other ideas before I change anything. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7633/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7633 To: martinvonz, #hg-reviewers Cc: dlax, mercurial-devel
pulkit added inline comments. INLINE COMMENTS > martinvonz wrote in cmdutil.py:263 > I don't think I'd be able to guess what a function by that name did either. `check_at_most_one_argument` seems clearer, but maybe too long. I'll wait a bit for other ideas before I change anything. I agree that the current name can be bit misleading. `check_at_most_one_argument` seems clearer. Maybe `argument` -> `arg` will make it bit smaller ;) REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7633/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7633 To: martinvonz, #hg-reviewers Cc: pulkit, dlax, mercurial-devel
martinvonz added inline comments. INLINE COMMENTS > pulkit wrote in cmdutil.py:263 > I agree that the current name can be bit misleading. `check_at_most_one_argument` seems clearer. Maybe `argument` -> `arg` will make it bit smaller ;) Done. It ended up exactly the same length with the `arugment` -> `arg` change :) REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7633/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7633 To: martinvonz, #hg-reviewers Cc: pulkit, dlax, mercurial-devel
Patch
diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -1889,8 +1889,7 @@ Returns 0 on success. """ opts = pycompat.byteskwargs(opts) - if opts.get(b'noupdate') and opts.get(b'updaterev'): - raise error.Abort(_(b"cannot specify both --noupdate and --updaterev")) + cmdutil.check_unique_argument(opts, b'noupdate', b'updaterev') # --include/--exclude can come from narrow or sparse. includepats, excludepats = None, None diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -260,6 +260,18 @@ _linebelow = b"^HG: ------------------------ >8 ------------------------$" +def check_unique_argument(opts, *args): + """abort if more than one of the arguments are in opts""" + previous = None + for x in args: + if opts.get(x): + if previous: + raise error.Abort( + _(b'cannot specify both --%s and --%s') % (previous, x) + ) + previous = x + + def resolvecommitoptions(ui, opts): """modify commit options dict to handle related options