D28856: Save disabling of desktop file components in kglobalshortcutsrc
meven added a comment. ping Would be important to land with https://invent.kde.org/frameworks/kglobalaccel/-/merge_requests/2 REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D28856 To: davidre, davidedmundson, fvogt, meven Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D28856: Save disabling of desktop file components in kglobalshortcutsrc
meven requested changes to this revision.
meven added a comment.
This revision now requires changes to proceed.
Just two qDebug to remove, seems fine otherwise
INLINE COMMENTS
> globalshortcutsregistry.cpp:95
> {
> +qDebug() << component->uniqueName();
> if (_components.value(component->uniqueName()))
To remove
> globalshortcutsregistry.cpp:98
> {
> +qDebug() << component->uniqueName();
> Q_ASSERT_X(false, "GlobalShortcutsRegistry::addComponent",
> "component already registered?!?!");
To remove
REPOSITORY
R268 KGlobalAccel
REVISION DETAIL
https://phabricator.kde.org/D28856
To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D28856: Save disabling of desktop file components in kglobalshortcutsrc
davidre updated this revision to Diff 80265. davidre added a comment. Don't parse the disabledGroup as component REPOSITORY R268 KGlobalAccel CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28856?vs=80206=80265 BRANCH disable (branched from master) REVISION DETAIL https://phabricator.kde.org/D28856 AFFECTED FILES src/runtime/globalshortcutsregistry.cpp src/runtime/globalshortcutsregistry.h src/runtime/kserviceactioncomponent.cpp To: davidre, davidedmundson, fvogt, meven Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D28856: Save disabling of desktop file components in kglobalshortcutsrc
davidre marked 4 inline comments as done. REPOSITORY R268 KGlobalAccel REVISION DETAIL https://phabricator.kde.org/D28856 To: davidre, davidedmundson, fvogt, meven Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D28856: Save disabling of desktop file components in kglobalshortcutsrc
davidre added inline comments.
INLINE COMMENTS
> globalshortcutsregistry.cpp:274
> +auto disabledComponents = KConfigGroup(&_config,
> "disabledComponents").readEntry("disabled", QStringList());
> for (const QString : groupList)
> {
good point
> globalshortcutsregistry.cpp:333
> for (const QString : lstDesktopFiles) {
> -if (_components.contains(desktopFile)) {
> +if (_components.contains(desktopFile) ||
> disabledComponents.contains(desktopFile)) {
> continue;
It actually is if you follow the constructor chain
REPOSITORY
R268 KGlobalAccel
REVISION DETAIL
https://phabricator.kde.org/D28856
To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D28856: Save disabling of desktop file components in kglobalshortcutsrc
fvogt requested changes to this revision.
fvogt added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> globalshortcutsregistry.cpp:274
> +auto disabledComponents = KConfigGroup(&_config,
> "disabledComponents").readEntry("disabled", QStringList());
> for (const QString : groupList)
> {
`disabledComponents` is the group name, right? It would also be part of
`groupList`, so it would try to load it as shortcut...
> globalshortcutsregistry.cpp:333
> for (const QString : lstDesktopFiles) {
> -if (_components.contains(desktopFile)) {
> +if (_components.contains(desktopFile) ||
> disabledComponents.contains(desktopFile)) {
> continue;
Is `desktopFile` the equivalent to `component->uniqueName()`? I would assume
no, so this check might need to be moved after the `KServiceActionComponent`
construction
REPOSITORY
R268 KGlobalAccel
REVISION DETAIL
https://phabricator.kde.org/D28856
To: davidre, davidedmundson, fvogt, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D28856: Save disabling of desktop file components in kglobalshortcutsrc
davidre updated this revision to Diff 80206. davidre added a comment. foo REPOSITORY R268 KGlobalAccel CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28856?vs=80205=80206 BRANCH disable (branched from master) REVISION DETAIL https://phabricator.kde.org/D28856 AFFECTED FILES src/runtime/globalshortcutsregistry.cpp src/runtime/globalshortcutsregistry.h src/runtime/kserviceactioncomponent.cpp To: davidre, davidedmundson, fvogt, meven Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
D28856: Save disabling of desktop file components in kglobalshortcutsrc
davidre created this revision. davidre added reviewers: davidedmundson, fvogt, meven. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. davidre requested review of this revision. REVISION SUMMARY Works for writable and not writable files. Additional positive we don't have to modify the desktop file That means a kcm doesn't need to do anything special as the component is enabled again automagically upon doRegister(). REPOSITORY R268 KGlobalAccel BRANCH disable (branched from master) REVISION DETAIL https://phabricator.kde.org/D28856 AFFECTED FILES src/runtime/globalshortcutsregistry.cpp src/runtime/globalshortcutsregistry.h src/runtime/kserviceactioncomponent.cpp To: davidre, davidedmundson, fvogt, meven Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
