fix handling of implicit and before parens

Fix an oddity in matching options and preferred content expressions such as
"foo (bar or baz)", which was incorrectly handled as if it were "(foo or
bar) and baz)" rather than the intended "foo and (bar or baz)"

Seemed like a change to consume should be able to handle this case
better, but I was having trouble writing it that way, so instead added
a separate pass that inserts the implicit ands explicitly. Also added
several test cases to make sure versions with and without explicit ands
generate the same.

diff --git a/CHANGELOG b/CHANGELOG
index a325d9067134aaa47c09b38f99f5b5a16e163645..80d84442447b257773096f60050acf14efbeb2e5 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -5,6 +5,10 @@ git-annex (8.20210128) UNRELEASED; urgency=medium
     of that special remote.
   * When adding files to an adjusted branch set up by --unlock-present,
     add them unlocked, not locked.
+  * Fix an oddity in matching options and preferred content expressions
+    such as "foo (bar or baz)", which was incorrectly handled as if
+    it were "(foo or bar) and baz)" rather than the intended
+    "foo and (bar or baz)"
 
  -- Joey Hess <id@joeyh.name>  Thu, 28 Jan 2021 12:34:32 -0400
 

diff --git a/Utility/Matcher.hs b/Utility/Matcher.hs
index 00f3ca3415f70d7a63958374244589443c4b39ad..1772c230b3ba47b6ff60c023c1ba638e1c2fd760 100644 (file)
--- a/Utility/Matcher.hs
+++ b/Utility/Matcher.hs
@@ -10,7 +10,7 @@
  - Is forgiving about misplaced closing parens, so "foo and (bar or baz"
  - will be handled, as will "foo and ( bar or baz ) )"
  -
- - Copyright 2011-2020 Joey Hess <id@joeyh.name>
+ - Copyright 2011-2021 Joey Hess <id@joeyh.name>
  -
  - License: BSD-2-clause
  -}
@@ -57,7 +57,7 @@ syntaxToken t = Left $ "unknown token " ++ t
 
 {- Converts a list of Tokens into a Matcher. -}
 generate :: [Token op] -> Matcher op
-generate = simplify . process MAny . tokenGroups
+generate = simplify . process MAny . implicitAnd . tokenGroups
   where
        process m [] = m
        process m ts = uncurry process $ consume m ts
@@ -108,6 +108,16 @@ findClose l =
                        in go (Group (reverse c') : c) ts'
                dispatch _ = go (One t:c) ts
 
+implicitAnd :: [TokenGroup op] -> [TokenGroup op]
+implicitAnd [] = []
+implicitAnd [v] = [v]
+implicitAnd (a:b:rest) | need a && need b = a : One And : implicitAnd (b:rest)
+  where
+       need (One (Operation _)) = True
+       need (Group _) = True
+       need _ = False
+implicitAnd (a:rest) = a : implicitAnd rest
+
 {- Checks if a Matcher matches, using a supplied function to check
  - the value of Operations. -}
 match :: (op -> v -> Bool) -> Matcher op -> v -> Bool
@@ -153,27 +163,56 @@ introspect :: (a -> Bool) -> Matcher a -> Bool
 introspect = any
 
 prop_matcher_sane :: Bool
-prop_matcher_sane = all (\m -> match dummy m ()) $ map generate
-       [ [Operation True]
-       , []
-       , [Operation False, Or, Operation True, Or, Operation False]
-       , [Operation True, Or, Operation True]
-       , [Operation True, And, Operation True]
-       , [Not, Open, Operation True, And, Operation False, Close]
-       , [Not, Open, Not, Open, Not, Operation False, Close, Close]
-       , [Not, Open, Not, Open, Not, Open, Not, Operation True, Close, Close]
-       , [Operation True, And, Not, Operation False]
-       , [Operation True, Not, Operation False]
-       , [Operation True, Not, Not, Not, Operation False]
-       , [Operation True, Not, Not, Not, Operation False, And, Operation True]
-       , [Operation True, Not, Not, Not, Operation False, Operation True]
-       , [Not, Open, Operation True, And, Operation False, Close, 
-               And, Open, 
-                       Open, Operation True, And, Operation False, Close,
-                       Or,
-                       Open, Operation True, And, Open, Not, Operation False, Close, Close,
-               Close, And,
-                       Open, Not, Operation False, Close]
+prop_matcher_sane = and
+       [ all (\m -> match (\b _ -> b) m ()) (map generate evaltrue)
+       , all (\(x,y) -> generate x == generate y) evalsame
        ]
   where
-       dummy b _ = b
+       evaltrue =
+               [ [Operation True]
+               , []
+               , [Operation False, Or, Operation True, Or, Operation False]
+               , [Operation True, Or, Operation True]
+               , [Operation True, And, Operation True]
+               , [Not, Open, Operation True, And, Operation False, Close]
+               , [Not, Open, Not, Open, Not, Operation False, Close, Close]
+               , [Not, Open, Not, Open, Not, Open, Not, Operation True, Close, Close]
+               , [Operation True, And, Not, Operation False]
+               , [Operation True, Not, Operation False]
+               , [Operation True, Not, Not, Not, Operation False]
+               , [Operation True, Not, Not, Not, Operation False, And, Operation True]
+               , [Operation True, Not, Not, Not, Operation False, Operation True]
+               , [Not, Open, Operation True, And, Operation False, Close, 
+                       And, Open, 
+                               Open, Operation True, And, Operation False, Close,
+                               Or,
+                               Open, Operation True, And, Open, Not, Operation False, Close, Close,
+                       Close, And,
+                               Open, Not, Operation False, Close]
+               ]
+       evalsame =
+               [
+                       ( [Operation "foo", Open, Operation "bar", Or, Operation "baz", Close]
+                       , [Operation "foo", And, Open, Operation "bar", Or, Operation "baz", Close]
+                       )
+               ,
+                       ( [Operation "foo", Not, Open, Operation "bar", Or, Operation "baz", Close]
+                       , [Operation "foo", And, Not, Open, Operation "bar", Or, Operation "baz", Close]
+                       )
+               ,
+                       ( [Open, Operation "bar", Or, Operation "baz", Close, Operation "foo"]
+                       , [Open, Operation "bar", Or, Operation "baz", Close, And, Operation "foo"]
+                       )
+               ,
+                       ( [Open, Operation "bar", Or, Operation "baz", Close, Not, Operation "foo"]
+                       , [Open, Operation "bar", Or, Operation "baz", Close, And, Not, Operation "foo"]
+                       )
+               ,
+                       ( [Operation "foo", Operation "bar"]
+                       , [Operation "foo", And, Operation "bar"]
+                       )
+               ,
+                       ( [Operation "foo", Not, Operation "bar"]
+                       , [Operation "foo", And, Not, Operation "bar"]
+                       )
+               ]

diff --git a/doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches.mdwn b/doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches.mdwn
index 03a4e3b4732e715bffba9f7e63b9e99d49dc4b2f..5faae9fcac3414247544e95ce30fbd0f6199eb5b 100644 (file)
--- a/doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches.mdwn
+++ b/doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches.mdwn
@@ -82,3 +82,5 @@ b.2
 ### Have you had any luck using git-annex before? (Sometimes we get tired of reading bug reports all day and a lil' positive end note does wonders)
 
 Yes! [My description from before](https://git-annex.branchable.com/bugs/async_external_special_remote__39__s_stdin_not_closed/) still applies. And I've continued growing my annex and expanded onto a new drive since then.
+
+> [[fixed|done]] --[[Joey]] 

diff --git a/doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches/comment_1_8160d9308dc19b3c6a0596f437134e28._comment b/doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches/comment_1_8160d9308dc19b3c6a0596f437134e28._comment
new file mode 100644 (file)
index 0000000..ece6214
--- /dev/null
+++ b/doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches/comment_1_8160d9308dc19b3c6a0596f437134e28._comment
@@ -0,0 +1,20 @@
+[[!comment format=mdwn
+ username="joey"
+ subject="""comment 1"""
+ date="2021-01-28T16:54:29Z"
+ content="""
+Let's see, here's the equivilant after parsing:
+
+       # ghci Utility/Matcher.hs
+       -- foo and (bar or baz)
+       ghci> generate [Operation "foo", And, Open, Operation "bar", Or, Operation "baz", Close]
+       MAnd (MOp "foo") (MOr (MOp "bar") (MOp "baz"))
+       -- foo (bar or baz)
+       ghci> generate [Operation "foo", Open, Operation "bar", Or, Operation "baz", Close]
+       MOr (MAnd (MOp "foo") (MOp "bar")) (MOp "baz")
+
+So it's interpreting "foo (bar or baz) like "(foo and bar) or baz"
+which is surely a bug.
+
+Fixed.
+"""]]