[PATCH] bisect: improve option validation message

Brandon McCaig bamccaig at gmail.com
Sat Jun 10 17:43:32 EDT 2017


On Sat, Jun 10, 2017 at 09:36:25AM -0400, Augie Fackler wrote:
> > +    for pair in itertools.product(incompatibles.keys(), repeat=2):
> > +      if pair[0] < pair[1]:
> 
> I’m not quite sure what this inequality is doing - can you
> elaborate? How about this construct?

The comparison was to eliminate matching pairs and duplicates (I
was not aware of permuations or combinations at the time, though
I wonder if this is any slower).

> for left, right in {tuple(sorted(pair)) for pair in itertools.permutations(incompatibles, 2)}:
>   if incompatibles[left] and incompatibles[right]:
>     raise error.Abort(…)

This is similar to itertools.combinations() that 'sfink'
suggested in IRC, which I think fits a bit better since we don't
need all permutations.

> (Mine may also be way too clever - the set comprehension and
> the tuple(sorted(pair)) dance seems clear to me, but I wrote
> it.)

It was a bit to take in at first. I guess the set comprehension
is there to eliminate the redundant permutations, which
combinations() can do for us instead. I don't like how long the
line gets either.

Now without the equality check sorting is necessary if we care
what order the checks are done in. I prefer it this way, but
maybe others don't care? I guess sorting the keys should be as
good as anything. Unpacking the tuple is probably nicer than
indexing the tuple too (though it works out to more characters
here). So how about this:

# HG changeset patch
# User Brandon McCaig <bamccaig at gmail.com>
# Date 1497053559 14400
#      Fri Jun 09 20:12:39 2017 -0400
# Node ID d1f2ee1cefc15f9043139c835487917d0831485c
# Parent  7e9d0d8ff938dcf8ca193c17db5321a05a48e718
bisect: Make option validation message more consistent with hg

Augie suggested improving upon the wording/formatting of the validation
messages as a follow up to a previous patch.

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -9,6 +9,7 @@
 
 import difflib
 import errno
+import itertools
 import os
 import re
 import sys
@@ -770,9 +771,19 @@
             reset = True
     elif extra:
         raise error.Abort(_('incompatible arguments'))
-    elif good + bad + skip + reset + extend + bool(command) > 1:
-        raise error.Abort(_('-g|--good, -b|--bad, -s|--skip, -r|--reset, '
-                            '-e|--extend, and -c|--command are exclusive'))
+
+    incompatibles = {
+        '--bad': bad,
+        '--command': bool(command),
+        '--extend': extend,
+        '--good': good,
+        '--reset': reset,
+        '--skip': skip,
+    }
+
+    for left, right in itertools.combinations(sorted(incompatibles), 2):
+      if incompatibles[left] and incompatibles[right]:
+        raise error.Abort(_('%s and %s are incompatible') % (left, right))
 
     if reset:
         hbisect.resetstate(repo)
diff --git a/tests/test-bisect.t b/tests/test-bisect.t
--- a/tests/test-bisect.t
+++ b/tests/test-bisect.t
@@ -615,47 +615,47 @@
 
   $ hg bisect -r
   $ hg bisect -b -c false
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --bad and --command are incompatible
   [255]
   $ hg bisect -b -e
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --bad and --extend are incompatible
   [255]
   $ hg bisect -b -g
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --bad and --good are incompatible
   [255]
   $ hg bisect -b -r
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --bad and --reset are incompatible
   [255]
   $ hg bisect -b -s
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --bad and --skip are incompatible
   [255]
   $ hg bisect -c false -e
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --command and --extend are incompatible
   [255]
   $ hg bisect -c false -g
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --command and --good are incompatible
   [255]
   $ hg bisect -c false -r
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --command and --reset are incompatible
   [255]
   $ hg bisect -c false -s
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --command and --skip are incompatible
   [255]
   $ hg bisect -e -g
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --extend and --good are incompatible
   [255]
   $ hg bisect -e -r
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --extend and --reset are incompatible
   [255]
   $ hg bisect -e -s
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --extend and --skip are incompatible
   [255]
   $ hg bisect -g -r
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --good and --reset are incompatible
   [255]
   $ hg bisect -g -s
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --good and --skip are incompatible
   [255]
   $ hg bisect -r -s
-  abort: -g|--good, -b|--bad, -s|--skip, -r|--reset, -e|--extend, and -c|--command are exclusive
+  abort: --reset and --skip are incompatible
   [255]

Regards,


-- 
Brandon McCaig <bamccaig at gmail.com> <bambams at castopulence.org>
Castopulence Software <https://www.castopulence.org/>
Blog <http://www.bambams.ca/>
perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20170610/fa4c1512/attachment.sig>


More information about the Mercurial-devel mailing list