D27451: Drop KToolInvocation support from KRun::runService

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> krun.cpp:757
> -// Resolve urls if needed, depending on what the app supports
> -const QList urls = resolveURLs(_urls, _service);
> -

Compiler now complains nothing uses `resolveURLS()`after this.
Clean up  proposed by D28537 

REPOSITORY
  R241 KIO

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

To: davidedmundson, dfaure
Cc: kossebau, ervin, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D27451: Drop KToolInvocation support from KRun::runService

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:f7cf3b676a4e: [krun] Drop KToolInvocation support from 
KRun::runService (authored by davidedmundson).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27451?vs=75867=76755

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

AFFECTED FILES
  src/widgets/krun.cpp

To: davidedmundson, dfaure
Cc: ervin, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27451: Drop KToolInvocation support from KRun::runService

[David Edmundson]

davidedmundson added a comment.


  > This makes runService and runApplication almost the same, right?
  
  Yeah, that was the goal.
  
  > KRecentDocument::add seems missing in runApplication, but that's a bug
  
  If that is a bug, then we may as well share the implementation. I'll do that.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: davidedmundson, dfaure
Cc: ervin, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27451: Drop KToolInvocation support from KRun::runService

[David Faure]

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


  This makes runService and runApplication almost the same, right?
  
  KRecentDocument::add seems missing in runApplication, but that's a bug, so 
one could just call the other?

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: davidedmundson, dfaure
Cc: ervin, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27451: Drop KToolInvocation support from KRun::runService

[David Edmundson]

davidedmundson updated this revision to Diff 75867.
davidedmundson added a comment.


  Remove extra include

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27451?vs=75811=75867

BRANCH
  master

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

AFFECTED FILES
  src/widgets/krun.cpp

To: davidedmundson, dfaure
Cc: ervin, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27451: Drop KToolInvocation support from KRun::runService

[Kevin Ottens]

ervin added a comment.


  For the record, it LGTM and I'm glad to see this point discussing during the 
sprint coming through. :-)

REPOSITORY
  R241 KIO

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

To: davidedmundson, dfaure
Cc: ervin, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27451: Drop KToolInvocation support from KRun::runService

[Ahmad Samir]

ahmadsamir added a reviewer: dfaure.

REPOSITORY
  R241 KIO

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

To: davidedmundson, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27451: Drop KToolInvocation support from KRun::runService

[David Edmundson]

davidedmundson created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
davidedmundson requested review of this revision.

REVISION SUMMARY
  This drops the use of kinit as per T12140 
. Few applications link to
  kdeinit correctly and the potential savings that come from library 
  linking are now negligible.
  
  We use the internal app spawning path for the majority of application
  launches anyway via the relatively new KRun::runApplication. There isn't
  a need for runService to support a more complex addition path.
  
  It will also simplify any future changes coming to KRun.

TEST PLAN
  Opened a file from kicker's recently used selection (which uses this 
runService)
  It opened as normal, with correct startup notifications

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/widgets/krun.cpp

To: davidedmundson
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns