From c35fa6975bc165f595c6370e801f8c656d931329 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 28 Jan 2021 13:50:38 -0400 Subject: [PATCH] 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. --- CHANGELOG | 4 + Utility/Matcher.hs | 87 ++++++++++++++----- ...licit_and__47__or_gives_wrong_matches.mdwn | 2 + ..._8160d9308dc19b3c6a0596f437134e28._comment | 20 +++++ 4 files changed, 89 insertions(+), 24 deletions(-) create mode 100644 doc/bugs/parens_without_explicit_and__47__or_gives_wrong_matches/comment_1_8160d9308dc19b3c6a0596f437134e28._comment diff --git a/CHANGELOG b/CHANGELOG index a325d90671..80d8444244 100644 --- 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 Thu, 28 Jan 2021 12:34:32 -0400 diff --git a/Utility/Matcher.hs b/Utility/Matcher.hs index 00f3ca3415..1772c230b3 100644 --- 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 + - Copyright 2011-2021 Joey Hess - - 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 03a4e3b473..5faae9fcac 100644 --- 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 index 0000000000..ece62140b6 --- /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. +"""]]