Commit fca0f4f9 authored by ivan tkachenko's avatar ivan tkachenko Committed by Tomaz Canabrava
Browse files

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
parent a67edd3f
Pipeline #180720 passed with stage
in 2 minutes and 15 seconds
......@@ -1230,8 +1230,8 @@ instead of the default profile.</para></listitem>
</varlistentry>
<varlistentry>
<term><option>--fallback-profile</option></term>
<listitem><para>Use the internal FALLBACK profile. This option is a shortcut for <option>--profile</option> <parameter>FALLBACK/</parameter>.<!--FIXME what is the internal Fallback profile--></para></listitem>
<term><option>--builtin-profile</option></term>
<listitem><para>Use the built-in profile instead of the current default profile.</para></listitem>
</varlistentry>
<varlistentry>
......
......@@ -52,7 +52,7 @@ void Application::populateCommandLineParser(QCommandLineParser *parser)
const auto options = QVector<QCommandLineOption>{
{{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 <name> is given, loads profile <name>.
// 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()
......
......@@ -7,14 +7,21 @@
// Own
#include "ProfileTest.h"
// KDE
// Qt
#include <QFile>
#include <QFileInfo>
#include <qtest.h>
#include <QStandardPaths>
#include <QTemporaryFile>
#include <QTest>
#include <QTextStream>
// KDE
// Konsole
#include "../profile/Profile.h"
#include "../profile/ProfileGroup.h"
#include "../profile/ProfileManager.cpp"
#include "../profile/ProfileReader.h"
#include "../profile/ProfileWriter.h"
#include <QStandardPaths>
......@@ -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<QString>(Profile::UntranslatedName), QStringLiteral("Default"));
QCOMPARE(fallback->property<QString>(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<QString>(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)
......@@ -22,7 +22,8 @@ private Q_SLOTS:
void testProfileGroup();
void testProfileFileNames();
void testProfileNameSorting();
void testFallbackProfile();
void testBuiltinProfile();
void testLoadProfileNamedAsBuiltin();
};
}
......
......@@ -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();
......
......@@ -143,6 +143,16 @@ const Profile::PropertyInfo Profile::DefaultPropertyNames[] = {
QHash<QString, Profile::PropertyInfo> Profile::PropertyInfoByName;
QHash<Profile::Property, Profile::PropertyInfo> 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
......
......@@ -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.
*/
......
......@@ -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 <App>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, QHash<Profile::Property
const QKeySequence origShortcut = shortcut(profile);
const bool isDefaultProfile = profile == defaultProfile();
const QString uniqueProfileName = generateUniqueName();
// Don't save a profile with an empty name on disk
persistent = persistent && !profile->name().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);
}
......
......@@ -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 "<konsole data dir>/My Profile.profile" then both
* "konsole/My Profile.profile" , "My Profile.profile" and
* is "<konsole data dir>/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<Profile::Ptr> _profiles;
Profile::Ptr _defaultProfile;
Profile::Ptr _fallbackProfile;
Profile::Ptr _builtinProfile;
struct ShortcutData {
Profile::Ptr profileKey;
......
......@@ -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;
......
......@@ -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();
......
......@@ -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();
}
......
......@@ -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;
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment