Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/#review51972 --- This review has been submitted with commit eea8251903f427f02452f390374335297afe0e09 by David Faure to branch master. - Commit Hook On Feb. 23, 2014, 11 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- (Updated Feb. 23, 2014, 11 a.m.) Review request for KDE Frameworks and Albert Astals Cid. Repository: kconfigwidgets Description --- 3 commits: Unittest: make errors readable -- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. REVIEW: 115959 -- Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - autotests/kconfigdialog_unittest.cpp e5322c1782c2a68c15451777066e28a9b7afea23 src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/#review51415 --- The implementation all looks fine. The only concern I have is that it's an odd location for it; I wouldn't expect to go looking for a method to invoke Help in KConfigWidgets. Although I'm not sure where it would go instead, given the reliance on QtGui and KConfig. Although, maybe is could go in KConfigGui? It's still a bit out of place, but it sort of fits in KConfig if you think of it as accessing a bit of system/application configuration (where to find help). src/khelpclient.cpp https://git.reviewboard.kde.org/r/115959/#comment36209 It might be worth documenting that this is what the anchor argument does, for apps that want to point to a website. - Alex Merry On Feb. 23, 2014, 11 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- (Updated Feb. 23, 2014, 11 a.m.) Review request for KDE Frameworks and Albert Astals Cid. Repository: kconfigwidgets Description --- 3 commits: Unittest: make errors readable -- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. REVIEW: 115959 -- Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - autotests/kconfigdialog_unittest.cpp e5322c1782c2a68c15451777066e28a9b7afea23 src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
On March 1, 2014, 11:15 a.m., Alex Merry wrote: The implementation all looks fine. The only concern I have is that it's an odd location for it; I wouldn't expect to go looking for a method to invoke Help in KConfigWidgets. Although I'm not sure where it would go instead, given the reliance on QtGui and KConfig. Although, maybe is could go in KConfigGui? It's still a bit out of place, but it sort of fits in KConfig if you think of it as accessing a bit of system/application configuration (where to find help). Yeah I thought about that alternative. We already abuse KConfigGui for kstandardshortcut, kwindowconfig, etc. -- i.e. things that are there for dependencies reasons, not because they are in the expected feature list for the KConfig framework. So I tried to not keep abusing it... One scary way to look at the issue is: what if one day we want to replace KConfig with another configuration technology? Then all that stuff we bundle into KConfigGui (i.e. into the KConfig framework itself), will be in the wrong place, since we'll want these APIs to keep existing, just not with kconfig as the underlying technology. (OTOH KConfigWidgets is a different framework, it's widgets with a need for configuration, doesn't have to be tied to KConfig as the underlying implementation) I know, I'm saying here that we did it wrong for kstandardshortcut and kwindowconfig, where I was probably the one selecting the current situation... I also don't honestly think that moving away from KConfig is a plan (but rather providing any new technology from within the KConfig API instead). I picked KConfigWidgets using the same logic as to why it was in xmlgui before: because that's where it's needed, at the lowest level. I can be convinced for KConfigGui, but it's not ideal either, for the reasons above. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/#review51415 --- On Feb. 23, 2014, 11 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- (Updated Feb. 23, 2014, 11 a.m.) Review request for KDE Frameworks and Albert Astals Cid. Repository: kconfigwidgets Description --- 3 commits: Unittest: make errors readable -- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. REVIEW: 115959 -- Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - autotests/kconfigdialog_unittest.cpp e5322c1782c2a68c15451777066e28a9b7afea23 src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/#review51426 --- Ship it! Ship It! - Alex Merry On Feb. 23, 2014, 11 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- (Updated Feb. 23, 2014, 11 a.m.) Review request for KDE Frameworks and Albert Astals Cid. Repository: kconfigwidgets Description --- 3 commits: Unittest: make errors readable -- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. REVIEW: 115959 -- Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - autotests/kconfigdialog_unittest.cpp e5322c1782c2a68c15451777066e28a9b7afea23 src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
On March 1, 2014, 11:15 a.m., Alex Merry wrote: The implementation all looks fine. The only concern I have is that it's an odd location for it; I wouldn't expect to go looking for a method to invoke Help in KConfigWidgets. Although I'm not sure where it would go instead, given the reliance on QtGui and KConfig. Although, maybe is could go in KConfigGui? It's still a bit out of place, but it sort of fits in KConfig if you think of it as accessing a bit of system/application configuration (where to find help). David Faure wrote: Yeah I thought about that alternative. We already abuse KConfigGui for kstandardshortcut, kwindowconfig, etc. -- i.e. things that are there for dependencies reasons, not because they are in the expected feature list for the KConfig framework. So I tried to not keep abusing it... One scary way to look at the issue is: what if one day we want to replace KConfig with another configuration technology? Then all that stuff we bundle into KConfigGui (i.e. into the KConfig framework itself), will be in the wrong place, since we'll want these APIs to keep existing, just not with kconfig as the underlying technology. (OTOH KConfigWidgets is a different framework, it's widgets with a need for configuration, doesn't have to be tied to KConfig as the underlying implementation) I know, I'm saying here that we did it wrong for kstandardshortcut and kwindowconfig, where I was probably the one selecting the current situation... I also don't honestly think that moving away from KConfig is a plan (but rather providing any new technology from within the KConfig API instead). I picked KConfigWidgets using the same logic as to why it was in xmlgui before: because that's where it's needed, at the lowest level. I can be convinced for KConfigGui, but it's not ideal either, for the reasons above. OK, if we just work on the basis that KConfigWidgets is misleadingly named (not that a better name occurs to me), and is essentially the tier 3 version of KWidgetsAddons, that seems fine. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/#review51415 --- On Feb. 23, 2014, 11 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- (Updated Feb. 23, 2014, 11 a.m.) Review request for KDE Frameworks and Albert Astals Cid. Repository: kconfigwidgets Description --- 3 commits: Unittest: make errors readable -- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. REVIEW: 115959 -- Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - autotests/kconfigdialog_unittest.cpp e5322c1782c2a68c15451777066e28a9b7afea23 src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
On Feb. 24, 2014, 9:22 p.m., Albert Astals Cid wrote:
src/khelpclient.cpp, line 35
https://git.reviewboard.kde.org/r/115959/diff/2/?file=245608#file245608line35
Just curious, who is doing this call setApplicationName() with the
name of the desktop file thing? I know KApplication did it but we're now
not recommending to use it. Or is this something every app developer has to
do? Is it documented somewhere?
I implemented in Qt 5.0 that applicationName() now defaults to the name of the
binary on disk. This takes care of 95% of the cases :-)
For the other cases, there's setApplicationName (as documented in the porting
doc when porting away from kapp, among other things), or very soon it will also
be done by KAboutData::setApplicationData(myAboutData) (there's a RR pending
for it).
On Feb. 24, 2014, 9:22 p.m., Albert Astals Cid wrote:
src/khelpclient.cpp, line 76
https://git.reviewboard.kde.org/r/115959/diff/2/?file=245608#file245608line76
url is always help:/ isn't it? Not sure i understand the comment
? Not sure I understand *your* comment :-)
QUrl url;
if (!docPath.isEmpty()) {
url =
QUrl(QLatin1String(help:/)).resolved(QUrl::fromUserInput(docPath));
} else {
url = QUrl(QString::fromLatin1(help:/%1/index.html).arg(appname));
}
if (!anchor.isEmpty()) {
QUrlQuery query(url);
query.addQueryItem(QString::fromLatin1(anchor), anchor);
url.setQuery(query);
}
How is this always help:/ ? There much stuff after that, in the path and
possibly in the query.
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115959/#review50765
---
On Feb. 23, 2014, 11 a.m., David Faure wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115959/
---
(Updated Feb. 23, 2014, 11 a.m.)
Review request for KDE Frameworks and Albert Astals Cid.
Repository: kconfigwidgets
Description
---
3 commits:
Unittest: make errors readable
--
Resurrect KConfigDialog::setHelp (used to come from KDialog).
It controls the default behavior of showHelp(), which is implemented
using KHelpClient.
REVIEW: 115959
--
Move KHelpClient down from kxmlgui, for use in KConfigDialog.
Diffs
-
autotests/kconfigdialog_unittest.cpp
e5322c1782c2a68c15451777066e28a9b7afea23
src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36
src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280
src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405
src/khelpclient.h PRE-CREATION
src/khelpclient.cpp PRE-CREATION
Diff: https://git.reviewboard.kde.org/r/115959/diff/
Testing
---
Compiled all of KF5.
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
On Feb. 24, 2014, 9:22 p.m., Albert Astals Cid wrote:
src/khelpclient.cpp, line 76
https://git.reviewboard.kde.org/r/115959/diff/2/?file=245608#file245608line76
url is always help:/ isn't it? Not sure i understand the comment
David Faure wrote:
? Not sure I understand *your* comment :-)
QUrl url;
if (!docPath.isEmpty()) {
url =
QUrl(QLatin1String(help:/)).resolved(QUrl::fromUserInput(docPath));
} else {
url =
QUrl(QString::fromLatin1(help:/%1/index.html).arg(appname));
}
if (!anchor.isEmpty()) {
QUrlQuery query(url);
query.addQueryItem(QString::fromLatin1(anchor), anchor);
url.setQuery(query);
}
How is this always help:/ ? There much stuff after that, in the path
and possibly in the query.
Albert Astals Cid wrote:
I mean starts always with help:/, sorry. And if it always starts with
help, the comment
// launch khelpcenter, or a browser for URIs not handled by khelpcenter
is a bit weird, since it's always the same kind of URIs, no?
Ah, I see.
No, the docPath (from the .desktop file) can be an absolute URL, and something
like QUrl(help:/).resolved(http://www.kde.org;) gives http://www.kde.org;.
(the point of the call to resolved is to handle relative urls, but it also
handles absolute urls by just returning that).
- David
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115959/#review50765
---
On Feb. 23, 2014, 11 a.m., David Faure wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115959/
---
(Updated Feb. 23, 2014, 11 a.m.)
Review request for KDE Frameworks and Albert Astals Cid.
Repository: kconfigwidgets
Description
---
3 commits:
Unittest: make errors readable
--
Resurrect KConfigDialog::setHelp (used to come from KDialog).
It controls the default behavior of showHelp(), which is implemented
using KHelpClient.
REVIEW: 115959
--
Move KHelpClient down from kxmlgui, for use in KConfigDialog.
Diffs
-
autotests/kconfigdialog_unittest.cpp
e5322c1782c2a68c15451777066e28a9b7afea23
src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36
src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280
src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405
src/khelpclient.h PRE-CREATION
src/khelpclient.cpp PRE-CREATION
Diff: https://git.reviewboard.kde.org/r/115959/diff/
Testing
---
Compiled all of KF5.
Thanks,
David Faure
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/#review50765 --- src/khelpclient.cpp https://git.reviewboard.kde.org/r/115959/#comment35626 Just curious, who is doing this call setApplicationName() with the name of the desktop file thing? I know KApplication did it but we're now not recommending to use it. Or is this something every app developer has to do? Is it documented somewhere? src/khelpclient.cpp https://git.reviewboard.kde.org/r/115959/#comment35624 url is always help:/ isn't it? Not sure i understand the comment - Albert Astals Cid On Feb. 23, 2014, 11 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- (Updated Feb. 23, 2014, 11 a.m.) Review request for KDE Frameworks and Albert Astals Cid. Repository: kconfigwidgets Description --- 3 commits: Unittest: make errors readable -- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. REVIEW: 115959 -- Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - autotests/kconfigdialog_unittest.cpp e5322c1782c2a68c15451777066e28a9b7afea23 src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- Review request for KDE Frameworks. Repository: kconfigwidgets Description --- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- (Updated Feb. 23, 2014, 9:56 a.m.) Review request for KDE Frameworks and Albert Astals Cid. Repository: kconfigwidgets Description --- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 115959: Resurrect KConfigDialog::setHelp (used to come from KDialog). Move KHelpClient down from kxmlgui, for use in KConfigDialog.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/#review50567 --- src/khelpclient.h https://git.reviewboard.kde.org/r/115959/#comment35567 There is no such parameter? - Kai Uwe Broulik On Feb. 23, 2014, 9:56 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115959/ --- (Updated Feb. 23, 2014, 9:56 a.m.) Review request for KDE Frameworks and Albert Astals Cid. Repository: kconfigwidgets Description --- Resurrect KConfigDialog::setHelp (used to come from KDialog). It controls the default behavior of showHelp(), which is implemented using KHelpClient. Move KHelpClient down from kxmlgui, for use in KConfigDialog. Diffs - src/CMakeLists.txt 7da7fba0c15153d6dee381c2b8f282e9837eae36 src/kconfigdialog.h b06efc588c772ed655d581a0e021d92af5e0e280 src/kconfigdialog.cpp 8db48e23f614530cef11a23a182b50d905327405 src/khelpclient.h PRE-CREATION src/khelpclient.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/115959/diff/ Testing --- Compiled all of KF5. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
