]> cloud.milkyroute.net Git - dolphin.git/commitdiff
fix(search): Correctly parse filename and/or content search
authorIsmael Asensio <isma.af@gmail.com>
Sat, 21 Dec 2019 18:14:17 +0000 (19:14 +0100)
committerIsmael Asensio <isma.af@gmail.com>
Sat, 21 Dec 2019 18:17:18 +0000 (19:17 +0100)
Summary:
Currently, the search url parsing does not detect if the search is based on Content or Filename, and it just keeps the last selection which can be inconsistent with the actual search.

This patch add such detection, and since an advanced user can combine filename and content search (using the keyword `filename:`), now the parsing detects both items and handles the four possible cases:

| Content | Filename | Search text | Search type |
|---|---|------------------------|------------------|
| T | T | abc filename:"xyz"  | Content          |
| T | F | abc                           | Content          |
| F | T | xyz                           | Filename         |
| F | F |                                  | do not set       |

Depends on: D25260

Test Plan: `bin/dolphinquerytest`: Added new test cases for searches with content text and/or filename

Reviewers: elvisangelaccio, bruns, #dolphin

Reviewed By: elvisangelaccio, #dolphin

Subscribers: kfm-devel

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D25416

src/search/dolphinquery.cpp
src/search/dolphinquery.h
src/search/dolphinsearchbox.cpp
src/tests/dolphinquerytest.cpp

index 92694c09387b332b5559903df644d5e6c4618880..4d5f8e132099810e19704c41cea636babcf00968 100644 (file)
@@ -19,6 +19,8 @@
 
 #include "dolphinquery.h"
 
+#include <QRegularExpression>
+
 #include <config-baloo.h>
 #ifdef HAVE_BALOO
 #include <Baloo/Query>
@@ -43,6 +45,30 @@ namespace {
         }
         return false;
     }
+
+    QString stripQuotes(const QString& text)
+    {
+        QString cleanedText = text;
+        if (!cleanedText.isEmpty() && cleanedText.at(0) == QLatin1Char('"')) {
+            cleanedText = cleanedText.mid(1);
+        }
+        if (!cleanedText.isEmpty() && cleanedText.back() == QLatin1Char('"')) {
+            cleanedText = cleanedText.mid(0, cleanedText.size() - 1);
+        }
+        return cleanedText;
+    }
+
+    QStringList splitOutsideQuotes(const QString& text)
+    {
+        const QRegularExpression subTermsRegExp("([^ ]*\"[^\"]*\"|(?<= |^)[^ ]+(?= |$))");
+        auto subTermsMatchIterator = subTermsRegExp.globalMatch(text);
+
+        QStringList textParts;
+        while (subTermsMatchIterator.hasNext()) {
+            textParts << subTermsMatchIterator.next().captured(0);
+        }
+        return textParts;
+    }
 }
 
 DolphinQuery DolphinQuery::fromBalooSearchUrl(const QUrl& searchUrl)
@@ -59,29 +85,35 @@ DolphinQuery DolphinQuery::fromBalooSearchUrl(const QUrl& searchUrl)
     model.m_fileType = types.isEmpty() ? QString() : types.first();
 
     QStringList textParts;
+    QString fileName;
 
-    const QStringList subTerms = query.searchString().split(' ', QString::SkipEmptyParts);
+    const QStringList subTerms = splitOutsideQuotes(query.searchString());
     foreach (const QString& subTerm, subTerms) {
-        QString value;
         if (subTerm.startsWith(QLatin1String("filename:"))) {
-            value = subTerm.mid(9);
+            fileName = stripQuotes(subTerm.mid(9));
+            if (!fileName.isEmpty()) {
+                model.m_hasFileName = true;
+            }
+            continue;
         } else if (isSearchTerm(subTerm)) {
             model.m_searchTerms << subTerm;
             continue;
         } else if (subTerm == QLatin1String("AND") && subTerm != subTerms.at(0) && subTerm != subTerms.back()) {
             continue;
         } else {
-            value = subTerm;
+            const QString cleanedTerm = stripQuotes(subTerm);
+            if (!cleanedTerm.isEmpty()) {
+                textParts << cleanedTerm;
+                model.m_hasContentSearch = true;
+            }
         }
+    }
 
