From c7a7a046ba27fa8dd52a1b52efeee92681b44846 Mon Sep 17 00:00:00 2001 From: Modspike Date: Fri, 19 Dec 2025 11:48:06 +0100 Subject: [PATCH 1/6] implemented context menu in folders view implemented the menu - the actions need more testing but so far it seems to work fine also fixed a bug in folderman related to incorrect unsynced count when user adds folder --- .../FoldersGui/accountfolderscontroller.cpp | 21 ++++++++++++-- src/gui/FoldersGui/accountfolderscontroller.h | 3 +- src/gui/FoldersGui/accountfoldersview.cpp | 29 +++++++++++++++++-- src/gui/FoldersGui/accountfoldersview.h | 8 ++++- src/gui/FoldersGui/foldermodelcontroller.h | 1 + src/gui/folderman.cpp | 1 + src/gui/settingsdialog.h | 2 +- 7 files changed, 58 insertions(+), 7 deletions(-) diff --git a/src/gui/FoldersGui/accountfolderscontroller.cpp b/src/gui/FoldersGui/accountfolderscontroller.cpp index 3fa347c5364..16191e8744f 100644 --- a/src/gui/FoldersGui/accountfolderscontroller.cpp +++ b/src/gui/FoldersGui/accountfolderscontroller.cpp @@ -60,7 +60,9 @@ AccountFoldersController::AccountFoldersController(AccountState *state, AccountF _sortModel = weightedModel; */ - _view->setItemModel(model); + _view->setItemModels(model, modelController->selectionModel()); + + buildMenuActions(); connect(_view, &AccountFoldersView::addFolderTriggered, this, &AccountFoldersController::slotAddFolder); // ui->quickWidget->engine()->addImageProvider(QStringLiteral("space"), new Spaces::SpaceImageProvider(_accountState->account()->spacesManager())); @@ -123,28 +125,40 @@ void AccountFoldersController::slotFolderWizardAccepted(OCC::FolderMan::SyncConn void AccountFoldersController::buildMenuActions() { + QList itemActions; + // show in finder _showInSystemFolder = new QAction(CommonStrings::showInFileBrowser(), this); + itemActions.push_back(_showInSystemFolder); connect(_showInSystemFolder, &QAction::triggered, this, &AccountFoldersController::onShowInSystemFolder); if (_accountState->account()->capabilities().privateLinkPropertyAvailable()) { _showInBrowser = new QAction(CommonStrings::showInWebBrowser(), this); + itemActions.push_back(_showInBrowser); connect(_showInBrowser, &QAction::triggered, this, &AccountFoldersController::onShowInBrowser); } + QAction *separator = new QAction(this); + separator->setSeparator(true); + itemActions.push_back(separator); + _forceSync = new QAction(tr("Force sync now"), this); + itemActions.push_back(_forceSync); connect(_forceSync, &QAction::triggered, this, &AccountFoldersController::onForceSync); _pauseSync = new QAction(this); + itemActions.push_back(_pauseSync); connect(_pauseSync, &QAction::triggered, this, &AccountFoldersController::onTogglePauseSync); _removeSync = new QAction(tr("Remove folder sync connection"), this); + itemActions.push_back(_removeSync); connect(_removeSync, &QAction::triggered, this, &AccountFoldersController::onRemoveSync); // we may want to treat this similar to the enable vfs option, because choose sync can only be activated // if the folder is not using vfs. So the idea is if force vfs is on the user will never be able to choose // so showing the option at all is probably not great _chooseSync = new QAction(tr("Choose what to sync"), this); + itemActions.push_back(_chooseSync); connect(_chooseSync, &QAction::triggered, this, &AccountFoldersController::onChooseSync); @@ -176,6 +190,9 @@ void AccountFoldersController::buildMenuActions() } _enableVfs = new QAction(this); */ + + connect(_view, &AccountFoldersView::requestActionsUpdate, this, &AccountFoldersController::updateActions); + _view->setFolderActions(itemActions); } @@ -337,7 +354,7 @@ void AccountFoldersController::buildMenuActions() void AccountFoldersController::updateActions() { - if (!_currentFolder) + if (!_currentFolder) // probably disable all actions here return; _showInSystemFolder->setEnabled(QFileInfo::exists(_currentFolder->path())); diff --git a/src/gui/FoldersGui/accountfolderscontroller.h b/src/gui/FoldersGui/accountfolderscontroller.h index 3c29f5b2c21..12d30ebc108 100644 --- a/src/gui/FoldersGui/accountfolderscontroller.h +++ b/src/gui/FoldersGui/accountfolderscontroller.h @@ -54,7 +54,9 @@ protected slots: QPointer _currentFolder = nullptr; AccountFoldersView *_view = nullptr; FolderModelController *_modelController = nullptr; + void buildMenuActions(); + void updateActions(); // menu actions: QAction *_showInSystemFolder = nullptr; @@ -73,6 +75,5 @@ protected slots: void onTogglePauseSync(); void onRemoveSync(); void onChooseSync(); - void updateActions(); }; } diff --git a/src/gui/FoldersGui/accountfoldersview.cpp b/src/gui/FoldersGui/accountfoldersview.cpp index 3735637396c..5ac49a222ba 100644 --- a/src/gui/FoldersGui/accountfoldersview.cpp +++ b/src/gui/FoldersGui/accountfoldersview.cpp @@ -15,6 +15,7 @@ #include #include +#include #include #include #include @@ -29,6 +30,7 @@ AccountFoldersView::AccountFoldersView(QWidget *parent) : QWidget{parent} { buildView(); + _itemMenu = new QMenu(this); } void AccountFoldersView::buildView() @@ -56,7 +58,10 @@ void AccountFoldersView::buildView() // I'm not sure always off is good but that's what we currently have _treeView->setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); _treeView->setVerticalScrollBarPolicy(Qt::ScrollBarAsNeeded); - _treeView->setLineWidth(2); + + _treeView->setContextMenuPolicy(Qt::CustomContextMenu); + connect(_treeView, &QWidget::customContextMenuRequested, this, &AccountFoldersView::popItemMenu); + mainLayout->addWidget(_treeView); @@ -67,9 +72,14 @@ void AccountFoldersView::buildView() setLayout(mainLayout); } -void AccountFoldersView::setItemModel(QStandardItemModel *model) +void AccountFoldersView::setItemModels(QStandardItemModel *model, QItemSelectionModel *selectionModel) { + // order here is important, if you set the selection model before the main model the selection model for the view + // reverts to the "default" selectionModel. + // also the tree view doesn't manage the lifetime of the selection model so we could/possibly should delete the default here + // I'll check that out _treeView->setModel(model); + _treeView->setSelectionModel(selectionModel); } void AccountFoldersView::setSyncedFolderCount(int synced, int total) @@ -81,4 +91,19 @@ void AccountFoldersView::enableAddFolder(bool enableAdd) { _addFolderButton->setEnabled(enableAdd); } + +void AccountFoldersView::setFolderActions(QList actions) +{ + if (_itemMenu) { + _itemMenu->clear(); + _itemMenu->addActions(actions); + } +} + +void AccountFoldersView::popItemMenu(const QPoint &pos) +{ + emit requestActionsUpdate(); + + _itemMenu->exec(_treeView->viewport()->mapToGlobal(pos)); +} } diff --git a/src/gui/FoldersGui/accountfoldersview.h b/src/gui/FoldersGui/accountfoldersview.h index 0b8f7fddd41..5bcfbbb7b65 100644 --- a/src/gui/FoldersGui/accountfoldersview.h +++ b/src/gui/FoldersGui/accountfoldersview.h @@ -16,9 +16,11 @@ #include class QStandardItemModel; +class QItemSelectionModel; class QTreeView; class QLabel; class QPushButton; +class QMenu; namespace OCC { @@ -28,19 +30,23 @@ class AccountFoldersView : public QWidget public: explicit AccountFoldersView(QWidget *parent = nullptr); - void setItemModel(QStandardItemModel *model); + void setItemModels(QStandardItemModel *model, QItemSelectionModel *selectionModel); void setFolderActions(QList actions); void setSyncedFolderCount(int synced, int total); void enableAddFolder(bool enableAdd); + void setMenuActions(QList actions); signals: void addFolderTriggered(); + void requestActionsUpdate(); private: void buildView(); + void popItemMenu(const QPoint &pos); QTreeView *_treeView = nullptr; QLabel *_syncedFolderCountLabel = nullptr; QPushButton *_addFolderButton = nullptr; + QMenu *_itemMenu = nullptr; }; } diff --git a/src/gui/FoldersGui/foldermodelcontroller.h b/src/gui/FoldersGui/foldermodelcontroller.h index 8fa23bd09f5..8f697bc8e0a 100644 --- a/src/gui/FoldersGui/foldermodelcontroller.h +++ b/src/gui/FoldersGui/foldermodelcontroller.h @@ -39,6 +39,7 @@ class FolderModelController : public QObject explicit FolderModelController(const QUuid &accountId, QObject *parent); QStandardItemModel *itemModel() const { return _model; } + QItemSelectionModel *selectionModel() const { return _selectionModel; } // getting creative to allow connecting to folderman via dependency injection. its a bit weird but let's see how it is // note we don't want to pass folderman to the ctr as we only need to connect to it, don't want it to be a member, diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 7508a0aab5f..7a78b256f6b 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -1188,6 +1188,7 @@ void FolderMan::addFolderFromGui(AccountState *accountState, const SyncConnectio if (_unsyncedSpaces.contains(accountId) && _unsyncedSpaces[accountId].contains(f->spaceId())) { + _unsyncedSpaces[accountId].remove(f->spaceId()); emit unsyncedSpaceCountChanged(accountId, _unsyncedSpaces[accountId].count(), accountState->account()->spacesManager()->spacesCount()); } diff --git a/src/gui/settingsdialog.h b/src/gui/settingsdialog.h index 3c2a8b47f7a..ddfc13476e3 100644 --- a/src/gui/settingsdialog.h +++ b/src/gui/settingsdialog.h @@ -22,7 +22,7 @@ #include #include -// #define USE_NEW_FOLDER_LIST +#define USE_NEW_FOLDER_LIST namespace OCC { From 7cf1ace06eac91a559e422771b4e13d664c2a724 Mon Sep 17 00:00:00 2001 From: Modspike Date: Fri, 19 Dec 2025 11:52:05 +0100 Subject: [PATCH 2/6] fix the squish tests --- src/gui/settingsdialog.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gui/settingsdialog.h b/src/gui/settingsdialog.h index ddfc13476e3..3c2a8b47f7a 100644 --- a/src/gui/settingsdialog.h +++ b/src/gui/settingsdialog.h @@ -22,7 +22,7 @@ #include #include -#define USE_NEW_FOLDER_LIST +// #define USE_NEW_FOLDER_LIST namespace OCC { From 9c69d15e4c2625f2800f44aea885ca0e905b2e82 Mon Sep 17 00:00:00 2001 From: Modspike Date: Fri, 9 Jan 2026 09:14:27 +0100 Subject: [PATCH 3/6] small bug fixes --- src/gui/FoldersGui/accountfolderscontroller.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gui/FoldersGui/accountfolderscontroller.cpp b/src/gui/FoldersGui/accountfolderscontroller.cpp index 16191e8744f..96ebb88803f 100644 --- a/src/gui/FoldersGui/accountfolderscontroller.cpp +++ b/src/gui/FoldersGui/accountfolderscontroller.cpp @@ -369,7 +369,7 @@ void AccountFoldersController::updateActions() // they made a mistake when adding the folder or whatever _pauseSync->setEnabled(_currentFolder->isAvailable()); - _chooseSync->setEnabled(!_currentFolder->virtualFilesEnabled()); + _chooseSync->setEnabled(_currentFolder->isAvailable() && !_currentFolder->virtualFilesEnabled()); } void OCC::AccountFoldersController::onShowInSystemFolder() @@ -386,7 +386,7 @@ void AccountFoldersController::onShowInBrowser() if (!_currentFolder) return; - QString path = _currentFolder->path(); + QString path = _currentFolder->remotePathTrailingSlash(); QUrl davUrl = _currentFolder->webDavUrl(); fetchPrivateLinkUrl(_accountState->account(), davUrl, path, this, [](const QUrl &url) { Utility::openBrowser(url, nullptr); }); } @@ -460,7 +460,7 @@ void AccountFoldersController::onRemoveSync() void AccountFoldersController::onChooseSync() { - if (!_currentFolder || _accountState || !_accountState->account()) { + if (!_currentFolder || !_accountState || !_accountState->account()) { return; } From 478629e81a0307c2c214d40998406ce8eac9e82b Mon Sep 17 00:00:00 2001 From: Modspike Date: Fri, 9 Jan 2026 16:20:24 +0100 Subject: [PATCH 4/6] more tweaks a few more tweaks and cleanup started adding some handling for the enable/disable vfs action. this has led to some extra refactoring so I want to commit this controller related stuff as a separate step - it may not build but better than munging all of this stuff into one commit I think. --- .../FoldersGui/accountfolderscontroller.cpp | 163 +++++------------- src/gui/FoldersGui/accountfolderscontroller.h | 2 + src/gui/FoldersGui/accountfoldersview.cpp | 1 + 3 files changed, 46 insertions(+), 120 deletions(-) diff --git a/src/gui/FoldersGui/accountfolderscontroller.cpp b/src/gui/FoldersGui/accountfolderscontroller.cpp index 96ebb88803f..cc75bcee91d 100644 --- a/src/gui/FoldersGui/accountfolderscontroller.cpp +++ b/src/gui/FoldersGui/accountfolderscontroller.cpp @@ -198,82 +198,6 @@ void AccountFoldersController::buildMenuActions() /*void AccountView::slotCustomContextMenuRequested(Folder *folder) { - // Refactoring todo: we need to eval defensive handling of the account state QPointer in more depth, and I - // am not able to easily determine what should happen in this handler if the state is null. For now we just assert - // to make the "source" of the nullptr obvious before it trickles down into sub-areas and causes a crash that's harder - // to id - Q_ASSERT(_accountState && _accountState->account()); - - // qpointer for async calls - const auto isDeployed = folder->isDeployed(); - const auto addRemoveFolderAction = [isDeployed, folder, this](QMenu *menu) { - Q_ASSERT(!isDeployed); - return menu->addAction(tr("Remove folder sync connection"), this, [folder, this] { slotRemoveCurrentFolder(folder); }); - }; - - - auto *menu = new QMenu(ui->accountFoldersView); - menu->setAccessibleName(tr("Sync options menu")); - menu->setAttribute(Qt::WA_DeleteOnClose); - connect(folder, &OCC::Folder::destroyed, menu, &QMenu::close); - // Only allow removal if the item isn't in "ready" state. - if (!folder->isReady() && !isDeployed) { - if (Theme::instance()->syncNewlyDiscoveredSpaces()) { - menu->addAction(tr("Folder is not ready yet"))->setEnabled(false); - } else { - addRemoveFolderAction(menu); - } - menu->popup(QCursor::pos()); - // accessibility - menu->setFocus(); - return; - } - // Add an action to open the folder in the system's file browser: - QAction *showInFileManagerAction = menu->addAction(CommonStrings::showInFileBrowser(), [folder]() { - qCInfo(lcAccountView) << "Opening local folder" << folder->path(); - if (QFileInfo::exists(folder->path())) { - showInFileManager(folder->path()); - } - }); - - if (!QFile::exists(folder->path())) { - showInFileManagerAction->setEnabled(false); - } - - // Add an action to open the folder on the server in a webbrowser: - // Refactoring todo: why are we using the folder accountState AND the local member? shouldn't the folder have the same account state - // as this settings panel?! - if (folder->accountState()->account()->capabilities().privateLinkPropertyAvailable()) { - QString path = folder->remotePathTrailingSlash(); - menu->addAction(CommonStrings::showInWebBrowser(), [path, davUrl = folder->webDavUrl(), this] { - fetchPrivateLinkUrl(_accountState->account(), davUrl, path, this, [](const QUrl &url) { Utility::openBrowser(url, nullptr); }); - }); - } - - - // Root-folder specific actions: - menu->addSeparator(); - - // qpointer for the async context menu - if (OC_ENSURE(folder->isReady())) { - const bool folderPaused = folder->syncPaused(); - - if (!folderPaused) { - QAction *forceSyncAction = menu->addAction(tr("Force sync now")); - if (folder->isSyncRunning()) { - forceSyncAction->setText(tr("Restart sync")); - } - forceSyncAction->setEnabled(folder->accountState()->isConnected()); - connect(forceSyncAction, &QAction::triggered, this, [folder, this] { slotForceSyncCurrentFolder(folder); }); - } - - QAction *resumeAction = menu->addAction(folderPaused ? tr("Resume sync") : tr("Pause sync")); - connect(resumeAction, &QAction::triggered, this, [folder, this] { slotEnableCurrentFolder(folder, true); }); - - if (!isDeployed) { - if (!Theme::instance()->syncNewlyDiscoveredSpaces()) { - addRemoveFolderAction(menu); - } auto maybeShowEnableVfs = [folder, menu, this]() { // Only show "Enable VFS" if a VFS mode is available @@ -300,34 +224,28 @@ void AccountFoldersController::buildMenuActions() } } } - if (!folder->virtualFilesEnabled()) { - menu->addAction(tr("Choose what to sync"), this, [folder, this] { showSelectiveSyncDialog(folder); }); - } - menu->popup(QCursor::pos()); - menu->setFocus(); // for accassebility (keyboard navigation) - } else { - menu->deleteLater(); - } + } -} +}*/ -/*void AccountFoldersController::slotEnableVfsCurrentFolder(Folder *folder) +void AccountFoldersController::onEnableVfs() { - if (OC_ENSURE(VfsPluginManager::instance().bestAvailableVfsMode() == Vfs::WindowsCfApi)) { - if (!folder) { - return; - } - qCInfo(lcAccountView) << "Enabling vfs support for folder" << folder->path(); + if (!_currentFolder || _currentFolder->virtualFilesEnabled() || VfsPluginManager::instance().bestAvailableVfsMode() != Vfs::WindowsCfApi) + return; - // Change the folder vfs mode and load the plugin - folder->setVirtualFilesEnabled(true); - } -}*/ + qCInfo(lcAccountView) << "Enabling vfs support for folder" << folder->path(); + + // Change the folder vfs mode and load the plugin + folder->setVirtualFilesEnabled(true); +} -/*void AccountFoldersController::slotDisableVfsCurrentFolder(Folder *folder) +void AccountFoldersController::onDisableVfs() { - auto msgBox = new QMessageBox(QMessageBox::Question, tr("Disable virtual file support?"), + if (!_currentFolder) + return; + + QMessageBox msgBox(QMessageBox::Question, tr("Disable virtual file support?"), tr("This action will disable virtual file support. As a consequence contents of folders that " "are currently marked as 'available online only' will be downloaded." "\n\n" @@ -335,41 +253,47 @@ void AccountFoldersController::buildMenuActions() "will become available again." "\n\n" "This action will abort any currently running synchronization.")); - auto acceptButton = msgBox->addButton(tr("Disable support"), QMessageBox::AcceptRole); - msgBox->addButton(tr("Cancel"), QMessageBox::RejectRole); - connect(msgBox, &QMessageBox::finished, msgBox, [msgBox, folder, acceptButton] { - msgBox->deleteLater(); - if (msgBox->clickedButton() != acceptButton || !folder) { - return; - } + msgBox.button(QMessageBox::Yes)->setText(tr("Disable support")); + msgBox.button(QMessageBox::No)->setText(tr("Cancel")); - qCInfo(lcAccountView) << "Disabling vfs support for folder" << folder->path(); + int result = msgBox.exec(); + if (result == QDialog::Rejected) + return; - // Also wipes virtual files, schedules remote discovery - folder->setVirtualFilesEnabled(false); - }); - msgBox->open(); -}*/ + qCInfo(lcAccountFoldersController) << "Disabling vfs support for folder" << _currentFolder->path(); + + // Also wipes virtual files, schedules remote discovery + _currentFolder->setVirtualFilesEnabled(false); +} void AccountFoldersController::updateActions() { - if (!_currentFolder) // probably disable all actions here - return; - - _showInSystemFolder->setEnabled(QFileInfo::exists(_currentFolder->path())); + // the checks for _currentFolder are twofold: + // obvs we don't want to try to access values that aren't there + // furthermore, we do need to disable if the current folder selection + // is empty so it's actually a key part of the evaluation for action state here + _showInSystemFolder->setEnabled(_currentFolder && QFileInfo::exists(_currentFolder->path())); if (_showInBrowser) - _showInBrowser->setEnabled(_currentFolder->isAvailable()); + _showInBrowser->setEnabled(_currentFolder && _currentFolder->isAvailable()); - _forceSync->setEnabled(!_currentFolder->isSyncRunning() && _currentFolder->canSync()); + _forceSync->setEnabled(_currentFolder && _currentFolder->canSync()); + _forceSync->setText(_currentFolder && _currentFolder->isSyncRunning() ? tr("Restart sync") : tr("Force sync now")); - _pauseSync->setText(_currentFolder->syncPaused() ? tr("Resume sync") : tr("Pause sync")); + _pauseSync->setText(_currentFolder && _currentFolder->syncPaused() ? tr("Resume sync") : tr("Pause sync")); // is this enough? I think we need to keep it enabled even if sync is running so the user can effectively cancel it + pause. as who knows. maybe // they made a mistake when adding the folder or whatever - _pauseSync->setEnabled(_currentFolder->isAvailable()); + _pauseSync->setEnabled(_currentFolder && _currentFolder->isAvailable()); + + _removeSync->setEnabled(_currentFolder); + + if (_toggleVfs) { + _toggleVfs->setText(_currentFolder && _currentFolder->virtualFilesEnabled() ? tr("Disable virtual file support") : tr("Enable virtual file support"); + _toggleVfs->setEnabled(); + } - _chooseSync->setEnabled(_currentFolder->isAvailable() && !_currentFolder->virtualFilesEnabled()); + _chooseSync->setEnabled(_currentFolder && _currentFolder->isAvailable() && !_currentFolder->virtualFilesEnabled()); } void OCC::AccountFoldersController::onShowInSystemFolder() @@ -401,7 +325,6 @@ void AccountFoldersController::onForceSync() tr("Synchronization is paused because the Internet connection is a metered connection" "

Do you really want to force a Synchronization now?"), QMessageBox::Yes | QMessageBox::No, ocApp()->gui()->settingsDialog()); - messageBox.setAttribute(Qt::WA_DeleteOnClose); int result = messageBox.exec(); if (result == QDialog::Rejected) return; diff --git a/src/gui/FoldersGui/accountfolderscontroller.h b/src/gui/FoldersGui/accountfolderscontroller.h index 12d30ebc108..57d79d852f4 100644 --- a/src/gui/FoldersGui/accountfolderscontroller.h +++ b/src/gui/FoldersGui/accountfolderscontroller.h @@ -75,5 +75,7 @@ protected slots: void onTogglePauseSync(); void onRemoveSync(); void onChooseSync(); + void onDisableVfs(); + void onEnableVfs(); }; } diff --git a/src/gui/FoldersGui/accountfoldersview.cpp b/src/gui/FoldersGui/accountfoldersview.cpp index 5ac49a222ba..d1edd90a7a8 100644 --- a/src/gui/FoldersGui/accountfoldersview.cpp +++ b/src/gui/FoldersGui/accountfoldersview.cpp @@ -31,6 +31,7 @@ AccountFoldersView::AccountFoldersView(QWidget *parent) { buildView(); _itemMenu = new QMenu(this); + _itemMenu->setAccessibleName(tr("Sync options menu")); } void AccountFoldersView::buildView() From c1b87cb679f588c4cf7470cfa51ac476fd341d3d Mon Sep 17 00:00:00 2001 From: Modspike Date: Fri, 9 Jan 2026 17:20:07 +0100 Subject: [PATCH 5/6] refactor method of checking path for vfs support killed deadwood renamed Vfs::checkAvailability and changed return type to simple string updated callers and added copious todo's related to this topic of having local paths that don't support vfs even when vfs is otherwise available. See DC-219 for that --- src/common/vfs.cpp | 2 +- src/common/vfs.h | 12 +++- .../FoldersGui/accountfolderscontroller.cpp | 62 +++++++++---------- src/gui/accountsettings.cpp | 2 +- src/gui/folder.cpp | 11 +++- src/gui/folderman.h | 6 -- src/gui/folderstatusmodel.cpp | 4 -- src/gui/folderwizard/folderwizard.cpp | 17 +++-- .../folderwizard/folderwizardlocalpath.cpp | 4 ++ .../advancedsettingspagecontroller.cpp | 10 ++- 10 files changed, 75 insertions(+), 55 deletions(-) diff --git a/src/common/vfs.cpp b/src/common/vfs.cpp index e0e06f09664..87789becae6 100644 --- a/src/common/vfs.cpp +++ b/src/common/vfs.cpp @@ -68,7 +68,7 @@ QString Utility::enumToString(Vfs::Mode mode) } } -Result Vfs::checkAvailability(const QString &path, Vfs::Mode mode) +QString Vfs::pathSupportDetail(const QString &path, Vfs::Mode mode) { #ifdef Q_OS_WIN if (mode == Mode::WindowsCfApi) { diff --git a/src/common/vfs.h b/src/common/vfs.h index b5b88e90bb6..2932def4c23 100644 --- a/src/common/vfs.h +++ b/src/common/vfs.h @@ -125,7 +125,17 @@ class OCSYNC_EXPORT Vfs : public QObject static Mode modeFromString(const QString &str); - static Result checkAvailability(const QString &path, OCC::Vfs::Mode mode); + /** + * @brief pathSupportDetail - needs a better name but the concept is that this function checks to see if the path is supported for vfs. + * if it is not supported the return string contains the "reason" it's not supported. + * @param path to check to for support + * @param mode which we want to check - note that the only arg that makes sense here at all is Vfs::WindowsCfApi + * @return empty string if the mode is supported on the given path. If the string is not empty, it contains a string explaining the + * reason the mode is not supported. + * + * as of this writing, the possible root reasons are: path is a drive, path is on a network drive, path fs is not ntfs. + */ + static QString pathSupportDetail(const QString &path, OCC::Vfs::Mode mode); enum class AvailabilityError { diff --git a/src/gui/FoldersGui/accountfolderscontroller.cpp b/src/gui/FoldersGui/accountfolderscontroller.cpp index cc75bcee91d..b626f38bcfb 100644 --- a/src/gui/FoldersGui/accountfolderscontroller.cpp +++ b/src/gui/FoldersGui/accountfolderscontroller.cpp @@ -161,35 +161,35 @@ void AccountFoldersController::buildMenuActions() itemActions.push_back(_chooseSync); connect(_chooseSync, &QAction::triggered, this, &AccountFoldersController::onChooseSync); - - // Only show "Enable VFS" if a VFS mode is available - /* const auto mode = VfsPluginManager::instance().bestAvailableVfsMode(); - if (mode == Vfs::WindowsCfApi) { - _enableVfs = new QAction(tr("Enable virtual file support"), this); - - if (FolderMan::instance()->checkVfsAvailability(folder->path(), mode)) { - if (mode == Vfs::WindowsCfApi) { - QAction *enableVfsAction = menu->addAction(tr("Enable virtual file support")); - connect(enableVfsAction, &QAction::triggered, this, [folder, this] { slotEnableVfsCurrentFolder(folder); }); - } - } - }; - - if (Theme::instance()->showVirtualFilesOption()) { - if (Theme::instance()->forceVirtualFilesOption()) { - if (!folder->virtualFilesEnabled()) { - // VFS is currently disabled, but is forced on by theming (e.g. due to a theme change) - maybeShowEnableVfs(); + /* auto maybeShowEnableVfs = [folder, menu, this]() { + // Only show "Enable VFS" if a VFS mode is available + const auto mode = VfsPluginManager::instance().bestAvailableVfsMode(); + if (mode == Vfs::WindowsCfApi) { + _enableVfs = new QAction(tr("Enable virtual file support"), this); + + if (FolderMan::instance()->checkVfsAvailability(folder->path(), mode)) { + if (mode == Vfs::WindowsCfApi) { + QAction *enableVfsAction = menu->addAction(tr("Enable virtual file support")); + connect(enableVfsAction, &QAction::triggered, this, [folder, this] { slotEnableVfsCurrentFolder(folder); }); + } } - } else { - if (folder->virtualFilesEnabled()) { - menu->addAction(tr("Disable virtual file support"), this, [folder, this] { slotDisableVfsCurrentFolder(folder); }); + }; + + if (Theme::instance()->showVirtualFilesOption()) { + if (Theme::instance()->forceVirtualFilesOption()) { + if (!folder->virtualFilesEnabled()) { + // VFS is currently disabled, but is forced on by theming (e.g. due to a theme change) + maybeShowEnableVfs(); + } } else { - maybeShowEnableVfs(); + if (folder->virtualFilesEnabled()) { + menu->addAction(tr("Disable virtual file support"), this, [folder, this] { slotDisableVfsCurrentFolder(folder); }); + } else { + maybeShowEnableVfs(); + } } - } - _enableVfs = new QAction(this); - */ + _enableVfs = new QAction(this); + */ connect(_view, &AccountFoldersView::requestActionsUpdate, this, &AccountFoldersController::updateActions); _view->setFolderActions(itemActions); @@ -234,10 +234,10 @@ void AccountFoldersController::onEnableVfs() if (!_currentFolder || _currentFolder->virtualFilesEnabled() || VfsPluginManager::instance().bestAvailableVfsMode() != Vfs::WindowsCfApi) return; - qCInfo(lcAccountView) << "Enabling vfs support for folder" << folder->path(); + qCInfo(lcAccountFoldersController) << "Enabling vfs support for folder" << _currentFolder->path(); // Change the folder vfs mode and load the plugin - folder->setVirtualFilesEnabled(true); + _currentFolder->setVirtualFilesEnabled(true); } void AccountFoldersController::onDisableVfs() @@ -288,9 +288,9 @@ void AccountFoldersController::updateActions() _removeSync->setEnabled(_currentFolder); - if (_toggleVfs) { - _toggleVfs->setText(_currentFolder && _currentFolder->virtualFilesEnabled() ? tr("Disable virtual file support") : tr("Enable virtual file support"); - _toggleVfs->setEnabled(); + if (_enableVfs) { + _enableVfs->setText(_currentFolder && _currentFolder->virtualFilesEnabled() ? tr("Disable virtual file support") : tr("Enable virtual file support")); + _enableVfs->setEnabled(_currentFolder->canSync()); } _chooseSync->setEnabled(_currentFolder && _currentFolder->isAvailable() && !_currentFolder->virtualFilesEnabled()); diff --git a/src/gui/accountsettings.cpp b/src/gui/accountsettings.cpp index b81ce35e46c..bf581466fbd 100644 --- a/src/gui/accountsettings.cpp +++ b/src/gui/accountsettings.cpp @@ -226,7 +226,7 @@ void AccountSettings::slotCustomContextMenuRequested(Folder *folder) auto maybeShowEnableVfs = [folder, menu, this]() { // Only show "Enable VFS" if a VFS mode is available const auto mode = VfsPluginManager::instance().bestAvailableVfsMode(); - if (FolderMan::instance()->checkVfsAvailability(folder->path(), mode)) { + if (Vfs::pathSupportDetail(folder->path(), mode).isEmpty()) { if (mode == Vfs::WindowsCfApi) { QAction *enableVfsAction = menu->addAction(tr("Enable virtual file support")); connect(enableVfsAction, &QAction::triggered, this, [folder, this] { slotEnableVfsCurrentFolder(folder); }); diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 584912e1e8f..f6108fd3a9a 100644 --- a/src/gui/folder.cpp +++ b/src/gui/folder.cpp @@ -513,9 +513,14 @@ void Folder::startVfs() OC_ENFORCE(_vfs); OC_ENFORCE(_vfs->mode() == _definition.virtualFilesMode()); - const auto result = Vfs::checkAvailability(path(), _vfs->mode()); - if (!result) { - _syncResult.appendErrorString(result.error()); + // todo: DC-219 check to see if this sync error makes it to the gui or not. We need to more clearly inform the user that they should + // create a new sync connection to the remote folder, and use improved checks in the folder wizard to make sure vfs is supported + // on the chosen path. + // ideally we want a check like this before we even get here, eg in the area of load folder from config - but need to be sure there is + // a place to clearly instruct the user that the folder sync needs to be re-built with a valid local path + QString result = Vfs::pathSupportDetail(path(), _vfs->mode()); + if (!result.isEmpty()) { + _syncResult.appendErrorString(result); setSyncState(SyncResult::SetupError); return; } diff --git a/src/gui/folderman.h b/src/gui/folderman.h index 9012769ee9e..82c4560cb16 100644 --- a/src/gui/folderman.h +++ b/src/gui/folderman.h @@ -259,12 +259,6 @@ class OWNCLOUDGUI_EXPORT FolderMan : public QObject void setDirtyProxy(); - /** Whether or not vfs is supported in the location. */ - bool checkVfsAvailability(const QString &path, Vfs::Mode mode = VfsPluginManager::instance().bestAvailableVfsMode()) const; - - /** If the folder configuration is no longer supported this will return an error string */ - Result unsupportedConfiguration(const QString &path) const; - [[nodiscard]] bool isSpaceSynced(GraphApi::Space *space) const; /** diff --git a/src/gui/folderstatusmodel.cpp b/src/gui/folderstatusmodel.cpp index 48dd7708df3..6503d6eb490 100644 --- a/src/gui/folderstatusmodel.cpp +++ b/src/gui/folderstatusmodel.cpp @@ -212,10 +212,6 @@ QVariant FolderStatusModel::data(const QModelIndex &index, int role) const auto getErrors = [f] { auto errors = f->syncResult().errorStrings(); - const Result notLegacyError = FolderMan::instance()->unsupportedConfiguration(f->path()); - if (!notLegacyError) { - errors.append(notLegacyError.error()); - } if (f->syncResult().hasUnresolvedConflicts()) { errors.append(tr("There are unresolved conflicts.")); } diff --git a/src/gui/folderwizard/folderwizard.cpp b/src/gui/folderwizard/folderwizard.cpp index cda5df0e4ff..38f54cb6e33 100644 --- a/src/gui/folderwizard/folderwizard.cpp +++ b/src/gui/folderwizard/folderwizard.cpp @@ -102,7 +102,7 @@ FolderWizardPrivate::FolderWizardPrivate(FolderWizard *q, Account *account) QString FolderWizardPrivate::initialLocalPath() const { return FolderMan::instance()->findGoodPathForNewSyncFolder( - defaultSyncRoot(), _spacesPage->currentSpace()->displayName(), FolderMan::NewFolderType::SpacesSyncRoot, _account->uuid()); + defaultSyncRoot(), _spacesPage->currentSpace()->displayName(), FolderMan::NewFolderType::SpacesFolder, _account->uuid()); } uint32_t FolderWizardPrivate::priority() const @@ -131,12 +131,16 @@ QString FolderWizardPrivate::displayName() const bool FolderWizardPrivate::useVirtualFiles() const { + // todo: see other todo's related to DC-219. Basically we should simply not allow user to pick a local path that is not supported + // for vfs if vfs is generally available. "Use virtual files" should not rely on the folder path check at all, as we should + // block use of unsupported paths from the start (generally via account wizard and folder wizard gui's) const auto mode = VfsPluginManager::instance().bestAvailableVfsMode(); const bool useVirtualFiles = (Theme::instance()->forceVirtualFilesOption() && mode == Vfs::WindowsCfApi) || (_folderWizardSelectiveSyncPage && _folderWizardSelectiveSyncPage->useVirtualFiles()); if (useVirtualFiles) { - const auto availability = Vfs::checkAvailability(initialLocalPath(), mode); - if (!availability) { - auto msg = new QMessageBox(QMessageBox::Warning, FolderWizard::tr("Virtual files are not available for the selected folder"), availability.error(), QMessageBox::Ok, ocApp()->gui()->settingsDialog()); + QString vfsSupported = Vfs::pathSupportDetail(initialLocalPath(), mode); + if (!vfsSupported.isEmpty()) { + auto msg = new QMessageBox(QMessageBox::Warning, FolderWizard::tr("Virtual files are not available for the selected folder"), vfsSupported, + QMessageBox::Ok, ocApp()->gui()->settingsDialog()); msg->setAttribute(Qt::WA_DeleteOnClose); msg->open(); return false; @@ -166,8 +170,11 @@ void FolderWizard::sendResult() FolderMan::SyncConnectionDescription FolderWizard::result() { Q_D(FolderWizard); - + // DC-219 todo: + // check this local path to see if it supports vfs if vfs is in play - this check should happen before the wizard is completed, + // eg validate whatever page has the user choice of local path const QString localPath = d->_folderWizardSourcePage->localPath(); + // leave this until we fix the weird handling in account wizard that leaves the sync root empty if selective sync is chosen if (!d->_account->hasDefaultSyncRoot()) { if (FileSystem::isChildPathOf(localPath, d->defaultSyncRoot())) { d->_account->setDefaultSyncRoot(d->defaultSyncRoot()); diff --git a/src/gui/folderwizard/folderwizardlocalpath.cpp b/src/gui/folderwizard/folderwizardlocalpath.cpp index bdff06c5f98..7f56159d240 100644 --- a/src/gui/folderwizard/folderwizardlocalpath.cpp +++ b/src/gui/folderwizard/folderwizardlocalpath.cpp @@ -63,6 +63,10 @@ QString FolderWizardLocalPath::localPath() const bool FolderWizardLocalPath::isComplete() const { + // todo: DC-219 we need a check here to see if the chosen local folder path supports vfs, provided vfs is generally available, + // and reject the path with reason it failed if it's no good. Regardless of whether the user currently wants to use vfs or not, + // we should not accept local paths that *can't* support vfs if they change their mind later. + // use Vfs::pathSupportDetail to get the reason vfs is not supported (path is supported if the return string is empty) auto folderType = FolderMan::NewFolderType::SpacesFolder; auto accountUuid = folderWizardPrivate()->uuid(); QString errorStr = FolderMan::instance()->checkPathValidity(localPath(), folderType, accountUuid); diff --git a/src/gui/newaccountwizard/advancedsettingspagecontroller.cpp b/src/gui/newaccountwizard/advancedsettingspagecontroller.cpp index 4e4ea0e8a5e..9d804326d6d 100644 --- a/src/gui/newaccountwizard/advancedsettingspagecontroller.cpp +++ b/src/gui/newaccountwizard/advancedsettingspagecontroller.cpp @@ -235,9 +235,13 @@ bool AdvancedSettingsPageController::validateSyncRoot(const QString &rootPath) // I'm not testing the case that vfs is actually the chosen mode here as if vfs is available we should just block dirs that don't support it, // the idea being that eg if they change the mode in the page after they select the dir, or use selective sync with vfs later, at least we know the default // root they select here will not cause any issues down the road - if (_vfsIsAvailable && !FolderMan::instance()->checkVfsAvailability(rootPath)) { - _errorField->setText(errorMessageTemplate.arg(rootPath, tr("selected path does not support using virtual file system."))); - return false; + // todo: use this same approach when fixing the folder wizard as part of DC-219 + if (_vfsIsAvailable) { + QString pathSupported = Vfs::pathSupportDetail(rootPath, Vfs::Mode::WindowsCfApi); + if (!pathSupported.isEmpty()) { + _errorField->setText(errorMessageTemplate.arg(rootPath, tr("selected path does not support using virtual file system. %1").arg(pathSupported))); + return false; + } } return true; From bb7465761a90677866466722774408b8778f2b2d Mon Sep 17 00:00:00 2001 From: Modspike Date: Fri, 9 Jan 2026 17:29:03 +0100 Subject: [PATCH 6/6] fix the build missed folderman on last push --- src/gui/folderman.cpp | 32 +------------------------------- 1 file changed, 1 insertion(+), 31 deletions(-) diff --git a/src/gui/folderman.cpp b/src/gui/folderman.cpp index 11b6ca481f8..edc9f8b4440 100644 --- a/src/gui/folderman.cpp +++ b/src/gui/folderman.cpp @@ -800,7 +800,7 @@ void FolderMan::removeFolderFromGui(Folder *f) void FolderMan::forceFolderSync(Folder *f) { if (!f) -return; + return; // Prevent new sync starts _scheduler->stop(); @@ -1029,8 +1029,6 @@ QString FolderMan::findExistingFolderAndCheckValidity(const QString &path, NewFo QString FolderMan::findGoodPathForNewSyncFolder( const QString &basePath, const QString &newFolder, FolderMan::NewFolderType folderType, const QUuid &accountUuid) const { - OC_ASSERT(!accountUuid.isNull() || folderType == FolderMan::NewFolderType::SpacesSyncRoot); - // reserve extra characters to allow appending of a number const QString normalisedPath = FileSystem::createPortableFileName(basePath, FileSystem::pathEscape(newFolder), std::string_view(" (100)").size()); @@ -1085,29 +1083,6 @@ void FolderMan::setIgnoreHiddenFiles(bool ignore) setSyncEnabled(true); } -Result FolderMan::unsupportedConfiguration(const QString &path) const -{ - auto it = _unsupportedConfigurationError.find(path); - if (it == _unsupportedConfigurationError.end()) { - it = _unsupportedConfigurationError.insert(path, [&]() -> Result { - if (numberOfSyncJournals(path) > 1) { - const QString error = tr("Multiple accounts are sharing the folder %1.\n" - "This configuration is know to lead to dataloss and is no longer supported.\n" - "Please consider removing this folder from the account and adding it again.") - .arg(path); - if (Theme::instance()->warnOnMultipleDb()) { - qCWarning(lcFolderMan) << error; - return error; - } else { - qCWarning(lcFolderMan) << error << "this error is not displayed to the user as this is a branded" - << "client and the error itself might be a false positive caused by a previous broken migration"; - } - } - return {}; - }()); - } - return *it; -} bool FolderMan::isSpaceSynced(GraphApi::Space *space) const { @@ -1124,11 +1099,6 @@ void FolderMan::slotReloadSyncOptions() } } -bool FolderMan::checkVfsAvailability(const QString &path, Vfs::Mode mode) const -{ - return unsupportedConfiguration(path) && Vfs::checkAvailability(path, mode); -} - Folder *FolderMan::addFolderFromScratch(AccountState *accountState, FolderDefinition &&folderDefinition, bool useVfs) { if (!FolderMan::prepareFolder(folderDefinition.localPath())) {