This is an archive of the discontinued Mercurial Phabricator instance.

clone: extract helper for checking mutually exclusive args
ClosedPublic

Authored by martinvonz on Dec 13 2019, 2:53 AM.

Details

Summary

We have some duplicated code for aborting if the user provided
mutually exclusive arguments. Extensions surely have more such
code. We also have duplicated translations and inconsistent output in
this area.

This patch introduces a simpler helper for checking if more than one
option among a given set was given on the command line. I've made the
clone code call the function to show that it works.

The function has no good way of checking arguments with hyphens in
them. I'll add that later if necessary. The function still won't be
applicable in all cases, but I think it's still better than nothing.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Dec 13 2019, 2:53 AM
dlax added a subscriber: dlax.Dec 13 2019, 3:18 AM

Useful refactoring!

mercurial/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?

martinvonz added inline comments.Dec 13 2019, 3:24 AM
mercurial/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.

pulkit added a subscriber: pulkit.Dec 16 2019, 12:51 AM
pulkit added inline comments.
mercurial/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 ;)

martinvonz updated this revision to Diff 18759.Dec 16 2019, 6:18 PM
martinvonz added inline comments.Dec 16 2019, 6:18 PM
mercurial/cmdutil.py
263

Done. It ended up exactly the same length with the arugment -> arg change :)

pulkit accepted this revision.Dec 17 2019, 2:09 AM
This revision is now accepted and ready to land.Dec 17 2019, 2:09 AM