Merge pull request #4536 from ronso0/lib-selection-tweaks

Library track selection tweaks

18 files changed, 260 insertions, 29 deletions

diff --git a/src/library/autodj/dlgautodj.cpp b/src/library/autodj/dlgautodj.cpp
index 87fcde8e82..adfe89852a 100644
--- a/src/library/autodj/dlgautodj.cpp
+++ b/src/library/autodj/dlgautodj.cpp
@@ -390,6 +390,6 @@ void DlgAutoDJ::saveCurrentViewState() {
     m_pTrackTableView->saveCurrentViewState();
 }
 
-void DlgAutoDJ::restoreCurrentViewState() {
-    m_pTrackTableView->restoreCurrentViewState();
+bool DlgAutoDJ::restoreCurrentViewState() {
+    return m_pTrackTableView->restoreCurrentViewState();
 }
diff --git a/src/library/autodj/dlgautodj.h b/src/library/autodj/dlgautodj.h
index c109a56363..d0976f2d3c 100644
--- a/src/library/autodj/dlgautodj.h
+++ b/src/library/autodj/dlgautodj.h
@@ -35,7 +35,7 @@ class DlgAutoDJ : public QWidget, public Ui::DlgAutoDJ, public LibraryView {
     void loadSelectedTrackToGroup(const QString& group, bool play) override;
     void moveSelection(int delta) override;
     void saveCurrentViewState() override;
-    void restoreCurrentViewState() override;
+    bool restoreCurrentViewState() override;
 
   public slots:
     void shufflePlaylistButton(bool buttonChecked);
diff --git a/src/library/dlganalysis.cpp b/src/library/dlganalysis.cpp
index ea804bb56b..0fc2adb4c4 100644
--- a/src/library/dlganalysis.cpp
+++ b/src/library/dlganalysis.cpp
@@ -234,6 +234,6 @@ void DlgAnalysis::saveCurrentViewState() {
     m_pAnalysisLibraryTableView->saveCurrentViewState();
 }
 
-void DlgAnalysis::restoreCurrentViewState() {
-    m_pAnalysisLibraryTableView->restoreCurrentViewState();
+bool DlgAnalysis::restoreCurrentViewState() {
+    return m_pAnalysisLibraryTableView->restoreCurrentViewState();
 }
diff --git a/src/library/dlganalysis.h b/src/library/dlganalysis.h
index e9955eae70..0ce5e4f591 100644
--- a/src/library/dlganalysis.h
+++ b/src/library/dlganalysis.h
@@ -36,7 +36,7 @@ class DlgAnalysis : public QWidget, public Ui::DlgAnalysis, public virtual Libra
         return m_pAnalysisLibraryTableModel->currentSearch();
     }
     void saveCurrentViewState() override;
-    void restoreCurrentViewState() override;
+    bool restoreCurrentViewState() override;
 
   public slots:
     void tableSelectionChanged(const QItemSelection& selected,
diff --git a/src/library/dlghidden.cpp b/src/library/dlghidden.cpp
index bab2bf7928..b65564093c 100644
--- a/src/library/dlghidden.cpp
+++ b/src/library/dlghidden.cpp
@@ -139,8 +139,8 @@ void DlgHidden::saveCurrentViewState() {
     m_pTrackTableView->saveCurrentViewState();
 }
 
-void DlgHidden::restoreCurrentViewState() {
-    m_pTrackTableView->restoreCurrentViewState();
+bool DlgHidden::restoreCurrentViewState() {
+    return m_pTrackTableView->restoreCurrentViewState();
 }
 
 void DlgHidden::setFocus() {
diff --git a/src/library/dlghidden.h b/src/library/dlghidden.h
index 2a459c7736..5db49bcd69 100644
--- a/src/library/dlghidden.h
+++ b/src/library/dlghidden.h
@@ -27,7 +27,7 @@ class DlgHidden : public QWidget, public Ui::DlgHidden, public LibraryView {
     void onSearch(const QString& text) override;
     QString currentSearch();
     void saveCurrentViewState() override;
-    void restoreCurrentViewState() override;
+    bool restoreCurrentViewState() override;
 
   public slots:
     void clicked();
diff --git a/src/library/dlgmissing.cpp b/src/library/dlgmissing.cpp
index 7c50bc8deb..9997accb29 100644
--- a/src/library/dlgmissing.cpp
+++ b/src/library/dlgmissing.cpp
@@ -96,8 +96,9 @@ bool DlgMissing::hasFocus() const {
 void DlgMissing::saveCurrentViewState() {
     m_pTrackTableView->saveCurrentViewState();
 };
-void DlgMissing::restoreCurrentViewState() {
-    m_pTrackTableView->restoreCurrentViewState();
+
+bool DlgMissing::restoreCurrentViewState() {
+    return m_pTrackTableView->restoreCurrentViewState();
 };
 
 void DlgMissing::setFocus() {
diff --git a/src/library/dlgmissing.h b/src/library/dlgmissing.h
index ac992ecf3f..3712680d74 100644
--- a/src/library/dlgmissing.h
+++ b/src/library/dlgmissing.h
@@ -27,7 +27,7 @@ class DlgMissing : public QWidget, public Ui::DlgMissing, public LibraryView {
     void onSearch(const QString& text) override;
     QString currentSearch();
     void saveCurrentViewState() override;
-    void restoreCurrentViewState() override;
+    bool restoreCurrentViewState() override;
 
   public slots:
     void clicked();
diff --git a/src/library/libraryview.h b/src/library/libraryview.h
index 74c9fa7d3c..9dc79cb323 100644
--- a/src/library/libraryview.h
+++ b/src/library/libraryview.h
@@ -35,8 +35,11 @@ class LibraryView {
     }
     virtual void saveCurrentViewState() {
     }
-    virtual void restoreCurrentViewState() {
-    }
+    /// @brief restores current view state.
+    /// @return true if restore succeeded
+    virtual bool restoreCurrentViewState() {
+        return false;
+    };
 
     /// If applicable, requests that the LibraryView load the selected track to
     /// the specified group. Does nothing otherwise.
diff --git a/src/library/recording/dlgrecording.cpp b/src/library/recording/dlgrecording.cpp
index 9ff66c54c2..06e452efe3 100644
--- a/src/library/recording/dlgrecording.cpp
+++ b/src/library/recording/dlgrecording.cpp
@@ -205,6 +205,6 @@ void DlgRecording::saveCurrentViewState() {
     m_pTrackTableView->saveCurrentViewState();
 }
 
-void DlgRecording::restoreCurrentViewState() {
-    m_pTrackTableView->restoreCurrentViewState();
+bool DlgRecording::restoreCurrentViewState() {
+    return m_pTrackTableView->restoreCurrentViewState();
 }
diff --git a/src/library/recording/dlgrecording.h b/src/library/recording/dlgrecording.h
index 079fd2d9af..34e1d5d91c 100644
--- a/src/library/recording/dlgrecording.h
+++ b/src/library/recording/dlgrecording.h
@@ -35,7 +35,7 @@ class DlgRecording : public QWidget, public Ui::DlgRecording, public virtual Lib
     void moveSelection(int delta) override;
     inline const QString currentSearch() { return m_proxyModel.currentSearch(); }
     void saveCurrentViewState() override;
-    void restoreCurrentViewState() override;
+    bool restoreCurrentViewState() override;
 
   public slots:
     void slotRecordingStateChanged(bool);
diff --git a/src/library/searchqueryparser.cpp b/src/library/searchqueryparser.cpp
index 591d7fd184..5430ceba4f 100644
--- a/src/library/searchqueryparser.cpp
+++ b/src/library/searchqueryparser.cpp
@@ -1,9 +1,14 @@
 #include "library/searchqueryparser.h"
 
+#include <QRegularExpression>
+
 #include "track/keyutils.h"
 
 constexpr char kNegatePrefix[] = "-";
 constexpr char kFuzzyPrefix[] = "~";
+// see https://stackoverflow.com/questions/1310473/regex-matching-spaces-but-not-in-strings
+const QRegularExpression kSplitIntoWordsRegexp = QRegularExpression(
+        QStringLiteral(" (?=[^\"]*(\"[^\"]*\"[^\"]*)*$)"));
 
 SearchQueryParser::SearchQueryParser(TrackCollection* pTrackCollection)
     : m_pTrackCollection(pTrackCollection) {
@@ -271,3 +276,43 @@ std::unique_ptr<QueryNode> SearchQueryParser::parseQuery(const QString& query,
 
     return pQuery;
 }
+
+QStringList SearchQueryParser::splitQueryIntoWords(const QString& query) {
+    QStringList queryWordList = query.split(kSplitIntoWordsRegexp, QString::SkipEmptyParts);
+    return queryWordList;
+}
+
+bool SearchQueryParser::queryIsLessSpecific(const QString& original, const QString& changed) {
+    // separate search query into tokens
+    QStringList oldWordList = SearchQueryParser::splitQueryIntoWords(original);
+    QStringList newWordList = SearchQueryParser::splitQueryIntoWords(changed);
+
+    // we sort the lists for length so the comperator will pop the longest match first
+    std::sort(oldWordList.begin(), oldWordList.end(), [=](const QString& v1, const QString& v2) {
+        return v1.length() > v2.length();
+    });
+    std::sort(newWordList.begin(), newWordList.end(), [=](const QString& v1, const QString& v2) {
+        return v1.length() > v2.length();
+    });
+
+    for (int i = 0; i < oldWordList.length(); i++) {
+        const QString& oldWord = oldWordList.at(i);
+        for (int j = 0; j < newWordList.length(); j++) {
+            const QString& newWord = newWordList.at(j);
+            // Note(ronso0) Look for missing '~' in newWord (fuzzy matching)?
+            if ((oldWord.startsWith("-") && oldWord.startsWith(newWord)) ||
+                    (!newWord.contains(":") && oldWord.contains(newWord)) ||
+                    (newWord.contains(":") && oldWord.startsWith(newWord))) {
+                // we found a match and can remove the search term list
+                newWordList.removeAt(j);
+                break;
+            }
+        }
+    }
+    // if the new search query list contains no more terms, we have a reduced
+    // search term
+    if (newWordList.empty()) {
+        return true;
+    }
+    return false;
+}
diff --git a/src/library/searchqueryparser.h b/src/library/searchqueryparser.h
index e6f6599c83..02c17d272a 100644
--- a/src/library/searchqueryparser.h
+++ b/src/library/searchqueryparser.h
@@ -19,6 +19,10 @@ class SearchQueryParser {
             const QStringList& searchColumns,
             const QString& extraFilter) const;
 
+    /// splits the query into a list of terms
+    static QStringList splitQueryIntoWords(const QString& query);
+    /// checks if the changed search query is less specific then the original term
+    static bool queryIsLessSpecific(const QString& original, const QString& changed);
 
   private:
     void parseTokens(QStringList tokens,
diff --git a/src/test/searchqueryparsertest.cpp b/src/test/searchqueryparsertest.cpp
index 25b9893e90..beb87cfabe 100644
--- a/src/test/searchqueryparsertest.cpp
+++ b/src/test/searchqueryparsertest.cpp
@@ -832,7 +832,6 @@ TEST_F(SearchQueryParserTest, ShortCrateFilter) {
     EXPECT_TRUE(pQuery->match(pTrackC));
 }
 
-
 TEST_F(SearchQueryParserTest, CrateFilterEmpty) {
     // Empty should match everything
     auto pQuery(m_parser.parseQuery(QString("crate: "), QStringList(), ""));
@@ -996,3 +995,105 @@ TEST_F(SearchQueryParserTest, CrateFilterWithCrateFilterAndNegation){
                             ") AND (NOT (" + m_crateFilterQuery.arg(searchTermB) + "))"),
                  qPrintable(pQueryB->toSql()));
 }
+
+TEST_F(SearchQueryParserTest, SplitQueryIntoWords) {
+    QStringList rv = SearchQueryParser::splitQueryIntoWords(QString("a test b"));
+    QStringList ex = QStringList() << "a"
+                                   << "test"
+                                   << "b";
+    qDebug() << rv << ex;
+    EXPECT_EQ(rv, ex);
+
+    QStringList rv2 = SearchQueryParser::splitQueryIntoWords(QString("a \"test ' b\" x"));
+    QStringList ex2 = QStringList() << "a"
+                                    << "\"test ' b\""
+                                    << "x";
+    qDebug() << rv2 << ex2;
+    EXPECT_EQ(rv2, ex2);
+
+    QStringList rv3 = SearchQueryParser::splitQueryIntoWords(QString("a x"));
+    QStringList ex3 = QStringList() << "a"
+                                    << "x";
+    qDebug() << rv3 << ex3;
+    EXPECT_EQ(rv3, ex3);
+
+    QStringList rv4 = SearchQueryParser::splitQueryIntoWords(
+            QString("a crate:x title:\"S p A C e\" ~key:2m"));
+    QStringList ex4 = QStringList() << "a"
+                                    << "crate:x"
+                                    << "title:\"S p A C e\""
+                                    << "~key:2m";
+    qDebug() << rv4 << ex4;
+    EXPECT_EQ(rv4, ex4);
+}
+
+TEST_F(SearchQueryParserTest, QueryIsLessSpecific) {
+    // Generate a file name for the temporary file
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("searchme"),
+            QStringLiteral("searchm")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("A B C"),
+            QStringLiteral("A C")));
+
+    EXPECT_FALSE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("A B C"),
+            QStringLiteral("A D C")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("A D C"),
+            QStringLiteral("A D C ")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("A D  C "),
+            QStringLiteral("A D C")));
+
+    EXPECT_FALSE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("A B  C"),
+            QStringLiteral("A D C")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("A D C"),
+            QStringLiteral("A D C ")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("A D  C "),
+            QStringLiteral("A D C")));
+
+    EXPECT_FALSE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("A B  C"),
+            QStringLiteral("A D C")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("Abba1 Abba2 Abb"),
+            QStringLiteral("Abba1 Abba Abb")));
+
+    EXPECT_FALSE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("Abba1 Abba2 Abb"),
+            QStringLiteral("Abba1 Aba Abb")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("Abba1"),
+            QLatin1String("")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("Abba1"),
+            QStringLiteral("bba")));
+
+    EXPECT_TRUE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("crate:abc"),
+            QStringLiteral("crate:ab")));
+
+    EXPECT_FALSE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("crate:\"a b c\""),
+            QStringLiteral("crate:\"a c\"")));
+
+    EXPECT_FALSE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("-crate:\"a b c\""),
+            QStringLiteral("crate:\"a b c\"")));
+
+    EXPECT_FALSE(SearchQueryParser::queryIsLessSpecific(
+            QStringLiteral("-crate:\"a b c\""),
+            QStringLiteral("crate:\"a b c\"")));
+}
diff --git a/src/widget/wlibrarytableview.cpp b/src/widget/wlibrarytableview.cpp
index ee2a847981..8bfbb207f0 100644
--- a/src/widget/wlibrarytableview.cpp
+++ b/src/widget/wlibrarytableview.cpp
@@ -133,12 +133,12 @@ void WLibraryTableView::saveTrackModelState(
     m_modelStateCache.insert(key, state, 1);
 }
 
-void WLibraryTableView::restoreTrackModelState(
+bool WLibraryTableView::restoreTrackModelState(
         const QAbstractItemModel* model, const QString& key) {
     //qDebug() << "restoreTrackModelState:" << model << key;
     //qDebug() << m_modelStateCache.keys();
     if (model == nullptr) {
-        return;
+        return false;
     }
 
     ModelState* state = m_modelStateCache.take(key);
@@ -148,7 +148,7 @@ void WLibraryTableView::restoreTrackModelState(
         verticalScrollBar()->setValue(0);
         horizontalScrollBar()->setValue(0);
         setCurrentIndex(QModelIndex());
-        return;
+        return false;
     }
 
     verticalScrollBar()->setValue(state->verticalScrollPosition);
@@ -171,6 +171,7 @@ void WLibraryTableView::restoreTrackModelState(
 
     // reinsert the state into the cache
     m_modelStateCache.insert(key, state, 1);
+    return true;
 }
 
 void WLibraryTableView::setTrackTableFont(const QFont& font) {
@@ -212,13 +213,13 @@ void WLibraryTableView::saveCurrentViewState() {
     saveTrackModelState(currentModel, key);
 }
 
-void WLibraryTableView::restoreCurrentViewState() {
+bool WLibraryTableView::restoreCurrentViewState() {
     const QAbstractItemModel* currentModel = model();
     QString key = getModelStateKey();
     if (!currentModel || key.isEmpty()) {
-        return;
+        return false;
     }
-    restoreTrackModelState(currentModel, key);
+    return restoreTrackModelState(currentModel, key);
 }
 
 void WLibraryTableView::focusInEvent(QFocusEvent* event) {
diff --git a/src/widget/wlibrarytableview.h b/src/widget/wlibrarytableview.h
index 6fde3e9e25..4f0c00d176 100644
--- a/src/widget/wlibrarytableview.h
+++ b/src/widget/wlibrarytableview.h
@@ -38,9 +38,12 @@ class WLibraryTableView : public QTableView, public virtual LibraryView {
     /// item selection and current index values associated with model by given
     /// key and restores it
     /// @param key unique for trackmodel
-    void restoreTrackModelState(const QAbstractItemModel* model, const QString& key);
+    /// @return true if restore succeeded
+    bool restoreTrackModelState(const QAbstractItemModel* model, const QString& key);
     void saveCurrentViewState() override;
-    void restoreCurrentViewState() override;
+    /// @brief restores current view state.
+    /// @return true if restore succeeded
+    bool restoreCurrentViewState() override;
 
   signals:
     void loadTrack(TrackPointer pTrack);
diff --git a/src/widget/wtracktableview.cpp b/src/widget/wtracktableview.cpp
index b65dbe8860..4f99006e6d 100644
--- a/src/widget/wtracktableview.cpp
+++ b/src/widget/wtracktableview.cpp
@@ -11,6 +11,7 @@
 #include "library/library.h"
 #include "library/library_prefs.h"
 #include "library/librarytablemodel.h"
+#include "library/searchqueryparser.h"
 #include "library/trackcollection.h"
 #include "library/trackcollectionmanager.h"
 #include "mixer/playermanager.h"
@@ -482,8 +483,30 @@ void WTrackTableView::onSearch(const QString& text) {
     TrackModel* trackModel = getTrackModel();
     if (trackModel) {
         saveCurrentViewState();
+        bool queryIsLessSpecific = SearchQueryParser::queryIsLessSpecific(
+                trackModel->currentSearch(), text);
+        QList<TrackId> selectedTracks = getSelectedTrackIds();
+        TrackId prevTrack = getCurrentTrackId();
+        int prevColumn = 0;
+        if (currentIndex().isValid()) {
+            prevColumn = currentIndex().column();
+        }
         trackModel->search(text);
-        restoreCurrentViewState();
+        if (queryIsLessSpecific) {
+            // If the user removed query terms, we try to select the same
+            // tracks as before
+            setCurrentTrackId(prevTrack, prevColumn);
+            setSelectedTracks(selectedTracks);
+        } else {
+            // The user created a more specific search query, try to restore a
+            // previous state
+            if (!restoreCurrentViewState()) {
+                // We found no saved state for this query, try to select the
+                // tracks last active, if they are part of the result set
+                setCurrentTrackId(prevTrack, prevColumn);
+                setSelectedTracks(selectedTracks);
+            }
+        }
     }
 }
 
@@ -884,6 +907,26 @@ QList<TrackId> WTrackTableView::getSelectedTrackIds() const {
     return trackIds;
 }
 
+TrackId WTrackTableView::getCurrentTrackId() const {
+    TrackModel* pTrackModel = getTrackModel();
+    VERIFY_OR_DEBUG_ASSERT(pTrackModel != nullptr) {
+        qWarning() << "No selected tracks available";
+        return {};
+    }
+
+    QItemSelectionModel* pSelectionModel = selectionModel();
+    VERIFY_OR_DEBUG_ASSERT(pSelectionModel != nullptr) {
+        qWarning() << "No selection model available";
+        return {};
+    }
+
+    const QModelIndex current = pSelectionModel->currentIndex();
+    if (current.isValid()) {
+        return pTrackModel->getTrackId(current);
+    }
+    return {};
+}
+
 void WTrackTableView::setSelectedTracks(const QList<TrackId>& trackIds) {
     QItemSelectionModel* pSelectionModel = selectionModel();
     VERIFY_OR_DEBUG_ASSERT(pSelectionModel != nullptr) {
@@ -907,6 +950,34 @@ void WTrackTableView::setSelectedTracks(const QList<TrackId>& trackIds) {
     }
 }
 
+bool WTrackTableView::setCurrentTrackId(const TrackId& trackId, int column) {
+    QItemSelectionModel* pSelectionModel = selectionModel();
+    VERIFY_OR_DEBUG_ASSERT(pSelectionModel != nullptr) {
+        qWarning() << "No selected tracks available";
+        return false;
+    }
+
+    TrackModel* pTrackModel = getTrackModel();
+    VERIFY_OR_DEBUG_ASSERT(pTrackModel != nullptr) {
+        qWarning() << "No selected tracks available";
+        return false;
+    }
+    const QVector<int> trackRows = pTrackModel->getTrackRows(trackId);
+    if (trackRows.empty()) {
+        return false;
+    }
+
+    QModelIndex idx = model()->index(trackRows[0], column);
+    // In case the column is not visible pick the left-most one
+    if (isIndexHidden(idx)) {
+        idx = model()->index(idx.row(), columnAt(0));
+    }
+    selectRow(idx.row());
+    pSelectionModel->setCurrentIndex(idx,
+            QItemSelectionModel::SelectCurrent | QItemSelectionModel::Select);
+    return true;
+}
+
 void WTrackTableView::addToAutoDJ(PlaylistDAO::AutoDJSendLoc loc) {
     auto* trackModel = getTrackModel();
     if (!trackModel->hasCapabilities(TrackModel::Capability::AddToAutoDJ)) {
diff --git a/src/widget/wtracktableview.h b/src/widget/wtracktableview.h
index 33c385efef..1f228ebeb9 100644
--- a/src/widget/wtracktableview.h
+++ b/src/widget/wtracktableview.h
@@ -42,6 +42,8 @@ class WTrackTableView : public WLibraryTableView {
     TrackModel::SortColumnId getColumnIdFromCurrentIndex() override;
     QList<TrackId> getSelectedTrackIds() const;
     void setSelectedTracks(const QList<TrackId>& tracks);
+    TrackId getCurrentTrackId() const;
+    bool setCurrentTrackId(const TrackId& trackId, int column = 0);
 
     double getBackgroundColorOpacity() const {
         return m_backgroundColorOpacity;
@@ -68,8 +70,8 @@ class WTrackTableView : public WLibraryTableView {
     void slotSaveCurrentViewState() {
         saveCurrentViewState();
     };
-    void slotRestoreCurrentViewState() {
-        restoreCurrentViewState();
+    bool slotRestoreCurrentViewState() {
+        return restoreCurrentViewState();
     };
 
   protected: