hidden unique_ptr into FileTreeItem

refactored pruning
don't show shell menu for directories
added collapse all in context menu

8 files changed, 214 insertions, 84 deletions

diff --git a/src/filetree.cpp b/src/filetree.cpp
index a826ed9a..41b10586 100644
--- a/src/filetree.cpp
+++ b/src/filetree.cpp
@@ -442,8 +442,11 @@ void FileTree::onContextMenu(const QPoint &pos)
   const auto m = QApplication::keyboardModifiers();
 
   if (m & Qt::ShiftModifier) {
-    showShellMenu(pos);
-    return;
+    // if no shell menu was available, continue on and show the regular
+    // context menu
+    if (showShellMenu(pos)) {
+      return;
+    }
   }
 
   QMenu menu;
@@ -481,13 +484,14 @@ QMainWindow* getMainWindow(QWidget* w)
   return nullptr;
 }
 
-void FileTree::showShellMenu(QPoint pos)
+bool FileTree::showShellMenu(QPoint pos)
 {
   auto* mw = getMainWindow(m_tree);
 
   // menus by origin
   std::map<int, env::ShellMenu> menus;
   int totalFiles = 0;
+  bool hasDirectory = false;
 
   for (auto&& index : m_tree->selectionModel()->selectedRows()) {
     auto* item = m_model->itemFromIndex(index);
@@ -495,6 +499,16 @@ void FileTree::showShellMenu(QPoint pos)
       continue;
     }
 
+    if (item->isDirectory()) {
+      hasDirectory = true;
+
+      log::warn(
+        "directories do not have shell menus; '{}' selected",
+        item->filename());
+
+      continue;
+    }
+
     auto itor = menus.find(item->originID());
     if (itor == menus.end()) {
       itor = menus.emplace(item->originID(), mw).first;
@@ -543,8 +557,13 @@ void FileTree::showShellMenu(QPoint pos)
   }
 
   if (menus.empty()) {
-    log::warn("no menus to show");
-    return;
+    // don't warn if a directory was selected, a warning has already been
+    // logged above
+    if (!hasDirectory) {
+      log::warn("no menus to show");
+    }
+
+    return false;
   }
   else if (menus.size() == 1) {
     auto& menu = menus.begin()->second;
@@ -576,6 +595,8 @@ void FileTree::showShellMenu(QPoint pos)
 
     mc.exec(m_tree->viewport()->mapToGlobal(pos));
   }
+
+  return true;
 }
 
 void FileTree::addDirectoryMenus(QMenu&, FileTreeItem&)
@@ -720,7 +741,11 @@ void FileTree::addCommonMenus(QMenu& menu)
     .hint(QObject::tr("Refreshes the list"))
     .addTo(menu);
 
-  MenuItem(QObject::tr("E&xpand All"))
+  MenuItem(QObject::tr("Ex&pand All"))
     .callback([&]{ m_tree->expandAll(); })
     .addTo(menu);
+
+  MenuItem(QObject::tr("&Collapse All"))
+    .callback([&]{ m_tree->collapseAll(); })
+    .addTo(menu);
 }
diff --git a/src/filetree.h b/src/filetree.h
index 39c9d0c6..80704f7b 100644
--- a/src/filetree.h
+++ b/src/filetree.h
@@ -51,7 +51,7 @@ private:
 
   void onExpandedChanged(const QModelIndex& index, bool expanded);
   void onContextMenu(const QPoint &pos);
-  void showShellMenu(QPoint pos);
+  bool showShellMenu(QPoint pos);
 
   void addDirectoryMenus(QMenu& menu, FileTreeItem& item);
   void addFileMenus(QMenu& menu, const MOShared::FileEntry& file, int originID);
diff --git a/src/filetreeitem.cpp b/src/filetreeitem.cpp
index 6d42f2dc..da4ce701 100644
--- a/src/filetreeitem.cpp
+++ b/src/filetreeitem.cpp
@@ -58,7 +58,17 @@ FileTreeItem::FileTreeItem(
 {
 }
 
-void FileTreeItem::insert(std::unique_ptr<FileTreeItem> child, std::size_t at)
+FileTreeItem::Ptr FileTreeItem::create(
+  FileTreeItem* parent, int originID,
+  std::wstring dataRelativeParentPath, std::wstring realPath, Flags flags,
+  std::wstring file, std::wstring mod)
+{
+  return std::unique_ptr<FileTreeItem>(new FileTreeItem(
+    parent, originID, std::move(dataRelativeParentPath), std::move(realPath),
+    flags, std::move(file), std::move(mod)));
+}
+
+void FileTreeItem::insert(FileTreeItem::Ptr child, std::size_t at)
 {
   if (at > m_children.size()) {
     log::error(
diff --git a/src/filetreeitem.h b/src/filetreeitem.h
index c5418d43..8ef42289 100644
--- a/src/filetreeitem.h
+++ b/src/filetreeitem.h
@@ -9,7 +9,8 @@ class FileTreeItem
   class Sorter;
 
 public:
-  using Children = std::vector<std::unique_ptr<FileTreeItem>>;
+  using Ptr = std::unique_ptr<FileTreeItem>;
+  using Children = std::vector<Ptr>;
 
   enum Flag
   {
@@ -22,7 +23,7 @@ public:
 
   Q_DECLARE_FLAGS(Flags, Flag);
 
-  FileTreeItem(
+  static Ptr create(
     FileTreeItem* parent, int originID,
     std::wstring dataRelativeParentPath, std::wstring realPath, Flags flags,
     std::wstring file, std::wstring mod);
@@ -32,13 +33,13 @@ public:
   FileTreeItem(FileTreeItem&&) = default;
   FileTreeItem& operator=(FileTreeItem&&) = default;
 
-  void add(std::unique_ptr<FileTreeItem> child)
+  void add(Ptr child)
   {
     child->m_indexGuess = m_children.size();
     m_children.push_back(std::move(child));
   }
 
-  void insert(std::unique_ptr<FileTreeItem> child, std::size_t at);
+  void insert(Ptr child, std::size_t at);
 
   template <class Itor>
   void insert(Itor begin, Itor end, std::size_t at)
@@ -269,6 +270,12 @@ private:
   bool m_expanded;
   Children m_children;
 
+
+  FileTreeItem(
+    FileTreeItem* parent, int originID,
+    std::wstring dataRelativeParentPath, std::wstring realPath, Flags flags,
+    std::wstring file, std::wstring mod);
+
   void getFileType() const;
 };
 
diff --git a/src/filetreemodel.cpp b/src/filetreemodel.cpp
index 91e6198f..3890ad2e 100644
--- a/src/filetreemodel.cpp
+++ b/src/filetreemodel.cpp
@@ -65,7 +65,7 @@ public:
     const auto parentIndex = m_model->indexFromItem(m_parentItem);
 
     // make sure the number of items is the same as the size of this range
-    Q_ASSERT(static_cast<int>(toAdd.size()) == (last - m_first));
+    Q_ASSERT(static_cast<int>(toAdd.size()) == (last - m_first + 1));
 
     trace(log::debug("Range::add() {} to {}", m_first, last));
 
@@ -124,12 +124,24 @@ private:
 };
 
 
+FileTreeItem* getItem(const QModelIndex& index)
+{
+  return static_cast<FileTreeItem*>(index.internalPointer());
+}
+
+void* makeInternalPointer(FileTreeItem* item)
+{
+  return item;
+}
+
+
 FileTreeModel::FileTreeModel(OrganizerCore& core, QObject* parent) :
   QAbstractItemModel(parent), m_core(core),
-  m_root(nullptr, 0, L"", L"", FileTreeItem::Directory, L"", L"<root>"),
+  m_root(FileTreeItem::create(
+    nullptr, 0, L"", L"", FileTreeItem::Directory, L"", L"<root>")),
   m_flags(NoFlags)
 {
-  m_root.setExpanded(true);
+  m_root->setExpanded(true);
 
   connect(&m_iconPendingTimer, &QTimer::timeout, [&]{ updatePendingIcons(); });
 }
@@ -137,19 +149,19 @@ FileTreeModel::FileTreeModel(OrganizerCore& core, QObject* parent) :
 void FileTreeModel::refresh()
 {
   TimeThis tt("FileTreeModel::refresh()");
-  update(m_root, *m_core.directoryStructure(), L"");
+  update(*m_root, *m_core.directoryStructure(), L"");
 }
 
 void FileTreeModel::clear()
 {
   beginResetModel();
-  m_root.clear();
+  m_root->clear();
   endResetModel();
 }
 
 bool FileTreeModel::showArchives() const
 {
-  return (m_flags & Archives) && m_core.getArchiveParsing();
+  return (m_flags.testFlag(Archives) && m_core.getArchiveParsing());
 }
 
 QModelIndex FileTreeModel::index(
@@ -160,7 +172,7 @@ QModelIndex FileTreeModel::index(
       return {};
     }
 
-    return createIndex(row, col, parentItem);
+    return createIndex(row, col, makeInternalPointer(parentItem));
   }
 
   log::error("FileTreeModel::index(): parentIndex has no internal pointer");
@@ -173,7 +185,7 @@ QModelIndex FileTreeModel::parent(const QModelIndex& index) const
     return {};
   }
 
-  auto* parentItem = static_cast<FileTreeItem*>(index.internalPointer());
+  auto* parentItem = getItem(index);
   if (!parentItem) {
     log::error("FileTreeModel::parent(): no internal pointer");
     return {};
@@ -341,7 +353,7 @@ void FileTreeModel::sort(int column, Qt::SortOrder order)
     oldItems.push_back({itemFromIndex(index), index.column()});
   }
 
-  m_root.sort(column, order);
+  m_root->sort(column, order);
 
   QModelIndexList newList;
   newList.reserve(itemCount);
@@ -359,10 +371,10 @@ void FileTreeModel::sort(int column, Qt::SortOrder order)
 FileTreeItem* FileTreeModel::itemFromIndex(const QModelIndex& index) const
 {
   if (!index.isValid()) {
-    return &m_root;
+    return m_root.get();
   }
 
-  auto* parentItem = static_cast<FileTreeItem*>(index.internalPointer());
+  auto* parentItem = getItem(index);
   if (!parentItem) {
     log::error("FileTreeModel::itemFromIndex(): no internal pointer");
     return nullptr;
@@ -395,7 +407,7 @@ QModelIndex FileTreeModel::indexFromItem(FileTreeItem& item, int col) const
     return {};
   }
 
-  return createIndex(index, col, parent);
+  return createIndex(index, col, makeInternalPointer(parent));
 }
 
 void FileTreeModel::update(
@@ -477,35 +489,22 @@ void FileTreeModel::removeDisappearingDirectories(
       if (item->areChildrenVisible()) {
         // the item is currently expanded, update it
         update(*item, *d, parentPath);
-      } else if (item->isLoaded()) {
-        // the item is loaded (previously expanded but now collapsed), mark it
-        // as unloaded
-        item->setLoaded(false);
       }
 
-      if ((m_flags & PruneDirectories)) {
-        // this directory must be checked to see if it's empty so it can be
-        // pruned
-        bool prune = false;
-
-        if (item->isLoaded() && item->children().empty()) {
-          // item is loaded and has no children; prune it
-          prune = true;
-        } else {
-          // item is not loaded, so children have to be checked manually
-          if (!hasFilesAnywhere(*d)) {
-            // item wouldn't have any children, prune it
-            prune = true;
-          }
+      if (shouldShowFolder(*d, item.get())) {
+        // folder should be left in the list
+        if (!item->areChildrenVisible() && item->isLoaded()) {
+          // the item is loaded (previously expanded but now collapsed), mark
+          // it as unloaded so it updates when next expanded
+          item->setLoaded(false);
         }
+      } else {
+        // item wouldn't have any children, prune it
+        trace(log::debug("dir {} is empty and pruned", item->filename()));
 
-        if (prune) {
-          trace(log::debug("dir {} is empty and pruned", item->filename()));
-
-          range.includeCurrent();
-          currentRemoved = true;
-          ++itor;
-        }
+        range.includeCurrent();
+        currentRemoved = true;
+        ++itor;
       }
 
       if (!currentRemoved) {
@@ -536,7 +535,7 @@ bool FileTreeModel::addNewDirectories(
   // keeps track of the contiguous directories that need to be added to
   // avoid calling beginAddRows(), etc. for each item
   Range range(this, parentItem);
-  std::vector<std::unique_ptr<FileTreeItem>> toAdd;
+  std::vector<FileTreeItem::Ptr> toAdd;
   bool added = false;
 
   // for each directory on the filesystem
@@ -553,7 +552,7 @@ bool FileTreeModel::addNewDirectories(
       range.add(std::move(toAdd));
       toAdd.clear();
     } else {
-      if ((m_flags & PruneDirectories) && !hasFilesAnywhere(*d)) {
+      if (!shouldShowFolder(*d, nullptr)) {
         // this is a new directory, but it doesn't contain anything interesting
         trace(log::debug("new dir {}, empty and pruned", QString::fromStdWString(d->getName())));
 
@@ -656,7 +655,7 @@ bool FileTreeModel::addNewFiles(
 {
   // keeps track of the contiguous files that need to be added to
   // avoid calling beginAddRows(), etc. for each item
-  std::vector<std::unique_ptr<FileTreeItem>> toAdd;
+  std::vector<FileTreeItem::Ptr> toAdd;
   Range range(this, parentItem, firstFileRow);
   bool added = false;
 
@@ -705,11 +704,11 @@ bool FileTreeModel::addNewFiles(
   return added;
 }
 
-std::unique_ptr<FileTreeItem> FileTreeModel::createDirectoryItem(
+FileTreeItem::Ptr FileTreeModel::createDirectoryItem(
   FileTreeItem& parentItem, const std::wstring& parentPath,
   const DirectoryEntry& d)
 {
-  auto item = std::make_unique<FileTreeItem>(
+  auto item = FileTreeItem::create(
     &parentItem, 0, parentPath, L"", FileTreeItem::Directory,
     d.getName(), L"");
 
@@ -722,7 +721,7 @@ std::unique_ptr<FileTreeItem> FileTreeModel::createDirectoryItem(
   return item;
 }
 
-std::unique_ptr<FileTreeItem> FileTreeModel::createFileItem(
+FileTreeItem::Ptr FileTreeModel::createFileItem(
   FileTreeItem& parentItem, const std::wstring& parentPath,
   const FileEntry& file)
 {
@@ -739,7 +738,7 @@ std::unique_ptr<FileTreeItem> FileTreeModel::createFileItem(
     flags |= FileTreeItem::Conflicted;
   }
 
-  auto item = std::make_unique<FileTreeItem>(
+  auto item = FileTreeItem::create(
     &parentItem, originID, parentPath, file.getFullPath(), flags,
     file.getName(), makeModName(file, originID));
 
@@ -759,22 +758,54 @@ std::unique_ptr<FileTreeItem> FileTreeModel::createFileItem(
 bool FileTreeModel::shouldShowFile(const FileEntry& file) const
 {
   if (showConflictsOnly() && (file.getAlternatives().size() == 0)) {
+    // only conflicts should be shown, but this file is not conflicted
     return false;
   }
 
-  if (!showArchives()) {
-    bool isArchive = false;
-    file.getOrigin(isArchive);
-    return !isArchive;
+  if (!showArchives() && file.isFromArchive()) {
+    // files from archives shouldn't be shown, but this file is from an archive
+    return false;
   }
 
   return true;
 }
 
-bool FileTreeModel::hasFilesAnywhere(const DirectoryEntry& dir) const
+bool FileTreeModel::shouldShowFolder(
+  const DirectoryEntry& dir, const FileTreeItem* item) const
 {
+  bool shouldPrune = m_flags.testFlag(PruneDirectories);
+
+  if (m_core.settings().archiveParsing()) {
+    if (!m_flags.testFlag(Archives)) {
+      // archive parsing is enabled but the tree shouldn't show archives; this
+      // is a bit of a special case for folders because they have to be hidden
+      // regardless of the PruneDirectories flag if they only exist in archives
+      //
+      // note that this test is inaccurate: if a loose folder exists but is
+      // empty, and the same folder exists in an archive but is _not_ empty,
+      // then it's considered to exist _only_ in an archive and will be pruned
+      //
+      // if directories are ever made first-class so they can retain their
+      // origins, this test can be made more accurate
+      shouldPrune = true;
+    }
+  }
+
+  if (!shouldPrune) {
+    // always show folders regardless of their content
+    return true;
+  }
+
+  if (item) {
+    if (item->isLoaded() && item->children().empty()) {
+      // item is loaded and has no children; prune it
+      return false;
+    }
+  }
+
   bool foundFile = false;
 
+  // check all files in this directory, return early if a file should be shown
   dir.forEachFile([&](auto&& f) {
     if (shouldShowFile(f)) {
       foundFile = true;
@@ -791,11 +822,9 @@ bool FileTreeModel::hasFilesAnywhere(const DirectoryEntry& dir) const
     return true;
   }
 
-  std::vector<DirectoryEntry*>::const_iterator begin, end;
-  dir.getSubDirectories(begin, end);
-
-  for (auto itor=begin; itor!=end; ++itor) {
-    if (hasFilesAnywhere(**itor)) {
+  // recurse into subdirectories
+  for (auto subdir : dir.getSubDirectories()) {
+    if (shouldShowFolder(*subdir, nullptr)) {
       return true;
     }
   }
diff --git a/src/filetreemodel.h b/src/filetreemodel.h
index 23640ac5..03bae602 100644
--- a/src/filetreemodel.h
+++ b/src/filetreemodel.h
@@ -69,7 +69,7 @@ private:
 
   using DirectoryIterator = std::vector<MOShared::DirectoryEntry*>::const_iterator;
   OrganizerCore& m_core;
-  mutable FileTreeItem m_root;
+  mutable FileTreeItem::Ptr m_root;
   Flags m_flags;
   mutable IconFetcher m_iconFetcher;
   mutable std::vector<QModelIndex> m_iconPending;
@@ -117,11 +117,11 @@ private:
     const std::unordered_set<MOShared::FileEntry::Index>& seen);
 
 
-  std::unique_ptr<FileTreeItem> createDirectoryItem(
+  FileTreeItem::Ptr createDirectoryItem(
     FileTreeItem& parentItem, const std::wstring& parentPath,
     const MOShared::DirectoryEntry& d);
 
-  std::unique_ptr<FileTreeItem> createFileItem(
+  FileTreeItem::Ptr createFileItem(
     FileTreeItem& parentItem, const std::wstring& parentPath,
     const MOShared::FileEntry& file);
 
@@ -134,7 +134,7 @@ private:
   void removePendingIcons(const QModelIndex& parent, int first, int last);
 
   bool shouldShowFile(const MOShared::FileEntry& file) const;
-  bool hasFilesAnywhere(const MOShared::DirectoryEntry& dir) const;
+  bool shouldShowFolder(const MOShared::DirectoryEntry& dir, const FileTreeItem* item) const;
   QString makeTooltip(const FileTreeItem& item) const;
   QVariant makeIcon(const FileTreeItem& item, const QModelIndex& index) const;
 
diff --git a/src/shared/util.cpp b/src/shared/util.cpp
index 009aad70..aa4ad4b3 100644
--- a/src/shared/util.cpp
+++ b/src/shared/util.cpp
@@ -25,6 +25,8 @@ along with Mod Organizer.  If not, see <http://www.gnu.org/licenses/>.
 #include <usvfs.h>

 #include <usvfs_version.h>

 

+using namespace MOBase;

+

 namespace MOShared

 {

 

@@ -199,7 +201,7 @@ std::wstring GetFileVersionString(const std::wstring &fileName)
   }

 }

 

-MOBase::VersionInfo createVersionInfo()

+VersionInfo createVersionInfo()

 {

   VS_FIXEDFILEINFO version = GetFileVersion(QApplication::applicationFilePath().toStdWString());

 

@@ -222,25 +224,25 @@ MOBase::VersionInfo createVersionInfo()
     if (noLetters)

     {

       // Default to pre-alpha when release type is unspecified

-      return MOBase::VersionInfo(version.dwFileVersionMS >> 16,

-                                 version.dwFileVersionMS & 0xFFFF,

-                                 version.dwFileVersionLS >> 16,

-                                 version.dwFileVersionLS & 0xFFFF,

-                                 MOBase::VersionInfo::RELEASE_PREALPHA);

+      return VersionInfo(version.dwFileVersionMS >> 16,

+                         version.dwFileVersionMS & 0xFFFF,

+                         version.dwFileVersionLS >> 16,

+                         version.dwFileVersionLS & 0xFFFF,

+                         VersionInfo::RELEASE_PREALPHA);

     }

     else

     {

       // Trust the string to make sense

-      return MOBase::VersionInfo(versionString);

+      return VersionInfo(versionString);

     }

   }

   else

   {

     // Non-pre-release builds just need their version numbers reading

-    return MOBase::VersionInfo(version.dwFileVersionMS >> 16,

-                               version.dwFileVersionMS & 0xFFFF,

-                               version.dwFileVersionLS >> 16,

-                               version.dwFileVersionLS & 0xFFFF);

+    return VersionInfo(version.dwFileVersionMS >> 16,

+                       version.dwFileVersionMS & 0xFFFF,

+                       version.dwFileVersionLS >> 16,

+                       version.dwFileVersionLS & 0xFFFF);

   }

 }

 

@@ -314,6 +316,62 @@ void SetThisThreadName(const QString& s)
   }

 }

 

+

+char shortcutChar(const QAction* a)

+{

+  const auto text = a->text();

+  char shortcut = 0;

+

+  for (int i=0; i<text.size(); ++i) {

+    const auto c = text[i];

+    if (c == '&') {

+      if (i >= (text.size() - 1)) {

+        log::error("ampersand at the end");

+        return 0;

+      }

+

+      return text[i + 1].toLatin1();

+    }

+  }

+

+  log::error("action {} has no shortcut", text);

+  return 0;

+}

+

+void checkDuplicateShortcuts(const QMenu& m)

+{

+  const auto actions = m.actions();

+

+  for (int i=0; i<actions.size(); ++i) {

+    const auto* action1 = actions[i];

+    if (action1->isSeparator()) {

+      continue;

+    }

+

+    const char shortcut1 = shortcutChar(action1);

+    if (shortcut1 == 0) {

+      continue;

+    }

+

+    for (int j=i+1; j<actions.size(); ++j) {

+      const auto* action2 = actions[j];

+      if (action2->isSeparator()) {

+        continue;

+      }

+

+      const char shortcut2 = shortcutChar(action2);

+

+      if (shortcut1 == shortcut2) {

+        log::error(

+          "duplicate shortcut {} for {} and {}",

+          shortcut1, action1->text(), action2->text());

+

+        break;

+      }

+    }

+  }

+}

+

 } // namespace MOShared

 

 

@@ -330,9 +388,9 @@ TimeThis::~TimeThis()
   const auto d = duration_cast<milliseconds>(end - m_start).count();

 

   if (m_what.isEmpty()) {

-    MOBase::log::debug("{} ms", d);

+    log::debug("{} ms", d);

   } else {

-    MOBase::log::debug("{} {} ms", m_what, d);

+    log::debug("{} {} ms", m_what, d);

   }

 }

 

@@ -358,7 +416,7 @@ bool ExitModOrganizer(ExitFlags e)
   }

 

   g_exiting = true;

-  MOBase::Guard g([&]{ g_exiting = false; });

+  Guard g([&]{ g_exiting = false; });

 

   if (!e.testFlag(Exit::Force)) {

     if (auto* mw=findMainWindow()) {

diff --git a/src/shared/util.h b/src/shared/util.h
index 79cadf71..05522c2d 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -48,6 +48,7 @@ MOBase::VersionInfo createVersionInfo();
 QString getUsvfsVersionString();

 

 void SetThisThreadName(const QString& s);

+void checkDuplicateShortcuts(const QMenu& m);

 

 } // namespace MOShared