From 6cb301bdbe7c83a6931389cbb9e6c065db084e05 Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Mon, 1 Mar 2021 13:32:14 +0200 Subject: [PATCH] ProfileManager dialog: improve handling profiles - Differentiate between a profile being deleteable and writable: - Disable the Edit key for read-only profiles, i.e. a '.profile' that doesn't have write permissions for the user - Disable the Delete key for a '.profile' that is in a directory that isn't writable for the user (i.e. a .profile can be read-only, but still deleteable by the user) - Change the model selection mode to single selection, that simplifies the code, besides it looks like editing more than one profile at the same time hasn't worked for a while, and there are no complaints AFAICS; remove the now redundant selectedProfiles() method. --- src/settings/ProfileSettings.cpp | 107 +++++++++++++------------------ src/settings/ProfileSettings.h | 3 +- src/settings/ProfileSettings.ui | 6 +- 3 files changed, 51 insertions(+), 65 deletions(-) diff --git a/src/settings/ProfileSettings.cpp b/src/settings/ProfileSettings.cpp index 7ab9fc93..ba6945c8 100644 --- a/src/settings/ProfileSettings.cpp +++ b/src/settings/ProfileSettings.cpp @@ -32,6 +32,7 @@ ProfileSettings::ProfileSettings(QWidget* parent) profilesList->setModel(ProfileModel::instance()); profilesList->setItemDelegateForColumn(ProfileModel::SHORTCUT, new ShortcutItemDelegate(this)); + profilesList->setSelectionMode(QAbstractItemView::SingleSelection); // double clicking the profile name opens the profile edit dialog connect(profilesList, &QAbstractItemView::doubleClicked, this, &Konsole::ProfileSettings::doubleClicked); @@ -61,7 +62,6 @@ void ProfileSettings::slotAccepted() void ProfileSettings::doubleClicked(const QModelIndex &idx) { - if (idx.column() == ProfileModel::NAME) { editSelected(); } @@ -92,32 +92,28 @@ void ProfileSettings::populateTable() void ProfileSettings::tableSelectionChanged(const QItemSelection&) { - const ProfileManager* manager = ProfileManager::instance(); - bool isNotDefault = true; - bool isDeletable = true; - - const auto profiles = selectedProfiles(); - for (const auto &profile: profiles) { - isNotDefault = isNotDefault && (profile != manager->defaultProfile()); - isDeletable = isDeletable && isProfileDeletable(profile); - } + const auto profile = currentProfile(); + const bool isNotDefault = profile != ProfileManager::instance()->defaultProfile(); + + newProfileButton->setEnabled(true); + + // See comment about isProfileWritable(profile) in editSelected() + editProfileButton->setEnabled(isProfileWritable(profile)); - newProfileButton->setEnabled(profiles.count() < 2); - // FIXME: At some point editing 2+ profiles no longer works - editProfileButton->setEnabled(profiles.count() == 1); - // do not allow the default session type to be removed - deleteProfileButton->setEnabled(isDeletable && isNotDefault && (profiles.count() > 0)); - setAsDefaultButton->setEnabled(isNotDefault && (profiles.count() == 1)); + // Do not allow the current default profile of the session to be removed + deleteProfileButton->setEnabled(isNotDefault && isProfileDeletable(profile)); + + setAsDefaultButton->setEnabled(isNotDefault); } void ProfileSettings::deleteSelected() { - const QList profiles = selectedProfiles(); - for (const Profile::Ptr &profile : profiles) { - if (profile != ProfileManager::instance()->defaultProfile()) { - ProfileManager::instance()->deleteProfile(profile); - } - } + const auto profile = currentProfile(); + + // The "Delete" button is disabled for the current default profile + Q_ASSERT(profile != ProfileManager::instance()->defaultProfile()); + + ProfileManager::instance()->deleteProfile(profile); } void ProfileSettings::setSelectedAsDefault() @@ -156,54 +152,40 @@ void ProfileSettings::createProfile() } void ProfileSettings::editSelected() { - const QList profiles = selectedProfiles(); + const auto profile = currentProfile(); + + // Read-only profiles, i.e. oens with .profile's that aren't writable + // for the user aren't editable, only clone-able by using the "New" + // button, this includes the Default/fallback profile, which is hardcoded. + if (!isProfileWritable(profile)) { + return; + } + EditProfileDialog *profileDialog = nullptr; - // sessions() returns a const QList - for (const Session *session : SessionManager::instance()->sessions()) { + const auto sessionsList = SessionManager::instance()->sessions(); + for (const Session *session : sessionsList) { for (TerminalDisplay *terminalDisplay : session->views()) { - // Searching for opened profiles + // Searching for already open EditProfileDialog instances + // for this profile profileDialog = terminalDisplay->sessionController()->profileDialogPointer(); if (profileDialog == nullptr) { continue; } - for (const Profile::Ptr &profile : profiles) { - if (profile->name() == profileDialog->lookupProfile()->name() - && profileDialog->isVisible()) { - // close opened edit dialog - profileDialog->close(); - } + + if (profile->name() == profileDialog->lookupProfile()->name() + && profileDialog->isVisible()) { + // close opened edit dialog + profileDialog->close(); } } } EditProfileDialog dialog(this); - // the dialog will delete the profile group when it is destroyed - ProfileGroup* group = new ProfileGroup; - for (const Profile::Ptr &profile : profiles) { - group->addProfile(profile); - } - group->updateValues(); - dialog.setProfile(Profile::Ptr(group)); + dialog.setProfile(profile); dialog.exec(); } -QList ProfileSettings::selectedProfiles() const -{ - QList list; - QItemSelectionModel* selection = profilesList->selectionModel(); - if (selection == nullptr) { - return list; - } - - const QList selectedIndexes = selection->selectedIndexes(); - for (const QModelIndex &index : selectedIndexes) { - if (index.column() == ProfileModel::PROFILE) { - list << index.data(ProfileModel::ProfilePtrRole).value(); - } - } - return list; -} Profile::Ptr ProfileSettings::currentProfile() const { QItemSelectionModel* selection = profilesList->selectionModel(); @@ -221,17 +203,20 @@ Profile::Ptr ProfileSettings::currentProfile() const bool ProfileSettings::isProfileDeletable(Profile::Ptr profile) const { - if (!profile) { + if (!profile || profile->isFallback()) { return false; } const QFileInfo fileInfo(profile->path()); - if (!fileInfo.exists()) { - return false; - } + return fileInfo.exists() + && QFileInfo(fileInfo.path()).isWritable(); // To delete a file, parent dir must be writable +} - const QFileInfo dirInfo(fileInfo.path()); - return dirInfo.isWritable(); +bool ProfileSettings::isProfileWritable(Profile::Ptr profile) const +{ + return profile + && !profile->isFallback() // Default/fallback profile is hardcoded + && QFileInfo(profile->path()).isWritable(); } void ProfileSettings::setShortcutEditorVisible(bool visible) diff --git a/src/settings/ProfileSettings.h b/src/settings/ProfileSettings.h index a1aedf54..8ff68385 100644 --- a/src/settings/ProfileSettings.h +++ b/src/settings/ProfileSettings.h @@ -71,8 +71,9 @@ private Q_SLOTS: private: QExplicitlySharedDataPointer currentProfile() const; - QList> selectedProfiles() const; + bool isProfileDeletable(QExplicitlySharedDataPointer profile) const; + bool isProfileWritable(QExplicitlySharedDataPointer profile) const; // updates the profile table to be in sync with the // session manager diff --git a/src/settings/ProfileSettings.ui b/src/settings/ProfileSettings.ui index 0f6292b0..1bf73a97 100644 --- a/src/settings/ProfileSettings.ui +++ b/src/settings/ProfileSettings.ui @@ -36,7 +36,7 @@ - Create a new profile based upon the selected profile + Create a new profile based on the selected profile &New... @@ -49,7 +49,7 @@ false - Edit the selected profile(s) + Edit the selected profile (this button is disabled for read-only profiles) &Edit... @@ -62,7 +62,7 @@ false - Delete the selected profile(s) + Delete the selected profile &Remove