| Summary: | Built-in/hardcoded profile should not be named "Default [Read-only]" | ||
|---|---|---|---|
| Product: | [Applications] konsole | Reporter: | ratijas <me> |
| Component: | kpart | Assignee: | Konsole Developer <konsole-devel> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | a.samirh78, auxsvr, nate, pilotgi |
| Priority: | NOR | Keywords: | usability |
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | https://invent.kde.org/utilities/konsole/commit/fca0f4f9d7de1158fb421bb28136bfff5fe11a7c | Version Fixed In: | |
| Sentry Crash Report: | |||
| Attachments: | Konsole Profiles configuration tab | ||
Patches are always welcome :) Yeah, go right ahead! 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. 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. 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). 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. 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 *** Bug 456602 has been marked as a duplicate of this bug. *** |
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 :)