From b1700f3cf6c837c7e14872d1ace1823dcd7e974f Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Wed, 27 Jan 2010 16:52:54 -0500 Subject: [PATCH] Enhancement - caseless compare - Fixed bug detecting multiple failed negative attribute matches. --- src/Att.cpp | 26 ++++++++++++++------------ src/Filter.cpp | 19 +++++++++++++------ src/tests/bug.hasnt.t | 1 + 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/Att.cpp b/src/Att.cpp index 466af6e1a..c01e50b98 100644 --- a/src/Att.cpp +++ b/src/Att.cpp @@ -104,7 +104,7 @@ Att::Att (const std::string& name, const std::string& mod, const std::string& va //////////////////////////////////////////////////////////////////////////////// Att::Att (const std::string& name, const std::string& mod, int value) { - mName = name; + mName = name; std::stringstream s; s << value; @@ -124,7 +124,7 @@ Att::Att (const std::string& name, const std::string& value) //////////////////////////////////////////////////////////////////////////////// Att::Att (const std::string& name, int value) { - mName = name; + mName = name; std::stringstream s; s << value; @@ -415,6 +415,7 @@ std::string Att::modType (const std::string& name) const { if (name == "hasnt" || name == "isnt" || + name == "not" || // TODO Verify this. name == "noword") return "negative"; @@ -486,32 +487,33 @@ void Att::parse (Nibbler& n) bool Att::match (const Att& other) const { // All matches are assumed to pass, any short-circuit on non-match. + bool case_sensitive = context.config.getBoolean ("search.case.sensitive"); // If there are no mods, just perform a straight compare on value. if (mMod == "") { - if (mValue != other.mValue) + if (!compare (mValue, other.mValue, (bool) case_sensitive)) return false; } // has = contains as a substring. else if (mMod == "has" || mMod == "contains") // TODO i18n { - if (other.mValue.find (mValue) == std::string::npos) + if (find (other.mValue, mValue, (bool) case_sensitive) == std::string::npos) return false; } // is = equal. Nop. else if (mMod == "is" || mMod == "equals") // TODO i18n { - if (mValue != other.mValue) + if (!compare (mValue, other.mValue, (bool) case_sensitive)) return false; } // isnt = not equal. else if (mMod == "isnt" || mMod == "not") // TODO i18n { - if (mValue == other.mValue) + if (compare (mValue, other.mValue, (bool) case_sensitive)) return false; } @@ -535,7 +537,7 @@ bool Att::match (const Att& other) const if (other.mValue.length () < mValue.length ()) return false; - if (mValue != other.mValue.substr (0, mValue.length ())) + if (!compare (mValue, other.mValue.substr (0, mValue.length ()))) return false; } @@ -545,16 +547,16 @@ bool Att::match (const Att& other) const if (other.mValue.length () < mValue.length ()) return false; - if (mValue != other.mValue.substr ( + if (!compare (mValue, other.mValue.substr ( other.mValue.length () - mValue.length (), - std::string::npos)) + std::string::npos), (bool) case_sensitive)) return false; } // hasnt = does not contain as a substring. else if (mMod == "hasnt") // TODO i18n { - if (other.mValue.find (mValue) != std::string::npos) + if (find (other.mValue, mValue, (bool) case_sensitive) != std::string::npos) return false; } @@ -622,7 +624,7 @@ bool Att::match (const Att& other) const else if (mMod == "word") // TODO i18n { // Fail if the substring is not found. - std::string::size_type sub = other.mValue.find (mValue); + std::string::size_type sub = find (other.mValue, mValue, (bool) case_sensitive); if (sub == std::string::npos) return false; @@ -638,7 +640,7 @@ bool Att::match (const Att& other) const else if (mMod == "noword") // TODO i18n { // Fail if the substring is not found. - std::string::size_type sub = other.mValue.find (mValue); + std::string::size_type sub = find (other.mValue, mValue); if (sub != std::string::npos && isWordStart (other.mValue, sub) && isWordEnd (other.mValue, sub + mValue.length () - 1)) diff --git a/src/Filter.cpp b/src/Filter.cpp index 27087afe8..1ed104852 100644 --- a/src/Filter.cpp +++ b/src/Filter.cpp @@ -42,17 +42,22 @@ bool Filter::pass (const Record& record) const { // Descriptions have special handling such that they are linked to // annotations, and filtering on description implies identical filtering - // on annotations, and that both filter matches must succeed for the filter - // to succeed overall. + // on annotations. + // + // For positive modifiers (has, is ...) either the description or the + // annotation filter must succeed. + // + // For negative modifiers (hasnt, isnt ...) both the description and the + // annotation filter must succeed. if (att->name () == "description") { - bool description_result = true; + bool pass = true; int annotation_pass_count = 0; int annotation_fail_count = 0; if ((r = record.find (att->name ())) != record.end ()) { - description_result = att->match (r->second); + pass = att->match (r->second); foreach (ra, record) { @@ -66,6 +71,8 @@ bool Filter::pass (const Record& record) const } } } + + // Missing attribute implies failure. else if (! att->match (Att ())) return false; @@ -74,12 +81,12 @@ bool Filter::pass (const Record& record) const // are willing to invest a week understanding and testing it. if (att->modType (att->mod ()) == "positive") { - if (! (description_result || annotation_pass_count > 0)) + if (! (pass || annotation_pass_count)) return false; } else { - if (!description_result || annotation_fail_count > 0) + if (! (pass && annotation_fail_count == 0)) return false; } } diff --git a/src/tests/bug.hasnt.t b/src/tests/bug.hasnt.t index 6cfcd8d6e..2a8b3da78 100755 --- a/src/tests/bug.hasnt.t +++ b/src/tests/bug.hasnt.t @@ -40,6 +40,7 @@ if (open my $fh, '>', 'hasnt.rc') } # 1 +diag ("Initialization taskes 6 seconds"); qx{../task rc:hasnt.rc add foo}; # 2