reapack

commit a911c85267d8ce0fc59cf7a2b429ba444fe7caa4
parent 27451e8a08ef552771d99208fe909b75fa9b5c97
Author: cfillion <cfillion@users.noreply.github.com>
Date:   Tue, 20 Oct 2020 14:35:54 -0400

filter: fix tokens following an AND subgroup in a OR group being added to the OR group

"a OR ( b ) c" was being parsed as "a OR ( b ) OR c"

Diffstat:
M
src/filter.cpp
 | 
25
++++++++++++-------------
M
src/filter.hpp
 | 
1
-
M
test/filter.cpp
 | 
16
++++++++++++++++

3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/src/filter.cpp b/src/filter.cpp
@@ -91,7 +91,7 @@ bool Filter::match(std::vector<std::string> rows) const
 }
 
 Filter::Group::Group(Type type, int flags, Group *parent)
-  : Node(flags), m_parent(parent), m_type(type), m_open(true)
+  : Node(flags), m_parent(parent), m_type(type)
 {
 }
 
@@ -107,11 +107,11 @@ Filter::Group *Filter::Group::push(const std::string &buf, int *flags)
     }
     else if(buf == "OR") {
       if(m_nodes.empty())
-        return this;
-      else if(m_type == MatchAny) {
-        m_open = true;
-        return this;
-      }
+        return this; // no previous token, ignore
+
+      Group *currentOr = dynamic_cast<Group *>(m_nodes.back().get());
+      if(currentOr && currentOr->m_type == MatchAny)
+        return currentOr;
 
       auto prev = std::move(m_nodes.back());
       m_nodes.pop_back();
@@ -126,22 +126,21 @@ Filter::Group *Filter::Group::push(const std::string &buf, int *flags)
       return newGroup;
     }
     else if(buf == ")") {
-      for(Group *parent = this; parent->m_parent; parent = parent->m_parent) {
+      for(Group *parent = m_parent; parent; parent = parent->m_parent) {
         if(parent->m_type == MatchAll)
-          return parent->m_parent;
+          return parent;
       }
 
       return this;
     }
   }
 
-  Group *group = m_open ? this : m_parent;
-  group->m_nodes.push_back(std::make_unique<Token>(buf, *flags));
+  m_nodes.push_back(std::make_unique<Token>(buf, *flags));
   *flags = 0;
 
-  if(group->m_type == MatchAny)
-    group->m_open = false;
-
+  Group *group = this;
+  while(group->m_type != MatchAll && group->m_parent)
+    group = group->m_parent;
   return group;
 }
 
diff --git a/src/filter.hpp b/src/filter.hpp
@@ -74,7 +74,6 @@ private:
 
     Group *m_parent;
     Type m_type;
-    bool m_open;
     std::vector<std::unique_ptr<Node>> m_nodes;
   };
 
diff --git a/test/filter.cpp b/test/filter.cpp
@@ -346,4 +346,20 @@ TEST_CASE("AND grouping", M) {
     REQUIRE(f.match({"("}));
     REQUIRE_FALSE(f.match({"foo"}));
   }
+
+  SECTION("closing a subgroup rewinds to the AND parent") {
+    f.set("a OR ( b ) c");
+
+    REQUIRE_FALSE(f.match({"c"}));
+    REQUIRE(f.match({"a c"}));
+    REQUIRE(f.match({"b c"}));
+  }
+
+  SECTION("closing a subgroup doesn't rewind too far") {
+    f.set("NOT ( a OR ( b ) c ) OR d");
+
+    REQUIRE_FALSE(f.match({"a c"}));
+    REQUIRE(f.match({"a z"}));
+    REQUIRE(f.match({"d"}));
+  }
 }