From fca0f4f9d7de1158fb421bb28136bfff5fe11a7c Mon Sep 17 00:00:00 2001 From: ivan tkachenko Date: Thu, 2 Sep 2021 22:16:42 +0300 Subject: [PATCH] Rename "fallback" profile to Built-in * Rename everything related to built-in profile both in code and UI. * Unified style for [Read-only] and [Default] badges in profile manager's list model. * Change --fallback-profile option to --builtin-profile. * Backward compatibility: yes. If a user happened to name their profile "Built-in", it would continue to work as normal, and even load with `--profile "Built-in"` command line flag. It will let them modify any property including Name; but changing the name back to "Built-in" shows a warning and reverts the change as usual. * Remove "This option is a shortcut for" sentence. While it is still technically possible to pass built-in profile's magic path, this option is not a shortcut, nor implemented as such. * Delete extra naming conditions in ProfileManager::changeProfile, because they could never be triggered anyway, due to pre-flight checks in EditProfileDialog::isProfileNameValid. Automatic unique profile names generation has been done even earlier in either SessionController or ProfileSettings. Just as before, users will continue experiencing a generic "A profile with the name \"%1\" already exists." message. Tests: * Improve test for uncreatable file name of built-in profile. * Add backward compatibility test for loading existing profile named "Built-in" which also references real built-in as its parent. BUG: 438309 --- doc/manual/index.docbook | 4 +- src/Application.cpp | 17 +++----- src/autotests/ProfileTest.cpp | 58 ++++++++++++++++++++----- src/autotests/ProfileTest.h | 3 +- src/autotests/TerminalInterfaceTest.cpp | 2 +- src/profile/Profile.cpp | 27 +++++++----- src/profile/Profile.h | 11 +++-- src/profile/ProfileManager.cpp | 56 ++++++++---------------- src/profile/ProfileManager.h | 25 +++++------ src/profile/ProfileModel.cpp | 10 ++--- src/session/SessionController.cpp | 6 +-- src/settings/ProfileSettings.cpp | 14 +++--- src/widgets/EditProfileDialog.cpp | 4 +- 13 files changed, 128 insertions(+), 109 deletions(-) diff --git a/doc/manual/index.docbook b/doc/manual/index.docbook index 08d91096c..79fc6859b 100644 --- a/doc/manual/index.docbook +++ b/doc/manual/index.docbook @@ -1230,8 +1230,8 @@ instead of the default profile. - -Use the internal FALLBACK profile. This option is a shortcut for FALLBACK/. + +Use the built-in profile instead of the current default profile. diff --git a/src/Application.cpp b/src/Application.cpp index 5e45cdf9d..587a69f32 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -52,7 +52,7 @@ void Application::populateCommandLineParser(QCommandLineParser *parser) const auto options = QVector{ {{QStringLiteral("profile")}, i18nc("@info:shell", "Name of profile to use for new Konsole instance"), QStringLiteral("name")}, {{QStringLiteral("layout")}, i18nc("@info:shell", "json layoutfile to be loaded to use for new Konsole instance"), QStringLiteral("file")}, - {{QStringLiteral("fallback-profile")}, i18nc("@info:shell", "Use the internal FALLBACK profile")}, + {{QStringLiteral("builtin-profile")}, i18nc("@info:shell", "Use the built-in profile instead of the default profile")}, {{QStringLiteral("workdir")}, i18nc("@info:shell", "Set the initial working directory of the new tab or window to 'dir'"), QStringLiteral("dir")}, {{QStringLiteral("hold"), QStringLiteral("noclose")}, i18nc("@info:shell", "Do not close the initial session automatically when it ends.")}, // BR: 373440 @@ -408,25 +408,20 @@ MainWindow *Application::processWindowArgs(bool &createdNewMainWindow) // Loads a profile. // If --profile is given, loads profile . -// If --fallback-profile is given, loads profile FALLBACK/. +// If --builtin-profile is given, loads built-in profile. // Else loads the default profile. Profile::Ptr Application::processProfileSelectArgs() { - Profile::Ptr defaultProfile = ProfileManager::instance()->defaultProfile(); - if (m_parser->isSet(QStringLiteral("profile"))) { Profile::Ptr profile = ProfileManager::instance()->loadProfile(m_parser->value(QStringLiteral("profile"))); if (profile) { return profile; } - } else if (m_parser->isSet(QStringLiteral("fallback-profile"))) { - Profile::Ptr profile = ProfileManager::instance()->loadProfile(QStringLiteral("FALLBACK/")); - if (profile) { - return profile; - } + } else if (m_parser->isSet(QStringLiteral("builtin-profile"))) { + // no need to check twice: built-in and default profiles are always available + return ProfileManager::instance()->builtinProfile(); } - - return defaultProfile; + return ProfileManager::instance()->defaultProfile(); } bool Application::processHelpArgs() diff --git a/src/autotests/ProfileTest.cpp b/src/autotests/ProfileTest.cpp index 9f4d6703c..2d1991a02 100644 --- a/src/autotests/ProfileTest.cpp +++ b/src/autotests/ProfileTest.cpp @@ -7,14 +7,21 @@ // Own #include "ProfileTest.h" -// KDE +// Qt +#include #include -#include +#include +#include +#include +#include + +// KDE // Konsole #include "../profile/Profile.h" #include "../profile/ProfileGroup.h" #include "../profile/ProfileManager.cpp" +#include "../profile/ProfileReader.h" #include "../profile/ProfileWriter.h" #include @@ -234,7 +241,7 @@ void ProfileTest::testProfileNameSorting() int counter = 1; QCOMPARE(list.size(), origCount + counter++); // Built-in profile always at the top - QCOMPARE(list.at(0)->name(), QStringView(u"Default")); + QCOMPARE(list.at(0)->name(), QStringLiteral("Built-in")); QVERIFY(std::is_sorted(list.cbegin(), list.cend(), profileNameLessThan)); @@ -252,18 +259,49 @@ void ProfileTest::testProfileNameSorting() QCOMPARE(list.size(), origCount + counter++); QVERIFY(std::is_sorted(list.cbegin(), list.cend(), profileNameLessThan)); - QCOMPARE(list.at(0)->name(), QLatin1String("Default")); + QCOMPARE(list.at(0)->name(), QStringLiteral("Built-in")); } -void ProfileTest::testFallbackProfile() +void ProfileTest::testBuiltinProfile() { // create a new profile - Profile *fallback = new Profile(); - fallback->useFallback(); + Profile::Ptr builtin = Profile::Ptr(new Profile); + QVERIFY(!builtin->isBuiltin()); - QCOMPARE(fallback->property(Profile::UntranslatedName), QStringLiteral("Default")); - QCOMPARE(fallback->property(Profile::Path), QStringLiteral("FALLBACK/")); - delete fallback; + builtin->useBuiltin(); + QVERIFY(builtin->isBuiltin()); + QCOMPARE(builtin->untranslatedName(), QStringLiteral("Built-in")); + QCOMPARE(builtin->path(), QStringLiteral("FALLBACK/")); +} + +void ProfileTest::testLoadProfileNamedAsBuiltin() +{ + // Create a new profile data which is literally named "Built-in". + // New code should support loading such profiles created in older versions, + // but new profiles must not be saved with such name. + Profile::Ptr builtin = Profile::Ptr(new Profile); + builtin->useBuiltin(); + + QString profileStr = QStringLiteral( + "[General]\n" + "Icon=terminator\n" + "Name=Built-in\n" + "Parent=FALLBACK/\n"); + + QTemporaryFile file(QStringLiteral("konsole.XXXXXX.profile")); + QVERIFY(file.open()); + QTextStream(&file) << profileStr; + + Profile::Ptr profile = Profile::Ptr(new Profile(builtin)); + + ProfileReader reader; + QString parentProfilePath; + QVERIFY(reader.readProfile(file.fileName(), profile, parentProfilePath)); + + QCOMPARE(profile->property(Profile::Icon), QStringLiteral("terminator")); + // It's called "Built-in", but its parent is the real built-in profile + QCOMPARE(profile->name(), builtin->name()); + QCOMPARE(parentProfilePath, builtin->path()); } QTEST_GUILESS_MAIN(ProfileTest) diff --git a/src/autotests/ProfileTest.h b/src/autotests/ProfileTest.h index 7b5a7322b..c38fc504d 100644 --- a/src/autotests/ProfileTest.h +++ b/src/autotests/ProfileTest.h @@ -22,7 +22,8 @@ private Q_SLOTS: void testProfileGroup(); void testProfileFileNames(); void testProfileNameSorting(); - void testFallbackProfile(); + void testBuiltinProfile(); + void testLoadProfileNamedAsBuiltin(); }; } diff --git a/src/autotests/TerminalInterfaceTest.cpp b/src/autotests/TerminalInterfaceTest.cpp index 867fb770b..b04d7100c 100644 --- a/src/autotests/TerminalInterfaceTest.cpp +++ b/src/autotests/TerminalInterfaceTest.cpp @@ -197,7 +197,7 @@ void TerminalInterfaceTest::testTerminalInterfaceV2() { #ifdef USE_TERMINALINTERFACEV2 Profile::Ptr testProfile(new Profile); - testProfile->useFallback(); + testProfile->useBuiltin(); ProfileManager::instance()->addProfile(testProfile); _terminalPart = createPart(); diff --git a/src/profile/Profile.cpp b/src/profile/Profile.cpp index 12853575f..de4418160 100644 --- a/src/profile/Profile.cpp +++ b/src/profile/Profile.cpp @@ -143,6 +143,16 @@ const Profile::PropertyInfo Profile::DefaultPropertyNames[] = { QHash Profile::PropertyInfoByName; QHash Profile::PropertyInfoByProperty; +// Magic path for the built-in profile which is not a valid file name, +// thus it can not interfere with regular profiles. +// For backward compatibility with existing profiles, it should never change. +static const QString BUILTIN_MAGIC_PATH = QStringLiteral("FALLBACK/"); + +// UntranslatedName property of the built-in profile, as a char array. +// +// Note: regular profiles created in earlier versions of Konsole may have this name too. +static const char BUILTIN_UNTRANSLATED_NAME_CHAR[] = "Built-in"; + void Profile::fillTableWithDefaultNames() { static bool filledDefaults = false; @@ -160,14 +170,11 @@ void Profile::fillTableWithDefaultNames() filledDefaults = true; } -void Profile::useFallback() +void Profile::useBuiltin() { - // Fallback settings - setProperty(Name, i18nc("Name of the default/builtin profile", "Default")); - setProperty(UntranslatedName, QStringLiteral("Default")); - // magic path for the fallback profile which is not a valid - // non-directory file name - setProperty(Path, QStringLiteral("FALLBACK/")); + setProperty(Name, i18nc("Name of the built-in profile", BUILTIN_UNTRANSLATED_NAME_CHAR)); + setProperty(UntranslatedName, QString::fromLatin1(BUILTIN_UNTRANSLATED_NAME_CHAR)); + setProperty(Path, BUILTIN_MAGIC_PATH); setProperty(Command, QString::fromUtf8(qgetenv("SHELL"))); // See Pty.cpp on why Arguments is populated setProperty(Arguments, QStringList() << QString::fromUtf8(qgetenv("SHELL"))); @@ -250,7 +257,7 @@ void Profile::useFallback() setProperty(VerticalLine, false); setProperty(VerticalLineAtChar, 80); setProperty(PeekPrimaryKeySequence, QString()); - // Fallback should not be shown in menus + // Built-in profile should not be shown in menus setHidden(true); } @@ -282,9 +289,9 @@ void Profile::clone(Profile::Ptr profile, bool differentOnly) Profile::~Profile() = default; -bool Profile::isFallback() const +bool Profile::isBuiltin() const { - return path() == QLatin1String{"FALLBACK/"}; + return path() == BUILTIN_MAGIC_PATH; } bool Profile::isHidden() const diff --git a/src/profile/Profile.h b/src/profile/Profile.h index 9a38bb25f..1310e9d14 100644 --- a/src/profile/Profile.h +++ b/src/profile/Profile.h @@ -375,10 +375,10 @@ public: /** * A profile which contains a number of default settings for various - * properties. This can be used as a parent for other profiles or a + * properties. This can be used as a parent for other profiles or as a * fallback in case a profile cannot be loaded from disk. */ - void useFallback(); + void useBuiltin(); /** * Changes the parent profile. When calling the property() method, @@ -422,16 +422,15 @@ public: bool isEmpty() const; /** - * Returns true if this profile is the fallback profile, i.e. the - * profile path is "FALLBACK/". + * Returns true if this profile is the built-in profile. */ - bool isFallback() const; + bool isBuiltin() const; /** * Returns true if this is a 'hidden' profile which should not be * displayed in menus or saved to disk. * - * This is used for the fallback profile, in case there are no profiles on + * This is true for the built-in profile, in case there are no profiles on * disk which can be loaded, or for overlay profiles created to handle * command-line arguments which change profile properties. */ diff --git a/src/profile/ProfileManager.cpp b/src/profile/ProfileManager.cpp index 18bc1dda8..73748f284 100644 --- a/src/profile/ProfileManager.cpp +++ b/src/profile/ProfileManager.cpp @@ -38,10 +38,10 @@ static bool stringLessThan(const QString &p1, const QString &p2) static bool profileNameLessThan(const Profile::Ptr &p1, const Profile::Ptr &p2) { - // Always put the Default/fallback profile at the top - if (p1->isFallback()) { + // Always put the built-in profile at the top + if (p1->isBuiltin()) { return true; - } else if (p2->isFallback()) { + } else if (p2->isBuiltin()) { return false; } @@ -51,9 +51,9 @@ static bool profileNameLessThan(const Profile::Ptr &p1, const Profile::Ptr &p2) ProfileManager::ProfileManager() : m_config(KSharedConfig::openConfig()) { - // load fallback profile - initFallbackProfile(); - _defaultProfile = _fallbackProfile; + // ensure built-in profile is available + initBuiltinProfile(); + _defaultProfile = _builtinProfile; // lookup the default profile specified in rc // For stand-alone Konsole, m_config is just "konsolerc" @@ -89,18 +89,17 @@ ProfileManager::Iterator ProfileManager::findProfile(const Profile::Ptr &profile return std::find(_profiles.cbegin(), _profiles.cend(), profile); } -void ProfileManager::initFallbackProfile() +void ProfileManager::initBuiltinProfile() { - _fallbackProfile = Profile::Ptr(new Profile()); - _fallbackProfile->useFallback(); - addProfile(_fallbackProfile); + _builtinProfile = Profile::Ptr(new Profile()); + _builtinProfile->useBuiltin(); + addProfile(_builtinProfile); } Profile::Ptr ProfileManager::loadProfile(const QString &shortPath) { - // the fallback profile has a 'special' path name, "FALLBACK/" - if (shortPath == _fallbackProfile->path()) { - return _fallbackProfile; + if (shortPath == builtinProfile()->path()) { + return builtinProfile(); } QString path = shortPath; @@ -144,14 +143,14 @@ Profile::Ptr ProfileManager::loadProfile(const QString &shortPath) if (recursionGuard.contains(path)) { qCDebug(KonsoleDebug) << "Ignoring attempt to load profile recursively from" << path; - return _fallbackProfile; + return _builtinProfile; } recursionGuard.push(path); // load the profile ProfileReader reader; - Profile::Ptr newProfile = Profile::Ptr(new Profile(fallbackProfile())); + Profile::Ptr newProfile = Profile::Ptr(new Profile(builtinProfile())); newProfile->setProperty(Profile::Path, path); QString parentProfilePath; @@ -237,9 +236,9 @@ Profile::Ptr ProfileManager::defaultProfile() const { return _defaultProfile; } -Profile::Ptr ProfileManager::fallbackProfile() const +Profile::Ptr ProfileManager::builtinProfile() const { - return _fallbackProfile; + return _builtinProfile; } QString ProfileManager::generateUniqueName() const @@ -276,36 +275,17 @@ void ProfileManager::changeProfile(Profile::Ptr profile, QHashname().isEmpty(); - bool messageShown = false; bool isNameChanged = false; + // Insert the changes into the existing Profile instance for (auto it = propertyMap.cbegin(); it != propertyMap.cend(); ++it) { const auto property = it.key(); auto value = it.value(); - isNameChanged = property == Profile::Name || property == Profile::UntranslatedName; - - // "Default" is reserved for the fallback profile, override it; - // The message is only shown if the user manually typed "Default" - // in the name box in the edit profile dialog; i.e. saving the - // fallback profile where the user didn't change the name at all, - // the uniqueProfileName is used silently a couple of lines above. - if (isNameChanged && value == QLatin1String("Default")) { - value = uniqueProfileName; - if (!messageShown) { - KMessageBox::sorry(nullptr, - i18n("The name \"Default\" is reserved for the built-in" - " fallback profile;\nthe profile is going to be" - " saved as \"%1\"", - uniqueProfileName)); - messageShown = true; - } - } + isNameChanged |= property == Profile::Name || property == Profile::UntranslatedName; profile->setProperty(property, value); } diff --git a/src/profile/ProfileManager.h b/src/profile/ProfileManager.h index 077c6ba71..2a68baa7d 100644 --- a/src/profile/ProfileManager.h +++ b/src/profile/ProfileManager.h @@ -76,8 +76,8 @@ public: * * @p path may be relative or absolute. The path may just be the * base name of the profile to load (eg. if the profile's full path - * is "/My Profile.profile" then both - * "konsole/My Profile.profile" , "My Profile.profile" and + * is "/My Profile.profile" then any of the + * "konsole/My Profile.profile", "My Profile.profile" and * "My Profile" will be accepted) * * @return Pointer to a profile which can be passed to @@ -87,12 +87,11 @@ public: Profile::Ptr loadProfile(const QString &shortPath); /** - * This creates a profile based on the fallback profile, it's shown - * as "Default". This is a special profile as it's not saved on disk - * but rather created from code in the Profile class, based on the default - * profile settings. + * Initialize built-in profile. It's shown as "Built-in". This is a + * special profile as it's not saved on disk but rather created from + * code in the Profile class, based on the default profile settings. */ - void initFallbackProfile(); + void initBuiltinProfile(); /** * Searches for available profiles on-disk and returns a list @@ -141,16 +140,16 @@ public: void setDefaultProfile(const Profile::Ptr &profile); /** - * Returns a Profile object describing the default profile + * Returns the current default profile. */ Profile::Ptr defaultProfile() const; /** - * Returns a Profile object with hard-coded settings which is always available. - * This can be used as a parent for new profiles which provides suitable default settings - * for all properties. + * Returns a Profile object with some built-in sane defaults. + * It is always available, and it is NOT loaded from or saved to a file. + * This can be used as a parent for new profiles. */ - Profile::Ptr fallbackProfile() const; + Profile::Ptr builtinProfile() const; /** * Associates a shortcut with a particular profile. @@ -228,7 +227,7 @@ private: std::vector _profiles; Profile::Ptr _defaultProfile; - Profile::Ptr _fallbackProfile; + Profile::Ptr _builtinProfile; struct ShortcutData { Profile::Ptr profileKey; diff --git a/src/profile/ProfileModel.cpp b/src/profile/ProfileModel.cpp index cdf83910e..46f33fd7e 100644 --- a/src/profile/ProfileModel.cpp +++ b/src/profile/ProfileModel.cpp @@ -32,7 +32,7 @@ int ProfileModel::rowCount(const QModelIndex &unused) const { Q_UNUSED(unused) - // All profiles plus the default profile that's always at index 0 on + // All profiles plus the built-in profile that's always on top return m_profiles.count(); } @@ -77,12 +77,12 @@ QVariant ProfileModel::data(const QModelIndex &idx, int role) const switch (role) { case Qt::DisplayRole: { QString txt = profile->name(); - if (profile->isFallback()) { - txt += i18nc("@label:textbox added to the name of the Default/fallback profile, to point out it's read-only/hardcoded", " [Read-only]"); + if (profile->isBuiltin()) { + txt += i18nc("@label:textbox added to the name of the built-in profile, to point out it's read-only", " [Read-only]"); } if (ProfileManager::instance()->defaultProfile() == profile) { - txt += i18nc("@label:textbox added to the name of the current default profile", " (default)"); + txt += i18nc("@label:textbox added to the name of the current default profile", " [Default]"); } return txt; @@ -97,7 +97,7 @@ QVariant ProfileModel::data(const QModelIndex &idx, int role) const } break; case Qt::ToolTipRole: - return profile->isFallback() ? QStringLiteral("Built-in/hardcoded") : profile->path(); + return profile->isBuiltin() ? i18nc("@info:tooltip", "Built-in profile is always available") : profile->path(); } } break; diff --git a/src/session/SessionController.cpp b/src/session/SessionController.cpp index 0f50aae69..eae7b746e 100644 --- a/src/session/SessionController.cpp +++ b/src/session/SessionController.cpp @@ -916,7 +916,7 @@ void SessionController::switchProfile(const Profile::Ptr &profile) void SessionController::setEditProfileActionText(const Profile::Ptr &profile) { QAction *action = actionCollection()->action(QStringLiteral("edit-current-profile")); - if (profile->isFallback()) { + if (profile->isBuiltin()) { action->setText(i18n("Create New Profile...")); } else { action->setText(i18n("Edit Current Profile...")); @@ -951,8 +951,8 @@ void SessionController::editCurrentProfile() auto profile = SessionManager::instance()->sessionProfile(session()); auto state = EditProfileDialog::ExistingProfile; - // Don't edit the Fallback profile, instead create a new one - if (profile->isFallback()) { + // Don't edit the built-in profile, instead create a new one + if (profile->isBuiltin()) { auto newProfile = Profile::Ptr(new Profile(profile)); newProfile->clone(profile, true); const QString uniqueName = ProfileManager::instance()->generateUniqueName(); diff --git a/src/settings/ProfileSettings.cpp b/src/settings/ProfileSettings.cpp index 6b9f90032..2c7e6b6dd 100644 --- a/src/settings/ProfileSettings.cpp +++ b/src/settings/ProfileSettings.cpp @@ -132,10 +132,10 @@ void ProfileSettings::setSelectedAsDefault() void ProfileSettings::createProfile() { - auto newProfile = Profile::Ptr(new Profile(ProfileManager::instance()->fallbackProfile())); + auto newProfile = Profile::Ptr(new Profile(ProfileManager::instance()->builtinProfile())); // If a profile is selected, clone its properties, otherwise the - // the fallback profile properties will be used + // the built-in profile's properties will be used if (currentProfile()) { newProfile->clone(currentProfile(), true); } @@ -156,9 +156,9 @@ void ProfileSettings::editSelected() { 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. + // Read-only profiles (i.e. with non-user-writable .profile location) + // aren't editable, and can only be cloned using the "New" button. + // This includes the built-in profile, which is hardcoded. if (!isProfileWritable(profile)) { return; } @@ -183,7 +183,7 @@ Profile::Ptr ProfileSettings::currentProfile() const bool ProfileSettings::isProfileDeletable(Profile::Ptr profile) const { - if (!profile || profile->isFallback()) { + if (!profile || profile->isBuiltin()) { return false; } @@ -193,7 +193,7 @@ bool ProfileSettings::isProfileDeletable(Profile::Ptr profile) const bool ProfileSettings::isProfileWritable(Profile::Ptr profile) const { - return profile && !profile->isFallback() // Default/fallback profile is hardcoded + return profile && !profile->isBuiltin() // Built-in profile is hardcoded and never stored. && QFileInfo(profile->path()).isWritable(); } diff --git a/src/widgets/EditProfileDialog.cpp b/src/widgets/EditProfileDialog.cpp index 6468e2b0f..45a01134b 100644 --- a/src/widgets/EditProfileDialog.cpp +++ b/src/widgets/EditProfileDialog.cpp @@ -239,10 +239,10 @@ void EditProfileDialog::save() // Update the default profile if needed if (defaultChanged) { - Q_ASSERT(_profile != ProfileManager::instance()->fallbackProfile()); + Q_ASSERT(_profile != ProfileManager::instance()->builtinProfile()); bool defaultChecked = _generalUi->setAsDefaultButton->isChecked(); - Profile::Ptr newDefault = defaultChecked ? _profile : ProfileManager::instance()->fallbackProfile(); + Profile::Ptr newDefault = defaultChecked ? _profile : ProfileManager::instance()->builtinProfile(); ProfileManager::instance()->setDefaultProfile(newDefault); _isDefault = defaultChecked; }