-        if (!value.isEmpty() && value.at(0) == QLatin1Char('"')) {
-            value = value.mid(1);
-        }
-        if (!value.isEmpty() && value.back() == QLatin1Char('"')) {
-            value = value.mid(0, value.size() - 1);
-        }
-        if (!value.isEmpty()) {
-            textParts << value;
+    if (model.m_hasFileName) {
+        if (model.m_hasContentSearch) {
+            textParts << QStringLiteral("filename:\"%1\"").arg(fileName);
+        } else {
+            textParts << fileName;
         }
     }
 
@@ -115,3 +147,13 @@ QString DolphinQuery::includeFolder() const
 {
     return m_includeFolder;
 }
+
+bool DolphinQuery::hasContentSearch() const
+{
+    return m_hasContentSearch;
+}
+
+bool DolphinQuery::hasFileName() const
+{
+    return m_hasFileName;
+}
index e60008e3bf8ca7d23f3fd42b171afe59318d70fe..544f246bcfe6bea0b184c982fead5df359d011c6 100644 (file)
@@ -48,6 +48,10 @@ public:
     /** @return Baloo::Query::includeFolder(), that is, the initial directory
      * for the query or an empty string if its a global search" */
     QString includeFolder() const;
+    /** @return whether the query includes search in file content */
+    bool hasContentSearch() const;
+    /** @return whether the query includes a filter by fileName */
+    bool hasFileName() const;
 
 private:
     QUrl m_searchUrl;
@@ -55,6 +59,8 @@ private:
     QString m_fileType;
     QStringList m_searchTerms;
     QString m_includeFolder;
+    bool m_hasContentSearch = false;
+    bool m_hasFileName = false;
 };
 
 #endif //DOLPHINQUERY_H
index 16f17bbcdd323c6e4b2dd3c153e2618fc0368dfc..302081d7d7450364853c19f7b90251a83d99afca 100644 (file)
@@ -517,6 +517,12 @@ void DolphinSearchBox::updateFromQuery(const DolphinQuery& query)
 
     setText(query.text());
 
+    if (query.hasContentSearch()) {
+        m_contentButton->setChecked(true);
+    } else if (query.hasFileName())  {
+        m_fileNameButton->setChecked(true);
+    }
+
     m_facetsWidget->resetOptions();
     m_facetsWidget->setFacetType(query.type());
     const QStringList searchTerms = query.searchTerms();
index e3c6fb8e35913e939ff88632f0465e602cd57a70..65dc7350e9f98d07e5711953cfb0df76797816d6 100644 (file)
@@ -41,8 +41,8 @@ private slots:
  */
 void DolphinSearchBoxTest::testBalooSearchParsing_data()
 {
-    const QString text = QStringLiteral("xyz");
-    const QString filename = QStringLiteral("filename:\"xyz\"");
+    const QString text = QStringLiteral("abc xyz");
+    const QString filename = QStringLiteral("filename:\"%1\"").arg(text);
     const QString rating = QStringLiteral("rating>=2");
     const QString modified = QString("modified>=2019-08-07");
     const QString tagA = QString("tag:tagA");
@@ -51,51 +51,56 @@ void DolphinSearchBoxTest::testBalooSearchParsing_data()
     QTest::addColumn<QString>("searchString");
     QTest::addColumn<QString>("expectedText");
     QTest::addColumn<QStringList>("expectedTerms");
+    QTest::addColumn<bool>("hasContent");
+    QTest::addColumn<bool>("hasFileName");
 
     // Test for "Content"
-    QTest::newRow("content")              << text    << text << QStringList();
-    QTest::newRow("content/empty")        << ""      << ""   << QStringList();
-    QTest::newRow("content/singleQuote")  << "\""    << ""   << QStringList();
-    QTest::newRow("content/doubleQuote")  << "\"\""  << ""   << QStringList();
+    QTest::newRow("content")              << text    << text << QStringList() << true  << false;
+    QTest::newRow("content/empty")        << ""      << ""   << QStringList() << false << false;
+    QTest::newRow("content/singleQuote")  << "\""    << ""   << QStringList() << false << false;
+    QTest::newRow("content/doubleQuote")  << "\"\""  << ""   << QStringList() << false << false;
 
-    // Test for "Filename"
-    QTest::newRow("filename")             << filename         << text << QStringList();
-    QTest::newRow("filename/empty")       << "filename:"      << ""   << QStringList();
-    QTest::newRow("filename/singleQuote") << "filename:\""    << ""   << QStringList();
-    QTest::newRow("filename/doubleQuote") << "filename:\"\""  << ""   << QStringList();
+    // Test for "FileName"
+    QTest::newRow("filename")             << filename         << text << QStringList() << false << true;
+    QTest::newRow("filename/empty")       << "filename:"      << ""   << QStringList() << false << false;
+    QTest::newRow("filename/singleQuote") << "filename:\""    << ""   << QStringList() << false << false;
+    QTest::newRow("filename/doubleQuote") << "filename:\"\""  << ""   << QStringList() << false << false;
+
+    // Combined content and filename search
+    QTest::newRow("content+filename") << text + " " + filename << text + " " + filename << QStringList() << true << true;
 
     // Test for rating
-    QTest::newRow("rating")          << rating                  << ""   << QStringList({rating});
-    QTest::newRow("rating+content")  << rating + " " + text     << text << QStringList({rating});
-    QTest::newRow("rating+filename") << rating + " " + filename << text << QStringList({rating});
+    QTest::newRow("rating")          << rating                  << ""   << QStringList({rating}) << false << false;
+    QTest::newRow("rating+content")  << rating + " " + text     << text << QStringList({rating}) << true  << false;
+    QTest::newRow("rating+filename") << rating + " " + filename << text << QStringList({rating}) << false << true;
 
     // Test for modified date
-    QTest::newRow("modified")          << modified                  << ""   << QStringList({modified});
-    QTest::newRow("modified+content")  << modified + " " + text     << text << QStringList({modified});
-    QTest::newRow("modified+filename") << modified + " " + filename << text << QStringList({modified});
+    QTest::newRow("modified")          << modified                  << ""   << QStringList({modified}) << false << false;
+    QTest::newRow("modified+content")  << modified + " " + text     << text << QStringList({modified}) << true  << false;
+    QTest::newRow("modified+filename") << modified + " " + filename << text << QStringList({modified}) << false << true;
 
     // Test for tags
-    QTest::newRow("tag")          << tagA                  << ""   << QStringList({tagA});
-    QTest::newRow("tag/double")   << tagA + " " + tagB     << ""   << QStringList({tagA, tagB});
-    QTest::newRow("tag+content")  << tagA + " " + text     << text << QStringList({tagA});
-    QTest::newRow("tag+filename") << tagA + " " + filename << text << QStringList({tagA});
-
-    // Combined tests
-    QTest::newRow("rating+modified")
-        << rating + " AND " + modified
-        << "" << QStringList({modified, rating});
+    QTest::newRow("tag")          << tagA                  << ""   << QStringList({tagA})       << false << false;
+    QTest::newRow("tag/double")   << tagA + " " + tagB     << ""   << QStringList({tagA, tagB}) << false << false;
+    QTest::newRow("tag+content")  << tagA + " " + text     << text << QStringList({tagA})       << true  << false;
+    QTest::newRow("tag+filename") << tagA + " " + filename << text << QStringList({tagA})       << false << true;
 
+    // Combined search terms
     QTest::newRow("allTerms")
         << rating + " AND " + modified + " AND " + tagA + " AND " + tagB
-        << "" << QStringList({modified, rating, tagA, tagB});
+        << "" << QStringList({modified, rating, tagA, tagB}) << false << false;
 
     QTest::newRow("allTerms+content")
         << rating + " AND " + modified + " " + text + " " + tagA + " AND " + tagB
-        << text << QStringList({modified, rating, tagA, tagB});
-    
+        << text << QStringList({modified, rating, tagA, tagB}) << true << false;
+
     QTest::newRow("allTerms+filename")
         << rating + " AND " + modified + " " + filename + " " + tagA + " AND " + tagB
-        << text << QStringList({modified, rating, tagA, tagB});
+        << text << QStringList({modified, rating, tagA, tagB}) << false << true;
+
+    QTest::newRow("allTerms+content+filename")
+        << text + " " + filename + " " + rating + " AND " + modified + " AND " + tagA + " AND " + tagB
+        << text + " " + filename << QStringList({modified, rating, tagA, tagB}) << true << true;
 }
 
 /**
@@ -130,6 +135,8 @@ void DolphinSearchBoxTest::testBalooSearchParsing()
     QFETCH(QString, searchString);
     QFETCH(QString, expectedText);
     QFETCH(QStringList, expectedTerms);
+    QFETCH(bool, hasContent);
+    QFETCH(bool, hasFileName);
 
     const QUrl testUrl = composeQueryUrl(searchString);
     const DolphinQuery query = DolphinQuery::fromBalooSearchUrl(testUrl);
@@ -145,6 +152,10 @@ void DolphinSearchBoxTest::testBalooSearchParsing()
     for (int i = 0; i < expectedTerms.count(); i++) {
         QCOMPARE(searchTerms.at(i), expectedTerms.at(i));
     }
+
+    // Check for filename and content detection
+    QCOMPARE(query.hasContentSearch(), hasContent);
+    QCOMPARE(query.hasFileName(), hasFileName);
 }
 
 QTEST_MAIN(DolphinSearchBoxTest)