[UrlHandler] Handle opening the online docs for KCM modules
ClosedPublic

Authored by ahmadsamir on Feb 24 2020, 8:21 AM.

Details

Summary

For KCM modules, clicking the "Help" button when khelpcenter isn't
installed should open the corresponding documentaion for that module
at docs.kde.org; for example with the Fonts KCM, the "&application="
part of the url should be "&application=kcontorl/fonts".

See D27518 for more details.

Thanks to ltoscano for fixing docs.kde.org to make "&application=kcontrol/<module name>"
actually resolve to the correct url path.

Test Plan
  • make && ctest
  • building systemsettings after applying D27518, make sure khelpcenter isn't available on the system, then open the fonts KCM and click "Help" the relevant page at docs.kde.org should open
  • in systemsettings from its menu open the "Systemsettings Handbook", it should open the relevant page at docs.kde.org

Diff Detail

Repository
R273 KGuiAddons
Branch
l-urlhandler-optimi (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 22998
Build 23016: arc lint + arc unit
ahmadsamir created this revision.Feb 24 2020, 8:21 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptFeb 24 2020, 8:21 AM
ahmadsamir requested review of this revision.Feb 24 2020, 8:21 AM
apol added a comment.Feb 24 2020, 4:16 PM

LGTM otherwise

src/util/urlhandler.cpp
56

won't this recurse forever?

I created D27615 first; then this one on top of it (optimising the code in a separate commit).

src/util/urlhandler.cpp
56

I could be wrong, but this is exactly equivalent to the original code, i.e. it won't reach this unless helpcenter is empty and it's not a kde app... and I actually wanted to ask about this bit from the original code, what is it supposed to do?

apol added inline comments.Feb 25 2020, 5:53 PM
src/util/urlhandler.cpp
56

I guess it was in case it wasn't found. Maybe just change into an error saying "can't open the following help url %1" where %1 is the url

ahmadsamir updated this revision to Diff 76463.Feb 26 2020, 1:09 PM
ahmadsamir retitled this revision from [UrlHandler] optimise the code and less if nesting to [UrlHandler] Handle opening the online docs for KCM modules.
ahmadsamir edited the summary of this revision. (Show Details)
ahmadsamir edited the test plan for this revision. (Show Details)

Show message when we can't open help:/ url

apol requested changes to this revision.Feb 26 2020, 2:14 PM

We can't add new dependencies to a tier1 framework, it would break the whole dependency system.

Just make it a qCWarning.

This revision now requires changes to proceed.Feb 26 2020, 2:14 PM
ahmadsamir updated this revision to Diff 76474.Feb 26 2020, 5:01 PM
ahmadsamir edited the summary of this revision. (Show Details)

Don't change dependencies for a low tier framework

In D27616#618382, @apol wrote:

We can't add new dependencies to a tier1 framework, it would break the whole dependency system.

Just make it a qCWarning.

I opted for qDebug() for now, and will change once a logging category is set up for KGuiAddons.

apol added inline comments.Feb 27 2020, 1:12 AM
src/CMakeLists.txt
33 ↗(On Diff #76474)

Remove this change too. Also ki18n.

ahmadsamir updated this revision to Diff 76522.Feb 27 2020, 9:03 AM

Really revert some changes

apol accepted this revision.Feb 27 2020, 2:19 PM
This revision is now accepted and ready to land.Feb 27 2020, 2:19 PM
This revision was automatically updated to reflect the committed changes.