D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:7bd7f38400b9: [KRun] when asked to open link in external 
browser, fall back to mimeapps.list… (authored by ngraham).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17371?vs=48040=48041

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

AFFECTED FILES
  src/widgets/krun.cpp

To: ngraham, #frameworks, broulik, cfeck, elvisangelaccio, dfaure
Cc: dfaure, rdieter, achauvel, kde-frameworks-devel, michaelh, ngraham, bruns

D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[Nathaniel Graham]

ngraham updated this revision to Diff 48040.
ngraham marked an inline comment as done.
ngraham added a comment.


  Style fix

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17371?vs=48022=48040

BRANCH
  arcpatch-D17371

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

AFFECTED FILES
  src/widgets/krun.cpp

To: ngraham, #frameworks, broulik, cfeck, elvisangelaccio, dfaure
Cc: dfaure, rdieter, achauvel, kde-frameworks-devel, michaelh, ngraham, bruns

D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[David Faure]

dfaure accepted this revision.
dfaure added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> krun.cpp:1423
> +// If a default browser isn't set in kdeglobals, fall back to 
> mimeapps.list
> +if (! d->m_externalBrowser.isEmpty()) {
> +return;

no space after "!"

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: ngraham, #frameworks, broulik, cfeck, elvisangelaccio, dfaure
Cc: dfaure, rdieter, achauvel, kde-frameworks-devel, michaelh, ngraham, bruns

D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[Nathaniel Graham]

ngraham marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, broulik, cfeck, elvisangelaccio
Cc: achauvel, kde-frameworks-devel, michaelh, ngraham, bruns

D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[Nathaniel Graham]

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


  Reduce unnecessary nesting with an early return

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17371?vs=48021=48022

BRANCH
  master

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

AFFECTED FILES
  src/widgets/krun.cpp

To: ngraham, #frameworks, broulik, cfeck, elvisangelaccio
Cc: achauvel, kde-frameworks-devel, michaelh, ngraham, bruns

D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[Nathaniel Graham]

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


  Reduce unnecessary nesting with an early return

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17371?vs=46913=48021

BRANCH
  default-browser-fallback-to-mimeapps-if-nothing-is-set-in-kdeglobals 
(branched from master)

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

AFFECTED FILES
  src/core/copyjob.cpp
  src/core/copyjob.h
  src/core/global.h
  src/core/job.cpp
  src/core/slaveinterface.cpp
  src/core/slaveinterface_p.h
  src/widgets/krun.cpp

To: ngraham, #frameworks, broulik, cfeck, elvisangelaccio
Cc: achauvel, kde-frameworks-devel, michaelh, ngraham, bruns

D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[Mélanie Chauvel]

achauvel added a comment.


  I think it’s better than what I proposed (without knowing about yours) in 
D17727 .

INLINE COMMENTS

> krun.cpp:1421-1431
> +
> +// If a default browser isn't set in kdeglobals, fall back to 
> mimeapps.list
> +if (d->m_externalBrowser.isEmpty()) {
> +KSharedConfig::Ptr profile = 
> KSharedConfig::openConfig(QStringLiteral("mimeapps.list"), 
> KConfig::NoGlobals, QStandardPaths::GenericConfigLocation);
> +KConfigGroup defaultApps(profile, "Default Applications");
> +
> +d->m_externalBrowser = 
> defaultApps.readEntry("x-scheme-handler/https");

I personally would have gone for

  // If a default browser isn't set in kdeglobals, fall back to mimeapps.list
  if (!d->m_externalBrowser.isEmpty()) {
  return;
  }
  KSharedConfig::Ptr profile = 
KSharedConfig::openConfig(QStringLiteral("mimeapps.list"), KConfig::NoGlobals, 
QStandardPaths::GenericConfigLocation);
  KConfigGroup defaultApps(profile, "Default Applications");
  
  d->m_externalBrowser = defaultApps.readEntry("x-scheme-handler/https");
  if (d->m_externalBrowser.isEmpty()) {
  d->m_externalBrowser = defaultApps.readEntry("x-scheme-handler/http");
  }

to avoid too much nesting.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, broulik, cfeck, elvisangelaccio
Cc: achauvel, kde-frameworks-devel, michaelh, ngraham, bruns

D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[Nathaniel Graham]

ngraham added reviewers: cfeck, elvisangelaccio.
ngraham added a comment.


  Can I get a review on this (or another version someone else submitted: D17727 
)? #frameworks 
 people? #dolphin 
? Anyone?

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, broulik, cfeck, elvisangelaccio
Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D17371: [KRun] when asked to open link in external browser, fall back to mimeapps.list if nothing is set in kdeglobals

[Nathaniel Graham]

ngraham retitled this revision from "[KRun] when asked to open link in external 
browser, ball back to mimeapps.list if nothing is set in kdeglobals" to "[KRun] 
when asked to open link in external browser, fall back to mimeapps.list if 
nothing is set in kdeglobals".

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns