better selection restoration model

- save TrackIds instead of index if possible
- restore model state only if the search query is less specific
- try to select the previous track if no old state exists
- fix scrollTo by executing in a later loop

src/library/autodj/dlgautodj.h

@@ -36,8 +36,8 @@ class DlgAutoDJ : public QWidget, public Ui::DlgAutoDJ, public LibraryView {
     void saveCurrentViewState() override {
         m_pTrackTableView->saveCurrentViewState();
     };
-    bool restoreCurrentViewState() override {
-        return m_pTrackTableView->restoreCurrentViewState();
+    bool restoreCurrentViewState(bool fromSearch = false) override {
+        return m_pTrackTableView->restoreCurrentViewState(fromSearch);
     };
 
   public slots:

src/library/basesqltablemodel.cpp

@@ -191,6 +191,7 @@ void BaseSqlTableModel::select() {
     if (!m_bInitialized) {
         return;
     }
+    beginResetModel();
     // We should be able to detect when a select() would be a no-op. The DAO's
     // do not currently broadcast signals for when common things happen. In the
     // future, we can turn this check on and avoid a lot of needless
@@ -329,7 +330,7 @@ void BaseSqlTableModel::select() {
             std::move(trackIdToRows));
     // Both rowInfo and trackIdToRows (might) have been moved and
     // must not be used afterwards!
-
+    endResetModel();
     qDebug() << this << "select() took" << time.elapsed().debugMillisWithUnit()
              << m_rowInfo.size();
 }

src/library/dlganalysis.h

@@ -37,8 +37,8 @@ class DlgAnalysis : public QWidget, public Ui::DlgAnalysis, public virtual Libra
     void saveCurrentViewState() override {
         m_pAnalysisLibraryTableView->saveCurrentViewState();
     };
-    bool restoreCurrentViewState() override {
-        return m_pAnalysisLibraryTableView->restoreCurrentViewState();
+    bool restoreCurrentViewState(bool fromSearch = false) override {
+        return m_pAnalysisLibraryTableView->restoreCurrentViewState(fromSearch);
     };
 
   public slots:

src/library/dlghidden.h

@@ -28,8 +28,8 @@ class DlgHidden : public QWidget, public Ui::DlgHidden, public LibraryView {
     void saveCurrentViewState() override {
         m_pTrackTableView->saveCurrentViewState();
     };
-    bool restoreCurrentViewState() override {
-        return m_pTrackTableView->restoreCurrentViewState();
+    bool restoreCurrentViewState(bool fromSearch = false) override {
+        return m_pTrackTableView->restoreCurrentViewState(fromSearch);
     };
 
   public slots:

src/library/dlgmissing.h

@@ -28,8 +28,8 @@ class DlgMissing : public QWidget, public Ui::DlgMissing, public LibraryView {
     void saveCurrentViewState() override {
         m_pTrackTableView->saveCurrentViewState();
     };
-    bool restoreCurrentViewState() override {
-        return m_pTrackTableView->restoreCurrentViewState();
+    bool restoreCurrentViewState(bool fromSearch = false) override {
+        return m_pTrackTableView->restoreCurrentViewState(fromSearch);
     };
 
   public slots:

src/library/libraryview.h

@@ -27,7 +27,8 @@ class LibraryView {
     virtual void slotAddToAutoDJTop() {};
     virtual void slotAddToAutoDJReplace() {};
     virtual void saveCurrentViewState(){};
-    virtual bool restoreCurrentViewState() {
+    virtual bool restoreCurrentViewState(bool fromSearch = false) {
+        Q_UNUSED(fromSearch);
         return false;
     };
 

src/library/recording/dlgrecording.cpp

@@ -72,7 +72,7 @@ DlgRecording::DlgRecording(
     connect(&m_browseModel,
             &BrowseTableModel::restoreModelState,
             m_pTrackTableView,
-            &WTrackTableView::restoreCurrentViewState);
+            &WTrackTableView::slotRestoreCurrentViewState);
 
     QBoxLayout* box = qobject_cast<QBoxLayout*>(layout());
     VERIFY_OR_DEBUG_ASSERT(box) { //Assumes the form layout is a QVBox/QHBoxLayout!

src/library/recording/dlgrecording.h

@@ -36,8 +36,8 @@ class DlgRecording : public QWidget, public Ui::DlgRecording, public virtual Lib
     void saveCurrentViewState() override {
         m_pTrackTableView->saveCurrentViewState();
     };
-    bool restoreCurrentViewState() override {
-        return m_pTrackTableView->restoreCurrentViewState();
+    bool restoreCurrentViewState(bool fromSearch = false) override {
+        return m_pTrackTableView->restoreCurrentViewState(fromSearch);
     };
 
   public slots:

src/library/searchqueryparser.cpp

@@ -1,11 +1,14 @@
 #include "library/searchqueryparser.h"
 
-#include "util/compatibility.h"
+#include <QRegExp>
 
 #include "track/keyutils.h"
+#include "util/compatibility.h"
 
 constexpr char kNegatePrefix[] = "-";
 constexpr char kFuzzyPrefix[] = "~";
+// see https://stackoverflow.com/questions/1310473/regex-matching-spaces-but-not-in-strings
+const QRegExp kSplitRegexp = QRegExp(" (?=[^\"]*(\"[^\"]*\"[^\"]*)*$)");
 
 SearchQueryParser::SearchQueryParser(TrackCollection* pTrackCollection)
     : m_pTrackCollection(pTrackCollection) {
@@ -258,9 +261,50 @@ std::unique_ptr<QueryNode> SearchQueryParser::parseQuery(const QString& query,
     }
 
     if (!query.isEmpty()) {
+        // FIXME(poelzi): refactor into a tokanizer that properly understands
+        // how search queries are build up and parses " correctly.
         QStringList tokens = query.split(" ");
         parseTokens(tokens, searchColumns, pQuery.get());
     }
 
     return pQuery;
 }
+
+QStringList SearchQueryParser::splitQuery(const QString& query) {
+    QStringList splitted = query.split(kSplitRegexp);
+    return splitted;
+}
+
+bool SearchQueryParser::isReducedTerm(const QString& original, const QString& changed) {
+    // seperate search query into tokens
+    QStringList oList = SearchQueryParser::splitQuery(original);
+    QStringList nList = SearchQueryParser::splitQuery(changed);
+
+    // we sort the lists for length so the comperator will pop the longest match first
+    std::sort(oList.begin(), oList.end(), [=](const QString& v1, const QString& v2) {
+        return v1.length() > v2.length();
+    });
+    std::sort(nList.begin(), nList.end(), [=](const QString& v1, const QString& v2) {
+        return v1.length() > v2.length();
+    });
+
+    for (int i = 0; i < oList.length(); i++) {
+        const QString& oword = oList.at(i);
+        for (int j = 0; j < nList.length(); j++) {
+            const QString& nword = nList.at(j);
+            if ((oword.startsWith("-") && oword.startsWith(nword)) ||
+                    (!nword.contains(":") && oword.contains(nword)) ||
+                    (nword.contains(":") && oword.startsWith(nword))) {
+                // we found a match and can remove the search term list
+                nList.removeAt(j);
+                break;
+            }
+        }
+    }
+    // if the new search query list contains no more terms, we have a reduced
+    // search term
+    if (nList.empty()) {
+        return true;
+    }
+    return false;
+}

src/library/searchqueryparser.h

@@ -19,6 +19,10 @@ class SearchQueryParser {
             const QStringList& searchColumns,
             const QString& extraFilter) const;
 
+    static QStringList splitQuery(const QString& query);
+    /// splits the query into a list of terms
+    static bool isReducedTerm(const QString& original, const QString& changed);
+    /// checks if the changed search query is less specific then the original term
 
   private:
     void parseTokens(QStringList tokens,

src/test/searchqueryparsertest.cpp

@@ -990,3 +990,90 @@ TEST_F(SearchQueryParserTest, CrateFilterWithCrateFilterAndNegation){
                             ") AND (NOT (" + m_crateFilterQuery.arg(searchTermB) + "))"),
                  qPrintable(pQueryB->toSql()));
 }
+
+TEST_F(SearchQueryParserTest, SplitTest) {
+    QStringList rv = SearchQueryParser::splitQuery(QString("a test b"));
+    QStringList ex = QStringList() << "a"
+                                   << "test"
+                                   << "b";
+    qDebug() << rv << ex;
+    EXPECT_EQ(rv, ex);
+
+    QStringList rv2 = SearchQueryParser::splitQuery(QString("a \"test ' b\" x"));
+    QStringList ex2 = QStringList() << "a"
+                                    << "\"test ' b\""
+                                    << "x";
+    qDebug() << rv2 << ex2;
+    EXPECT_EQ(rv2, ex2);
+
+    QStringList rv3 = SearchQueryParser::splitQuery(QString("a x"));
+    QStringList ex3 = QStringList() << "a"
+                                    << "x";
+    qDebug() << rv3 << ex3;
+    EXPECT_EQ(rv3, ex3);
+
+    QStringList rv4 = SearchQueryParser::splitQuery(
+            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, testIsReduced) {
+    // Generate a file name for the temporary file
+    EXPECT_TRUE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("searchme"),
+            QStringLiteral("searchm")));
+
+    EXPECT_TRUE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("A B C"),
+            QStringLiteral("A C")));
+
+    EXPECT_FALSE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("A B C"),
+            QStringLiteral("A D C")));
+
+    EXPECT_TRUE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("Abba1 Abba2 Abb"),
+            QStringLiteral("Abba1 Abba Abb")));
+
+    EXPECT_FALSE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("Abba1 Abba2 Abb"),
+            QStringLiteral("Abba1 Aba Abb")));
+
+    EXPECT_TRUE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("Abba1"),
+            QStringLiteral("")));
+
+    EXPECT_TRUE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("Abba1"),
+            QStringLiteral("bba")));
+
+    EXPECT_TRUE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("crate:abc"),
+            QStringLiteral("crate:ab")));
+
+    // FIXME(poelzi): the architecture of the query parser makes it
+    // very hard to correctly understand the search query since the
+    // interpretation and lexing is done in the same step.
+    // The query parser should be correctly split into lexing and query
+    // construction
+    // EXPECT_TRUE(StringUtils::isReducedTerm(
+    //     QStringLiteral("crate:\"a b c\""),
+    //     QStringLiteral("crate:\"a b\"")));
+
+    EXPECT_FALSE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("crate:\"a b c\""),
+            QStringLiteral("crate:\"a c\"")));
+
+    EXPECT_FALSE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("-crate:\"a b c\""),
+            QStringLiteral("crate:\"a b c\"")));
+
+    EXPECT_FALSE(SearchQueryParser::isReducedTerm(
+            QStringLiteral("-crate:\"a b c\""),
+            QStringLiteral("crate:\"a b c\"")));
+}

src/widget/wlibrarytableview.cpp

@@ -103,7 +103,7 @@ void WLibraryTableView::moveSelection(int delta) {
                 delta++;
             }
         }
-        // this wired combination works when switching between models
+        // this weired combination works when switching between models
         setCurrentIndex(newIndex);
         currentSelection->select(newIndex, QItemSelectionModel::ClearAndSelect);
         // why does scrollTo not work ?
@@ -126,16 +126,7 @@ void WLibraryTableView::saveTrackModelState(const QAbstractItemModel* model, con
     // qDebug() << "save: saveTrackModelState:" << key << model << verticalScrollBar()->value() << " | ";
     state->verticalScrollPosition = verticalScrollBar()->value();
     state->horizontalScrollPosition = horizontalScrollBar()->value();
-    if (!selectionModel()->selectedIndexes().isEmpty()) {
-        state->selectionIndex = selectionModel()->selectedIndexes();
-    } else {
-        state->selectionIndex = QModelIndexList();
-    }
-    if (selectionModel()->currentIndex().isValid()) {
-        state->currentIndex = selectionModel()->currentIndex();
-    } else {
-        state->currentIndex = QModelIndex();
-    }
+    fillModelState(*state);
     m_modelStateCache.insert(key, state, 1);
 
     WTrackTableViewHeader* pHeader = qobject_cast<WTrackTableViewHeader*>(horizontalHeader());
@@ -145,16 +136,18 @@ void WLibraryTableView::saveTrackModelState(const QAbstractItemModel* model, con
 }
 
 bool WLibraryTableView::restoreTrackModelState(
-        const QAbstractItemModel* model, const QString& key) {
+        const QAbstractItemModel* model, const QString& key, bool fromSearch) {
     updateGeometries();
     //qDebug() << "restoreTrackModelState:" << key << model << m_vModelState.keys();
     if (model == nullptr) {
         return false;
     }
 
-    WTrackTableViewHeader* pHeader = qobject_cast<WTrackTableViewHeader*>(horizontalHeader());
-    if (pHeader) {
-        pHeader->restoreHeaderState();
+    if (!fromSearch) {
+        WTrackTableViewHeader* pHeader = qobject_cast<WTrackTableViewHeader*>(horizontalHeader());
+        if (pHeader) {
+            pHeader->restoreHeaderState();
+        }
     }
 
     ModelState* state = m_modelStateCache.take(key);
@@ -165,19 +158,40 @@ bool WLibraryTableView::restoreTrackModelState(
     verticalScrollBar()->setValue(state->verticalScrollPosition);
     horizontalScrollBar()->setValue(state->horizontalScrollPosition);
 
+    applyModelState(*state);
+    // reinsert the state into the cache
+    m_modelStateCache.insert(key, state, 1);
+    return true;
+}
+
+void WLibraryTableView::fillModelState(ModelState& state) {
+    if (!selectionModel()->selectedIndexes().isEmpty()) {
+        state.selectionIndex = selectionModel()->selectedIndexes();
+    } else {
+        state.selectionIndex = QModelIndexList();
+    }
+    if (selectionModel()->currentIndex().isValid()) {
+        state.currentIndex = selectionModel()->currentIndex();
+    } else {
+        state.currentIndex = QModelIndex();
+    }
+}
+
+bool WLibraryTableView::applyModelState(ModelState& state) {
     auto selection = selectionModel();
     selection->clearSelection();
-    if (!state->selectionIndex.isEmpty()) {
-        for (auto index : qAsConst(state->selectionIndex)) {
+    if (!state.selectionIndex.isEmpty()) {
+        for (auto index : qAsConst(state.selectionIndex)) {
             selection->select(index, QItemSelectionModel::Select);
         }
     }
-    if (state->currentIndex.isValid()) {
-        selection->setCurrentIndex(state->currentIndex, QItemSelectionModel::NoUpdate);
+    if (state.currentIndex.isValid()) {
+        selection->setCurrentIndex(state.currentIndex,
+                QItemSelectionModel::Select |
+                        QItemSelectionModel::SelectCurrent);
+        return true;
     }
-    // reinsert the state into the cache
-    m_modelStateCache.insert(key, state, 1);
-    return true;
+    return false;
 }
 
 void WLibraryTableView::setTrackTableFont(const QFont& font) {
@@ -208,13 +222,13 @@ void WLibraryTableView::saveCurrentViewState() {
     saveTrackModelState(currentModel, key);
 }
 
-bool WLibraryTableView::restoreCurrentViewState() {
+bool WLibraryTableView::restoreCurrentViewState(bool fromSearch) {
     const QAbstractItemModel* currentModel = model();
     QString key = getStateKey();
     if (!currentModel || key.isEmpty()) {
         return false;
     }
-    return restoreTrackModelState(currentModel, key);
+    return restoreTrackModelState(currentModel, key, fromSearch);
 }
 
 void WLibraryTableView::focusInEvent(QFocusEvent* event) {