diff --git a/src/library/browse/browsefeature.cpp b/src/library/browse/browsefeature.cpp index 56ca30aa1a82..180c67008d0b 100644 --- a/src/library/browse/browsefeature.cpp +++ b/src/library/browse/browsefeature.cpp @@ -19,8 +19,21 @@ namespace { +const QString kViewName = QStringLiteral("BROWSEHOME"); + const QString kQuickLinksSeparator = QStringLiteral("-+-"); +#if defined(__LINUX__) +const QStringList removableDriveRootPaths() { + QStringList paths; + const QString user = QString::fromLocal8Bit(qgetenv("USER")); + paths.append("/media"); + paths.append(QStringLiteral("/media/") + user); + paths.append(QStringLiteral("/run/media/") + user); + return paths; +} +#endif + } // anonymous namespace BrowseFeature::BrowseFeature( @@ -79,7 +92,7 @@ BrowseFeature::BrowseFeature( // show drive letters QFileInfoList drives = QDir::drives(); // show drive letters - foreach (QFileInfo drive, drives) { + for (const QFileInfo& drive : std::as_const(drives)) { // Using drive.filePath() to get path to display instead of drive.canonicalPath() // as it delay the startup too much if there is a network share mounted // (drive letter assigned) but unavailable @@ -123,7 +136,7 @@ BrowseFeature::BrowseFeature( loadQuickLinks(); - foreach (QString quickLinkPath, m_quickLinkList) { + for (const QString& quickLinkPath : std::as_const(m_quickLinkList)) { QString name = extractNameFromPath(quickLinkPath); qDebug() << "Appending Quick Link: " << name << "---" << quickLinkPath; m_pQuickLinkItem->appendChild(name, quickLinkPath); @@ -222,7 +235,7 @@ void BrowseFeature::bindLibraryWidget(WLibrary* libraryWidget, Q_UNUSED(keyboard); WLibraryTextBrowser* edit = new WLibraryTextBrowser(libraryWidget); edit->setHtml(getRootViewHtml()); - libraryWidget->registerView("BROWSEHOME", edit); + libraryWidget->registerView(kViewName, edit); } void BrowseFeature::bindSidebarWidget(WLibrarySidebar* pSidebarWidget) { @@ -231,7 +244,7 @@ void BrowseFeature::bindSidebarWidget(WLibrarySidebar* pSidebarWidget) { } void BrowseFeature::activate() { - emit switchToView("BROWSEHOME"); + emit switchToView(kViewName); emit disableSearch(); emit enableCoverArtDisplay(false); } @@ -246,6 +259,7 @@ void BrowseFeature::activateChild(const QModelIndex& index) { QString path = item->getData().toString(); if (path == QUICK_LINK_NODE || path == DEVICE_NODE) { emit saveModelState(); + // Clear the tracks view m_browseModel.setPath({}); } else { // Open a security token for this path and if we do not have access, ask @@ -274,10 +288,6 @@ void BrowseFeature::activateChild(const QModelIndex& index) { void BrowseFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex& index) { TreeItem *item = static_cast(index.internalPointer()); - - // Make sure that this is reset when the related TreeItem is deleted. - m_pLastRightClickedItem = item; - if (!item) { return; } @@ -288,7 +298,12 @@ void BrowseFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex return; } + // Make sure that this is reset when TreeItems are deleted in onLazyChildExpandation() + m_pLastRightClickedItem = item; + QMenu menu(m_pSidebarWidget); + + // If this a QuickLink show only the Remove action if (item->parent()->getData().toString() == QUICK_LINK_NODE) { menu.addAction(m_pRemoveQuickLinkAction); menu.exec(globalPos); @@ -296,10 +311,9 @@ void BrowseFeature::onRightClickChild(const QPoint& globalPos, const QModelIndex return; } + // If path is in the QuickLinks list show only the Remove action foreach (const QString& str, m_quickLinkList) { if (str == path) { - // if path is a QuickLink: - // show remove action menu.addAction(m_pRemoveQuickLinkAction); menu.exec(globalPos); onLazyChildExpandation(index); @@ -319,9 +333,9 @@ std::vector> createRemovableDevices() { std::vector> ret; #if defined(__WINDOWS__) // Repopulate drive list - QFileInfoList drives = QDir::drives(); + const QFileInfoList drives = QDir::drives(); // show drive letters - foreach (QFileInfo drive, drives) { + for (const QFileInfo& drive : std::as_const(drives)) { // Using drive.filePath() instead of drive.canonicalPath() as it // freezes interface too much if there is a network share mounted // (drive letter assigned) but unavailable @@ -339,21 +353,19 @@ std::vector> createRemovableDevices() { drive.filePath())); // Displays C:/ } #elif defined(__LINUX__) - // To get devices on Linux, we look for directories under /media and - // /run/media/$USER. QFileInfoList devices; - - // Add folders under /media to devices. - devices += QDir("/media").entryInfoList( - QDir::AllDirs | QDir::NoDotAndDotDot); - - // Add folders under /run/media/$USER to devices. - QDir run_media_user_dir(QStringLiteral("/run/media/") + QString::fromLocal8Bit(qgetenv("USER"))); - devices += run_media_user_dir.entryInfoList( - QDir::AllDirs | QDir::NoDotAndDotDot); + for (const QString& path : removableDriveRootPaths()) { + devices += QDir(path).entryInfoList(QDir::AllDirs | QDir::NoDotAndDotDot); + } // Convert devices into a QList for display. - foreach(QFileInfo device, devices) { + for (const QFileInfo& device : std::as_const(devices)) { + // On Linux, devices can be mounted in /media and /media/user and /run/media/[user] + // but there's no benefit of displaying the [user] dir in Devices. + // Show its children but skip the dir itself. + if (removableDriveRootPaths().contains(device.absoluteFilePath())) { + continue; + } ret.push_back(std::make_unique( device.fileName(), QVariant(device.filePath() + QStringLiteral("/")))); @@ -402,6 +414,13 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) { // If we are on the special device node if (path == DEVICE_NODE) { +#if defined(__LINUX__) + // Tell the model to remove the cached 'hasChildren' states of all sub- + // directories when we expand the Device node. + // This ensures we show the real dir tree. This is relevant when devices + // were unmounted, changed and mounted again. + m_pSidebarModel->removeChildDirsFromCache(removableDriveRootPaths()); +#endif folders = createRemovableDevices(); } else { // we assume that the path refers to a folder in the file system diff --git a/src/library/browse/foldertreemodel.cpp b/src/library/browse/foldertreemodel.cpp index 3f56a74c3bed..e2cf3a8cd3dd 100644 --- a/src/library/browse/foldertreemodel.cpp +++ b/src/library/browse/foldertreemodel.cpp @@ -20,20 +20,18 @@ FolderTreeModel::FolderTreeModel(QObject *parent) FolderTreeModel::~FolderTreeModel() { } -/* A tree model of the filesystem should be initialized lazy. - * It will take the universe to iterate over all files over filesystem - * hasChildren() returns true if a folder has subfolders although - * we do not know the precise number of subfolders. - * - * Note that BrowseFeature inserts folder trees dynamically and rowCount() - * is only called if necessary. - */ +// A tree model of the filesystem should be initialized lazy. +// It will take the universe to iterate over all files over filesystem +// hasChildren() returns true if a folder has subfolders although +// we do not know the precise number of subfolders. +// +// Note that BrowseFeature inserts folder trees dynamically and rowCount() +// is only called if necessary. bool FolderTreeModel::hasChildren(const QModelIndex& parent) const { TreeItem *item = static_cast(parent.internalPointer()); - /* Usually the child count is 0 because we do lazy initialization - * However, for, buid-in items such as 'Quick Links' there exist - * child items at init time - */ + // Usually the child count is 0 because we do lazy initialization + // However, for, buid-in items such as 'Quick Links' there exist + // child items at init time if (item->getData().toString() == QUICK_LINK_NODE) { return true; } @@ -56,18 +54,15 @@ bool FolderTreeModel::directoryHasChildren(const QString& path) const { // Acquire a security token for the path. const auto dirAccess = mixxx::FileAccess(mixxx::FileInfo(path)); - /* - * The following code is too expensive, general and SLOW since - * QDIR::EntryInfoList returns a full QFileInfolist - * - * - * QDir dir(item->getData().toString()); - * QFileInfoList all = dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot); - * return (all.count() > 0); - * - * We can benefit from low-level filesystem APIs, i.e., - * Windows API or SystemCalls - */ + // The following code is too expensive, general and SLOW since + // QDIR::EntryInfoList returns a full QFileInfolist + // + // QDir dir(item->getData().toString()); + // QFileInfoList all = dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot); + // return (all.count() > 0); + // + // We can benefit from low-level filesystem APIs, i.e., + // Windows API or SystemCalls bool has_children = false; @@ -116,3 +111,38 @@ bool FolderTreeModel::directoryHasChildren(const QString& path) const { m_directoryCache[path] = has_children; return has_children; } + +void FolderTreeModel::removeChildDirsFromCache(const QStringList& rootPaths) { + // PerformanceTimer time; + // const auto start = time.elapsed(); + if (rootPaths.isEmpty()) { + return; + } + + // Just a quick check that prevents iterating the cache pointlessly + for (const auto& rootPath : rootPaths) { + VERIFY_OR_DEBUG_ASSERT(!rootPath.isEmpty()) { + // List contains at least one non-empty path + break; + } + } + + // int checked = 0; + // int removed = 0; + QHashIterator it(m_directoryCache); + while (it.hasNext()) { + it.next(); + // checked++; + const QString cachedPath = it.key(); + for (const auto& rootPath : rootPaths) { + if (!rootPath.isEmpty() && cachedPath.startsWith(rootPath)) { + m_directoryCache.remove(cachedPath); + // removed++; + } + } + } + + // qWarning() << " checked:" << checked << "| removed:" << removed; + // qWarning() << " elapsed:" << mixxx::Duration(time.elapsed() - + // start).debugMicrosWithUnit(); +} diff --git a/src/library/browse/foldertreemodel.h b/src/library/browse/foldertreemodel.h index ee6cbea35d5b..575e54ae46da 100644 --- a/src/library/browse/foldertreemodel.h +++ b/src/library/browse/foldertreemodel.h @@ -16,8 +16,13 @@ class FolderTreeModel : public TreeItemModel { virtual ~FolderTreeModel(); virtual bool hasChildren(const QModelIndex& parent = QModelIndex()) const; bool directoryHasChildren(const QString& path) const; + void removeChildDirsFromCache(const QStringList& rootPaths); private: - // Used for memoizing the results of directoryHasChildren + // Used for memorizing the results of directoryHasChildren. + // Note: this means we won't see directory tree changes after the initial + // tree population after first expansion. I.e. newly added directories won't + // be displayed and removed dirs will remain in the sidebar tree. + // removeChildDirsFromCache() can be used to reset selected directories. mutable QHash m_directoryCache; }; diff --git a/src/library/treeitemmodel.cpp b/src/library/treeitemmodel.cpp index bf056ee859fe..d2b06230ebd8 100644 --- a/src/library/treeitemmodel.cpp +++ b/src/library/treeitemmodel.cpp @@ -3,30 +3,29 @@ #include "library/treeitem.h" #include "moc_treeitemmodel.cpp" -/* - * Just a word about how the TreeItem objects and TreeItemModels are used in general: - * TreeItems are used by the TreeItemModel class to display tree - * structures in the sidebar. - * - * The constructor has 4 arguments: - * 1. argument represents a name shown in the sidebar view later on - * 2. argument represents the absolute path of this tree item - * 3. argument is a library feature object. - * This is necessary because in sidebar.cpp we handle 'activateChid' events - * 4. the parent TreeItem object - * The constructor does not add this TreeItem object to the parent's child list - * - * In case of no arguments, the standard constructor creates a - * root item that is not visible in the sidebar. - * - * Once the TreeItem objects are inserted to models, the models take care of their - * deletion. - * - * Examples on how to use TreeItem and TreeItemModels can be found in - * - playlistfeature.cpp - * - cratefeature.cpp - * - *feature.cpp - */ +// Just a word about how the TreeItem objects and TreeItemModels are used in general: +// TreeItems are used by the TreeItemModel class to display tree +// structures in the sidebar. +// +// The constructor has 4 arguments: +// 1. argument represents a name shown in the sidebar view later on +// 2. argument represents the absolute path of this tree item +// 3. argument is a library feature object. +// This is necessary because in sidebar.cpp we handle 'activateChid' events +// 4. the parent TreeItem object +// The constructor does not add this TreeItem object to the parent's child list +// +// In case of no arguments, the standard constructor creates a +// root item that is not visible in the sidebar. +// +// Once the TreeItem objects are inserted to models, the models take care of their +// deletion. +// +// Examples on how to use TreeItem and TreeItemModels can be found in +// - playlistfeature.cpp +// - cratefeature.cpp +// - *feature.cpp + TreeItemModel::TreeItemModel(QObject *parent) : QAbstractItemModel(parent), m_pRootItem(std::make_unique()) { @@ -35,7 +34,7 @@ TreeItemModel::TreeItemModel(QObject *parent) TreeItemModel::~TreeItemModel() { } -//Our Treeview Model supports exactly a single column +// Our Treeview Model supports exactly a single column int TreeItemModel::columnCount(const QModelIndex &parent) const { Q_UNUSED(parent); return 1; @@ -153,10 +152,8 @@ int TreeItemModel::rowCount(const QModelIndex& parent) const { return parentItem->childRows(); } -/** - * Populates the model and notifies the view. - * Call this method first, before you do call any other methods. - */ +// Populates the model and notifies the view. +// Call this method first, before you do call any other methods. TreeItem* TreeItemModel::setRootItem(std::unique_ptr pRootItem) { beginResetModel(); m_pRootItem = std::move(pRootItem); @@ -168,10 +165,8 @@ const QModelIndex TreeItemModel::getRootIndex() { return createIndex(0, 0, getRootItem()); } -/** - * Before you can resize the data model dynamically by using 'insertRows' and 'removeRows' - * make sure you have initialized. - */ +// Before you can resize the data model dynamically by using 'insertRows' and 'removeRows' +// make sure you have initialized. void TreeItemModel::insertTreeItemRows( std::vector>&& rows, int position,