Merge pull request #1160 from isanae/filetree-bugs

Filetree bugs and improvements

7 files changed, 194 insertions, 77 deletions

diff --git a/src/datatab.cpp b/src/datatab.cpp
index af7eb18f..21c205e5 100644
--- a/src/datatab.cpp
+++ b/src/datatab.cpp
@@ -118,9 +118,9 @@ void DataTab::updateTree()
 void DataTab::ensureFullyLoaded()
 {
   if (!m_filetree->fullyLoaded()) {
-    m_filter.proxyModel()->setRecursiveFilteringEnabled(false);
+    m_filter.setFilteringEnabled(false);
     m_filetree->ensureFullyLoaded();
-    m_filter.proxyModel()->setRecursiveFilteringEnabled(true);
+    m_filter.setFilteringEnabled(true);
   }
 }
 
diff --git a/src/filetree.cpp b/src/filetree.cpp
index 2d92c954..1dadfaad 100644
--- a/src/filetree.cpp
+++ b/src/filetree.cpp
@@ -802,11 +802,11 @@ void FileTree::addCommonMenus(QMenu& menu)
     .addTo(menu);
 
   MenuItem(tr("Ex&pand All"))
-    .callback([&]{ m_tree->expandAll(); })
+    .callback([&]{ expandAll(); })
     .addTo(menu);
 
   MenuItem(tr("&Collapse All"))
-    .callback([&]{ m_tree->collapseAll(); })
+    .callback([&]{ collapseAll(); })
     .addTo(menu);
 }
 
@@ -820,3 +820,15 @@ QModelIndex FileTree::proxiedIndex(const QModelIndex& index)
     return index;
   }
 }
+
+void FileTree::collapseAll()
+{
+  m_tree->collapseAll();
+}
+
+void FileTree::expandAll()
+{
+  m_model->aboutToExpandAll();
+  m_tree->expandAll();
+  m_model->expandedAll();
+}
diff --git a/src/filetree.h b/src/filetree.h
index 2669e53b..8d71abb3 100644
--- a/src/filetree.h
+++ b/src/filetree.h
@@ -25,6 +25,9 @@ public:
   bool fullyLoaded() const;
   void ensureFullyLoaded();
 
+  void expandAll();
+  void collapseAll();
+
   void open(FileTreeItem* item=nullptr);
   void openHooked(FileTreeItem* item=nullptr);
   void preview(FileTreeItem* item=nullptr);
diff --git a/src/filetreeitem.cpp b/src/filetreeitem.cpp
index 788b4129..49bc65ac 100644
--- a/src/filetreeitem.cpp
+++ b/src/filetreeitem.cpp
@@ -14,9 +14,7 @@ constexpr bool AlwaysSortDirectoriesFirst = true;
 
 const QString& directoryFileType()
 {
-  static QString name;
-
-  if (name.isEmpty()) {
+  static const QString name = [] {
     const DWORD flags = SHGFI_TYPENAME;
     SHFILEINFOW sfi = {};
 
@@ -30,15 +28,88 @@ const QString& directoryFileType()
         "SHGetFileInfoW failed for folder file type, {}",
         formatSystemMessage(e));
 
-      name = "File folder";
+      return QString("File folder");
     } else {
-      name = QString::fromWCharArray(sfi.szTypeName);
+      return QString::fromWCharArray(sfi.szTypeName);
     }
-  }
+  }();
+
+  return name;
+}
+
+const QString& cachedFileTypeNoExtension()
+{
+  static const QString name = [] {
+    const DWORD flags = SHGFI_TYPENAME;
+    SHFILEINFOW sfi = {};
+
+    // dummy filename with no extension
+    const auto r = SHGetFileInfoW(L"file", 0, &sfi, sizeof(sfi), flags);
+
+    if (!r) {
+      const auto e = GetLastError();
+
+      log::error(
+        "SHGetFileInfoW failed for file without extension, {}",
+        formatSystemMessage(e));
+
+      return QString("File");
+    } else {
+      return QString::fromWCharArray(sfi.szTypeName);
+    }
+  }();
 
   return name;
 }
 
+const QString& cachedFileType(const std::wstring& file, bool isOnFilesystem)
+{
+  static std::map<std::wstring, QString, std::less<>> map;
+  static std::mutex mutex;
+
+  const auto dot = file.find_last_of(L'.');
+  if (dot == std::wstring::npos) {
+    return cachedFileTypeNoExtension();
+  }
+
+  std::scoped_lock lock(mutex);
+  const auto sv = std::wstring_view(file.c_str() + dot, file.size() - dot);
+
+  auto itor = map.find(sv);
+  if (itor != map.end()) {
+    return itor->second;
+  }
+
+
+  DWORD flags = SHGFI_TYPENAME;
+
+  if (!isOnFilesystem) {
+    // files from archives are not on the filesystem; this flag forces
+    // SHGetFileInfoW() to only work with the filename
+    flags |= SHGFI_USEFILEATTRIBUTES;
+  }
+
+  SHFILEINFOW sfi = {};
+  const auto r = SHGetFileInfoW(file.c_str(), 0, &sfi, sizeof(sfi), flags);
+
+  QString s;
+
+  if (!r) {
+    const auto e = GetLastError();
+
+    log::error(
+      "SHGetFileInfoW failed for '{}', {}",
+      file, formatSystemMessage(e));
+
+    s = cachedFileTypeNoExtension();
+  } else {
+    s = QString::fromWCharArray(sfi.szTypeName);
+  }
+
+  return map.emplace(sv, s).first->second;
+}
+
+
 
 FileTreeItem::FileTreeItem(
   FileTreeModel* model, FileTreeItem* parent,
@@ -176,50 +247,59 @@ public:
   }
 };
 
-void FileTreeItem::sort()
+void FileTreeItem::queueSort()
 {
   if (!m_children.empty()) {
-    m_model->sortItem(*this, true);
+    m_model->queueSortItem(this);
+  }
+}
+
+void FileTreeItem::makeSortingStale()
+{
+  m_sortingStale = true;
+
+  for (auto& c : m_children) {
+    c->makeSortingStale();
   }
 }
 
 void FileTreeItem::sort(int column, Qt::SortOrder order, bool force)
 {
-  if (!force && !m_expanded) {
+  if (!m_expanded) {
     m_sortingStale = true;
     return;
   }
 
-  if (m_sortingStale) {
+  if (m_sortingStale || force) {
     //log::debug("sorting is stale for {}, sorting now", debugName());
     m_sortingStale = false;
-  }
 
-  std::sort(m_children.begin(), m_children.end(), [&](auto&& a, auto&& b) {
-    int r = 0;
+    std::sort(m_children.begin(), m_children.end(), [&](auto&& a, auto&& b) {
+      int r = 0;
 
-    if (a->isDirectory() && !b->isDirectory()) {
-      if constexpr (AlwaysSortDirectoriesFirst) {
-        return true;
+      if (a->isDirectory() && !b->isDirectory()) {
+        if constexpr (AlwaysSortDirectoriesFirst) {
+          return true;
+        } else {
+          r = -1;
+        }
+      } else if (!a->isDirectory() && b->isDirectory()) {
+        if constexpr (AlwaysSortDirectoriesFirst) {
+          return false;
+        } else {
+          r = 1;
+        }
       } else {
-        r = -1;
+        r = FileTreeItem::Sorter::compare(column, a.get(), b.get());
       }
-    } else if (!a->isDirectory() && b->isDirectory()) {
-      if constexpr (AlwaysSortDirectoriesFirst) {
-        return false;
+
+      if (order == Qt::AscendingOrder) {
+        return (r < 0);
       } else {
-        r = 1;
+        return (r > 0);
       }
-    } else {
-      r = FileTreeItem::Sorter::compare(column, a.get(), b.get());
-    }
-
-    if (order == Qt::AscendingOrder) {
-      return (r < 0);
-    } else {
-      return (r > 0);
-    }
-  });
+    });
+  }
 
   for (auto& child : m_children) {
     child->sort(column, order, force);
@@ -321,28 +401,11 @@ void FileTreeItem::getFileType() const
     return;
   }
 
-  DWORD flags = SHGFI_TYPENAME;
-
-  if (isFromArchive()) {
-    // files from archives are not on the filesystem; this flag forces
-    // SHGetFileInfoW() to only work with the filename
-    flags |= SHGFI_USEFILEATTRIBUTES;
-  }
-
-  SHFILEINFOW sfi = {};
-  const auto r = SHGetFileInfoW(
-    m_wsRealPath.c_str(), 0, &sfi, sizeof(sfi), flags);
-
-  if (!r) {
-    const auto e = GetLastError();
-
-    log::error(
-      "SHGetFileInfoW failed for '{}', {}",
-      m_realPath, formatSystemMessage(e));
-
+  const auto& t = cachedFileType(m_wsRealPath, !isFromArchive());
+  if (t.isEmpty()) {
     m_fileType.fail();
   } else {
-    m_fileType.set(QString::fromWCharArray(sfi.szTypeName));
+    m_fileType.set(t);
   }
 }
 
diff --git a/src/filetreeitem.h b/src/filetreeitem.h
index 2092782e..750e4719 100644
--- a/src/filetreeitem.h
+++ b/src/filetreeitem.h
@@ -93,6 +93,7 @@ public:
   }
 
   void sort(int column, Qt::SortOrder order, bool force);
+  void makeSortingStale();
 
   FileTreeItem* parent()
   {
@@ -223,7 +224,7 @@ public:
     m_expanded = b;
 
     if (m_expanded && m_sortingStale) {
-      sort();
+      queueSort();
     }
   }
 
@@ -314,7 +315,7 @@ private:
     std::wstring dataRelativeParentPath, bool isDirectory, std::wstring file);
 
   void getFileType() const;
-  void sort();
+  void queueSort();
 };
 
 #endif // MODORGANIZER_FILETREEITEM_INCLUDED
diff --git a/src/filetreemodel.cpp b/src/filetreemodel.cpp
index 11901c43..59025479 100644
--- a/src/filetreemodel.cpp
+++ b/src/filetreemodel.cpp
@@ -196,13 +196,13 @@ void* makeInternalPointer(FileTreeItem* item)
 FileTreeModel::FileTreeModel(OrganizerCore& core, QObject* parent) :
   QAbstractItemModel(parent), m_core(core), m_enabled(true),
   m_root(FileTreeItem::createDirectory(this, nullptr, L"", L"")),
-  m_flags(NoFlags), m_fullyLoaded(false)
+  m_flags(NoFlags), m_fullyLoaded(false), m_sortingEnabled(true)
 {
   m_root->setExpanded(true);
+  m_sortTimer.setSingleShot(true);
 
   connect(&m_removeTimer, &QTimer::timeout, [&]{ removeItems(); });
   connect(&m_sortTimer, &QTimer::timeout, [&]{ sortItems(); });
-
   connect(&m_iconPendingTimer, &QTimer::timeout, [&]{ updatePendingIcons(); });
 }
 
@@ -212,6 +212,7 @@ void FileTreeModel::refresh()
 
   m_fullyLoaded = false;
   update(*m_root, *m_core.directoryStructure(), L"", false);
+  sortItem(*m_root, false);
 }
 
 void FileTreeModel::clear()
@@ -226,7 +227,7 @@ void FileTreeModel::clear()
 void FileTreeModel::recursiveFetchMore(const QModelIndex& m)
 {
   if (canFetchMore(m)) {
-    doFetchMore(m, false);
+    doFetchMore(m, false, false);
   }
 
   for (int i=0; i<rowCount(m); ++i) {
@@ -239,6 +240,7 @@ void FileTreeModel::ensureFullyLoaded()
   if (!m_fullyLoaded) {
     TimeThis tt("FileTreeModel:: fully loading for search");
     recursiveFetchMore(QModelIndex());
+    sortItem(*m_root, false);
     m_fullyLoaded = true;
   }
 }
@@ -253,6 +255,17 @@ void FileTreeModel::setEnabled(bool b)
   m_enabled = b;
 }
 
+void FileTreeModel::aboutToExpandAll()
+{
+  m_sortingEnabled = false;
+}
+
+void FileTreeModel::expandedAll()
+{
+  m_sortingEnabled = true;
+  sortItem(*m_root, false);
+}
+
 const FileTreeModel::SortInfo& FileTreeModel::sortInfo() const
 {
   return m_sort;
@@ -338,10 +351,11 @@ bool FileTreeModel::canFetchMore(const QModelIndex& parent) const
 
 void FileTreeModel::fetchMore(const QModelIndex& parent)
 {
-  doFetchMore(parent, true);
+  doFetchMore(parent, true, true);
 }
 
-void FileTreeModel::doFetchMore(const QModelIndex& parent, bool forFetch)
+void FileTreeModel::doFetchMore(
+  const QModelIndex& parent, bool forFetch, bool doSort)
 {
   FileTreeItem* item = itemFromIndex(parent);
   if (!item) {
@@ -360,6 +374,10 @@ void FileTreeModel::doFetchMore(const QModelIndex& parent, bool forFetch)
 
   const auto parentPath = item->dataRelativeParentPath();
   update(*item, *parentEntry, parentPath.toStdWString(), forFetch);
+
+  if (!forFetch && doSort) {
+    sortItem(*item, false);
+  }
 }
 
 QVariant FileTreeModel::data(const QModelIndex& index, int role) const
@@ -485,7 +503,7 @@ void FileTreeModel::sort(int column, Qt::SortOrder order)
   m_sort.column = column;
   m_sort.order = order;
 
-  sortItem(*m_root, false);
+  sortItem(*m_root, true);
 }
 
 FileTreeItem* FileTreeModel::itemFromIndex(const QModelIndex& index) const
@@ -558,11 +576,15 @@ void FileTreeModel::update(
   }
 
   if (added) {
+    parentItem.makeSortingStale();
+
     // see comment at the top of this file
-    if (forFetching)
-      queueSortItem(&parentItem);
-    else
-      sortItem(parentItem, true);
+    if (forFetching) {
+      // don't pass a specific item, this will start a timer and re-sort the
+      // whole tree, which is faster than potentially queuing every single
+      // node if the whole tree is expanded
+      queueSortItem(nullptr);
+    }
   }
 }
 
@@ -873,21 +895,32 @@ void FileTreeModel::removeItems()
 
 void FileTreeModel::queueSortItem(FileTreeItem* item)
 {
-  m_sortItems.push_back(item);
+  if (!m_sortingEnabled) {
+    return;
+  }
+
+  if (item) {
+    m_sortItems.push_back(item);
+  }
+
   m_sortTimer.start(1);
 }
 
 void FileTreeModel::sortItems()
 {
   // see comment at the top of this file
-  trace(log::debug("sort item timer: sorting {} items", m_sortItems.size()));
 
-  auto copy = std::move(m_sortItems);
-  m_sortItems.clear();
-  m_sortTimer.stop();
+  if (m_sortItems.empty()) {
+    sortItem(*m_root, false);
+  } else {
+    log::debug("sort item timer: sorting {} items", m_sortItems.size());
 
-  for (auto&& f : copy) {
-    sortItem(*f, true);
+    auto items = std::move(m_sortItems);
+    m_sortItems.clear();
+
+    for (auto* item : items) {
+      sortItem(*item, false);
+    }
   }
 }
 
diff --git a/src/filetreemodel.h b/src/filetreemodel.h
index 334a0577..73a18d91 100644
--- a/src/filetreemodel.h
+++ b/src/filetreemodel.h
@@ -61,6 +61,10 @@ public:
   bool enabled() const;
   void setEnabled(bool b);
 
+  void aboutToExpandAll();
+  void expandedAll();
+
+
   const SortInfo& sortInfo() const;
 
   QModelIndex index(int row, int col, const QModelIndex& parent={}) const override;
@@ -77,6 +81,7 @@ public:
 
   FileTreeItem* itemFromIndex(const QModelIndex& index) const;
   void sortItem(FileTreeItem& item, bool force);
+  void queueSortItem(FileTreeItem* item);
 
 private:
   class Range;
@@ -92,11 +97,12 @@ private:
   mutable QTimer m_iconPendingTimer;
   SortInfo m_sort;
   bool m_fullyLoaded;
+  bool m_sortingEnabled;
 
   // see top of filetreemodel.cpp
   std::vector<FileTreeItem*> m_removeItems;
-  QTimer m_removeTimer;
   std::vector<FileTreeItem*> m_sortItems;
+  QTimer m_removeTimer;
   QTimer m_sortTimer;
 
 
@@ -113,12 +119,11 @@ private:
     FileTreeItem& parentItem, const MOShared::DirectoryEntry& parentEntry,
     const std::wstring& parentPath, bool forFetching);
 
-  void doFetchMore(const QModelIndex& parent, bool forFetch);
+  void doFetchMore(const QModelIndex& parent, bool forFetch, bool doSort);
 
   void queueRemoveItem(FileTreeItem* item);
   void removeItems();
 
-  void queueSortItem(FileTreeItem* item);
   void sortItems();