D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham closed this revision.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: ltoscano, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Marco Martin]

mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R296 KDeclarative

BRANCH
  create-kcmshell-openSystemSettings (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: ltoscano, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham updated this revision to Diff 82971.
ngraham added a comment.


  Only open in System Settings or Info Center if systemsettings is installed

REPOSITORY
  R296 KDeclarative

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29711?vs=82751&id=82971

BRANCH
  create-kcmshell-openSystemSettings (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29711

AFFECTED FILES
  src/qmlcontrols/kquickcontrolsaddons/CMakeLists.txt
  src/qmlcontrols/kquickcontrolsaddons/kcmshell.cpp
  src/qmlcontrols/kquickcontrolsaddons/kcmshell.h

To: ngraham, #plasma, mart
Cc: ltoscano, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham added a comment.


  In D29711#671616 , @mart wrote:
  
  > hmm, should probably check before whether systemsettings5 and kinfocenter 
exist beforehand and fallback to kcmshell if not?
  
  
  Yeah Christoph and Luigi brought that up too. Will do.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: ltoscano, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Marco Martin]

mart added a comment.


  hmm, should probably check before whether systemsettings5 and kinfocenter 
exist beforehand and fallback to kcmshell if not?

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: ltoscano, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham added a comment.


  In D29711#670503 , @ltoscano wrote:
  
  > If the scope of this change is just about Plasma applet, of course 
systemsettings is going to be around.
  
  
  Yes the scope of this change is purely for Plasma applets; look at the 
dependent patches.
  
  > But are you sure you want to kill kcmshell? KCMs are not for Plasma only. 
Opening them outside Plasma would still require a minimal shell.
  > 
  > Regarding this change, I guess you may have discussed it already, but 
wouldn't it be possible to find out whether systemsettings is available (and 
then kinfocenter) and use it, without adding a new function?
  
  We didn't kill kcmshell, it's explicitly still around.
  
  I can't add new behaviors to the existing function because of the reasons 
listed in the description section of this patch.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: ltoscano, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Luigi Toscano]

ltoscano added a comment.


  In D29711#670470 , @ngraham wrote:
  
  > In D29711#670458 , @cfeck wrote:
  >
  > > Sorry if I don't understand the scope, but does this mean I am forced to 
install systemsettings to be able to use KCMs?
  >
  >
  > Why, is the "I'm using Plasma but I don't have System Settings installed" 
use case something that you think we should handle?
  >
  > The context is that users were requesting for KCMs to be opened in System 
Settings, not KCMShell (https://bugs.kde.org/show_bug.cgi?id=402790) and we 
implemented that for the KCM runner for Plasma 5.19. The issue fixed here is an 
inconsistency in that opening KCMs from Plasma applet context menu would still 
open it in KCMShell, not System Settings or Info Center (which in 5.19 is now 
just System Settings; the same app shows both).
  
  
  If the scope of this change is just about Plasma applet, of course 
systemsettings is going to be around.
  But are you sure you want to kill kcmshell? KCMs are not for Plasma only. 
Opening them outside Plasma would still require a minimal shell.
  
  Regarding this change, I guess you may have discussed it already, but 
wouldn't it be possible to find out whether systemsettings is available (and 
then kinfocenter) and use it, without adding a new function?

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: ltoscano, cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham added a comment.


  In D29711#670458 , @cfeck wrote:
  
  > Sorry if I don't understand the scope, but does this mean I am forced to 
install systemsettings to be able to use KCMs?
  
  
  Why, is the "I'm using Plasma but I don't have System Settings installed" use 
case something that you think we should handle?
  
  The context is that users were requesting for KCMs to be opened in System 
Settings, not KCMShell (https://bugs.kde.org/show_bug.cgi?id=402790) and we 
implemented that for the KCM runner for Plasma 5.19. The issue fixed here is an 
inconsistency in that opening KCMs from Plasma applet context menu would still 
open it in KCMShell, not System Settings or Info Center (which in 5.19 is now 
just System Settings; the same app shows both).

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Christoph Feck]

cfeck added a comment.


  Sorry if I don't understand the scope, but does this mean I am forced to 
install systemsettings to be able to use KCMs?

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: cfeck, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham added a dependent revision: D29716: [applet] Show KCM in System 
Settings.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham added a dependent revision: D29715: [applet] Show KCM in System 
Settings.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham added a dependent revision: D29714: [applets] Show KCMs in System 
Settings.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham added a dependent revision: D29713: [applets] Show KCMs in System 
Settings.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham added a dependent revision: D29712: [applets] Show KCMs in System 
Settings or Info Center, as appropriate.

REPOSITORY
  R296 KDeclarative

REVISION DETAIL
  https://phabricator.kde.org/D29711

To: ngraham, #plasma, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29711: Create kcmshell.openSystemSettings() and kcmshell.openInfoCenter() functions

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Plasma, mart.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Many Plasma applets use `kcmshell.open` to open the appropriate KCM. However
  now that we're de-emphasizing kcmshell, we want these to open in System
  Settings or Info Center instead.
  
  We can't just change the existing function for a few reasons:
  
  1. This is a frameworks; changing functionality like that isn't nice.
  2. The existing function takes a string list, which KCMShell supports to open 
multiple KCMs, but System Settings does not support this.
  3. There's still an occasional need to open KCMs in KCMShell (e.g. from 
another KCM open in System Settings).
  4. We need to distinguish between opening in System Settings and opening in 
Info Center
  
  Accordingly, two new functions are created: one to open a KCM in System
  Settings, and another to open in Info Center. Applets will be ported to
  use these.

TEST PLAN
  Test dependent patches (see Stack)

REPOSITORY
  R296 KDeclarative

BRANCH
  create-kcmshell-openSystemSettings (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D29711

AFFECTED FILES
  src/qmlcontrols/kquickcontrolsaddons/kcmshell.cpp
  src/qmlcontrols/kquickcontrolsaddons/kcmshell.h

To: ngraham, #plasma, mart
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns