D27137: Handle spawning kinfocenter with a full path

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R124:a1d44475d4be: Handle spawning kinfocenter with a full 
path (authored by davidedmundson).

REPOSITORY
  R124 System Settings

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27137?vs=74943=75353

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

AFFECTED FILES
  app/main.cpp

To: davidedmundson, #plasma, ngraham
Cc: pino, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27137: Handle spawning kinfocenter with a full path

[David Edmundson]

davidedmundson added a comment.


  I'm going to ship this as it fixes an easy to hit bug that is definitely 
wrong. 
  I'm not trying to ingore any other proposals, we can always expand and change.

REPOSITORY
  R124 System Settings

BRANCH
  master

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

To: davidedmundson, #plasma, ngraham
Cc: pino, ngraham, plasma-devel, mlcarr, Orage, LeGast00n, The-Feren-OS-Dev, 
cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, 
himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
ahiemstra, mart

D27137: Handle spawning kinfocenter with a full path

[David Edmundson]

davidedmundson added a comment.


  > Hello symlink my old friend...
  
  Well sure, but not from something we ship. 
  We don't "definitely" want systemsettings mode either, and it has to be one 
of the two.
  
  Right now it's definitely broken.
  
  @pino do you have any viable counter proposals?

REPOSITORY
  R124 System Settings

BRANCH
  master

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

To: davidedmundson, #plasma, ngraham
Cc: pino, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27137: Handle spawning kinfocenter with a full path

[Nathaniel Graham]

ngraham added a comment.


  If you manually create a symlink called `my-kinfocenter` that points to 
`systemsettings` then clearly you're actually trying to make kinfocenter open 
just with a different binary name for your own convenience, so this new code is 
actually more correct.

REPOSITORY
  R124 System Settings

BRANCH
  master

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

To: davidedmundson, #plasma, ngraham
Cc: pino, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27137: Handle spawning kinfocenter with a full path

[Pino Toscano]

pino added a comment.


  In D27137#605459 , @davidedmundson 
wrote:
  
  > If you run /usr/bin/my-kinfocenter you wouldn't spawn this executable in 
the first place
  
  
  Hello symlink my old friend...

REPOSITORY
  R124 System Settings

BRANCH
  master

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

To: davidedmundson, #plasma, ngraham
Cc: pino, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27137: Handle spawning kinfocenter with a full path

[David Edmundson]

davidedmundson added a comment.


  If you run /usr/bin/my-kinfocenter you wouldn't spawn this executable in the 
first place

REPOSITORY
  R124 System Settings

BRANCH
  master

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

To: davidedmundson, #plasma, ngraham
Cc: pino, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27137: Handle spawning kinfocenter with a full path

[Pino Toscano]

pino added inline comments.

INLINE COMMENTS

> main.cpp:42
> +BaseMode::ApplicationMode mode = BaseMode::SystemSettings;
> +if (executableName.endsWith(QLatin1String("kinfocenter"))) {
> +binaryName = QStringLiteral("kinfocenter");

This will consider also `/usr/bin/my-kinfocenter` as kinfocenter, which is 
definitely not what you want.

REPOSITORY
  R124 System Settings

BRANCH
  master

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

To: davidedmundson, #plasma, ngraham
Cc: pino, ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27137: Handle spawning kinfocenter with a full path

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Nice fix.

REPOSITORY
  R124 System Settings

BRANCH
  master

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

To: davidedmundson, #plasma, ngraham
Cc: ngraham, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27137: Handle spawning kinfocenter with a full path

[David Edmundson]

davidedmundson created this revision.
davidedmundson added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  /usr/bin/kinfocenter doesn't match "kinfocenter"
  
  it also cannot be using as the binaryName in the rest of this method.

TEST PLAN
  Launched
  systemsettings5
  kinfocenter
  /opt/kde5/bin/kinfocenter

REPOSITORY
  R124 System Settings

BRANCH
  master

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

AFFECTED FILES
  app/main.cpp

To: davidedmundson, #plasma
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart