The config keys are now in a separate file, instead of having them at two places.
Runner
The duplicate call to reloadConfiguration has been removed and aliases/codes get validated.
Additionally the match method has been simplified and the character is now copied to the clipboard when selected.
KCM
Validation of the add/delete buttons, error message now in GUI and deprecated methods removed.
Details
Details
- Reviewers
davidedmundson ngraham sitter broulik - Group Reviewers
Plasma - Commits
- R114:8dd8fe555a1e: characters: Refactoring, improve validation in kcm, optimize
Compile, test runner and test editing entries.
Manually add alias(and no hex value) to ~/.config/krunnerrc and reload the kcm, error message should be shown.
Before:
After:
Diff Detail
Diff Detail
- Repository
- R114 Plasma Addons
- Branch
- charrunner_improvements (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 22294 Build 22312: arc lint + arc unit
| runners/characters/charrunner.cpp | ||
|---|---|---|
| 40 ↗ | (On Diff #74849) | m_triggerWord is empty when this executes. I think that needs to move back to reloadConfiguration |
| 50 ↗ | (On Diff #74849) | unnecessary ref |
| 90 ↗ | (On Diff #74849) | newline |
| runners/characters/charrunner_config.cpp | ||
| 64 ↗ | (On Diff #74849) | There is no purpose to this size variable. Compilers will optimize i < aliasList.size(). |
| 72 ↗ | (On Diff #74849) | this needs i18nc() |
| 89 ↗ | (On Diff #74849) | count not useful |
| 120 ↗ | (On Diff #74849) | newline |
| 124 ↗ | (On Diff #74849) | newline |
| runners/characters/config_keys.h | ||
|---|---|---|
| 23 | You could use static constexpr QStringView CONFIG_TRIGGERWORD = u"triggerWord" to get something that's compile-time and easily used with QString API | |
Comment Actions
First of all thanks for the idea, but is the QString API needed ? These strings are just used for reading the config and if you have a look at the implementation of the readEntry method:
QStringList KConfigGroup::readEntry(const QString &key, const QStringList &aDefault) const
{
return readEntry(key.toUtf8().constData(), aDefault);
}Basically the method is just converting the QString to a char array and calls the readEntry method (which is currently directly called).
Link: https://api.kde.org/frameworks-api/frameworks-apidocs/frameworks/kconfig/html/kconfiggroup_8cpp_source.html#l00717
Comment Actions
Huh, guess kconfig never goes to QString after all.
@davidre had a similar suggestion of using auto instead and let the compiler do its work IIRC
dvratil's comment still applies to DEFAULT_TRIGGERWORD though.
Generally speaking the global static keyword strings aren't very popluar in our code. Although, TBH, I do not know why.