Bug 438309 - Built-in/hardcoded profile should not be named "Default [Read-only]"
Summary: Built-in/hardcoded profile should not be named "Default [Read-only]"
Status: RESOLVED FIXED
Alias: None
Product: konsole
Classification: Applications
Component: kpart (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Konsole Developer
URL:
Keywords: usability
: 456602 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-06-09 08:12 UTC by ratijas
Modified: 2022-07-13 21:41 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Konsole Profiles configuration tab (16.00 KB, image/png)
2021-06-09 08:12 UTC, ratijas
Details

Note You need to log in before you can comment on or make changes to this bug.
Description ratijas 2021-06-09 08:12:01 UTC
Created attachment 139128 [details]
Konsole Profiles configuration tab

SUMMARY

Built-in/hardcoded profile should not be named "Default [Read-only]", because it leads to confusing naming once user creates more profiles, and set some other one to be the new default.

Also, it's unclear why "Default" can't be deleted (inactive button) until you hover mouse over it and read the pop-up.


STEPS TO REPRODUCE
1. Launch Konsole (fresh install)
2. Create new profile (e.g. duplicate the built-in one)
3. Set you new profile as default on Profiles configuration tab.

OBSERVED RESULT

For example, in my case profile list looks like on the attachment:

- Default [Read-only]
- Best (default)

^ Only first of them can be "set as default" (corresponding button enabled);
^ Only second of them can be edited;
^ Neither of them can be deleted.


EXPECTED RESULT

I certainly don't expect such overloading of the word "Default" in this context. It is confusing, because both instances actually mean different things.

I suggest moving the "Built-in/hardcoded" pop-up to be the name of the "Default" profile itself, like this:

- Built-in [Read-only]
- Best (default)

SOFTWARE/OS VERSIONS
Operating System: Arch Linux
KDE Plasma Version: 5.21.5
KDE Frameworks Version: 5.82.0
Qt Version: 5.15.2
Kernel Version: 5.12.9-arch1-1
OS Type: 64-bit
Graphics Platform: X11

ADDITIONAL INFORMATION

I want to send a patch myself, if this bug/request is approved.
Please, give me a chance :)
Comment 1 Ahmad Samir 2021-06-09 10:52:52 UTC
Patches are always welcome :)
Comment 2 Nate Graham 2021-06-09 23:10:28 UTC
Yeah, go right ahead!
Comment 3 ratijas 2021-06-16 08:35:26 UTC
Turned out little bit more complicated than I expected.

Strangely enough, profiles are identified by their name, not some internal UUID or stuff like that. Which means, some migration code path is needed to ensure that no user-crated profile with as stupid name as "Built-in/Hardcoded" would mess up their uniqueness.

Second, internally code base itself is not consistent about naming things: the "Default" profile aka "Built-in/Hardcoded" is also called "fallback" profile. I think that requires a proper uniform refactoring across all profile-related code.
Comment 4 ratijas 2021-06-16 08:39:06 UTC
Just for the record,

Related code is in the `konsoleprofile` CMake library target (which is in the `src/profile` directory) of https://invent.kde.org/utilities/konsole repo.
Comment 5 Ahmad Samir 2021-06-17 19:51:35 UTC
Yeah, 20+ years worth of code :)

The "fallback" word is used only internally in the code, so it's not visible to users AFAIK. "Built-in" does indeed sound like a good name (which is used sometimes in commit messages related to that profile).
Comment 6 ratijas 2021-07-04 20:37:06 UTC
Just started working on it. There are quite some things to change.

I am renaming everything related to fallback/hardcoded profile to
- "Built-in" in UI, and
- "Builtin" or "builtin" in the code.

Unfortunately that includes command line option `--fallback-profile`,
which I hope no one ever tried to script.

I figured, migration and compatibility should not be a problem, because
built-in profile uses impossible file path name -- and profiles are loaded and identified by their path.
Comment 7 tcanabrava 2022-05-25 11:18:01 UTC
Git commit fca0f4f9d7de1158fb421bb28136bfff5fe11a7c by Tomaz  Canabrava, on behalf of ivan tkachenko.
Committed on 25/05/2022 at 11:15.
Pushed by tcanabrava into branch 'master'.

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.

M  +2    -2    doc/manual/index.docbook
M  +6    -11   src/Application.cpp
M  +48   -10   src/autotests/ProfileTest.cpp
M  +2    -1    src/autotests/ProfileTest.h
M  +1    -1    src/autotests/TerminalInterfaceTest.cpp
M  +17   -10   src/profile/Profile.cpp
M  +5    -6    src/profile/Profile.h
M  +18   -38   src/profile/ProfileManager.cpp
M  +12   -13   src/profile/ProfileManager.h
M  +5    -5    src/profile/ProfileModel.cpp
M  +3    -3    src/session/SessionController.cpp
M  +7    -7    src/settings/ProfileSettings.cpp
M  +2    -2    src/widgets/EditProfileDialog.cpp

https://invent.kde.org/utilities/konsole/commit/fca0f4f9d7de1158fb421bb28136bfff5fe11a7c
Comment 8 Ahmad Samir 2022-07-13 21:41:13 UTC
*** Bug 456602 has been marked as a duplicate of this bug. ***