| davidedmundson | |
| ltoscano |
| Frameworks |
This change migrates libkcompactdisc to using modern logging for debugging purposes. It introduces a cd base category since a cd.playlist category existed already.
An additional cd.test category is introduced in the testkcd utility, which also enables logging on all cd categories.
Works as expected.
It appears to be necessary to enable debug output on the cd.test category explicitly, despite setting the QT_LOGGING_RULES env. var, probably because the tKCD instance is created before main() and thus qputenv() are called. I can probably avoid that by using the approach used in the Q_LOGGING_CATEGORY macro but I'm not certain that's worth the trouble?
| Lint Skipped |
| Unit Tests Skipped |
| Buildable 4792 | |
| Build 4810: arc lint + arc unit |
| src/kcompactdisc_p.cpp | ||
|---|---|---|
| 30 | there's a trend to use "kf5.frameworkname.subpart" as the category. | |
I strongly suggest to port to ecm_qt_declare_logging_category for both the old and the new category. This script can automated the process:
https://cgit.kde.org/kde-dev-scripts.git/tree/kf5/convert-qkdebug-to-qcdebug.sh
| tests/testkcd.cpp | ||
|---|---|---|
| 63 | Please don't set this here. It can be controlled at runtime. | |
| 68 | ditto | |
| src/kcompactdisc_p.cpp | ||
|---|---|---|
| 30 | On the other side, this is not a frameworks, but a generic library. I've seen (and pushed) org.kde.<application>[.<subpart>] for applications. What could it be a proper choice here? org.kde.lib.<name>[.<subpart>]? Or simply org.kde.<application>[.<subpart>]? | |
Some thoughts, I'll update the patch later today.
| src/kcompactdisc_p.cpp | ||
|---|---|---|
| 30 | I see KCalCore does ecm_qt_declare_logging_category(kcalcore_LIB_SRCS HEADER kcalcore_debug.h IDENTIFIER KCALCORE_LOG CATEGORY_NAME org.kde.pim.kcalcore). Note the absence of a lib namespace/whatever. | |
| tests/testkcd.cpp | ||
| 63 | I know it can, and I'd agree for regular applications. This is a test application, its users should not have to go figure out why it produces no output and how to make it print something. You're right though that QT_LOGGING_RULES can already have been set, I can avoid changing it. | |
| 68 | And ditto: why would you write an application that's supposed to print test results and NOT enable the printing by default? | |
| tests/testkcd.cpp | ||
|---|---|---|
| 63 | If the output is mandatory, why use a category rule at all, or even logging? (and I know this is a pre-existing problem, but still) | |
Simply because there's no quick and convenient other way for doing output in Qt, I think. But even then, if QPrint existed you'd be using it in testkcd but most likely not in the library itself and I'd still propose to activate debug output from the library in testkcd.
FWIW, the autotests I've run by hand all seem to generate output that looks suspiciously as if it's generated by the logging classes, and they don't require setting a rule.
Information printed by command-line programs can be done with qInfo(), which is enabled by default, while qDebug() should be disabled by defualt.
Using qInfo() in testkcd. Curious that qInfo() is enabled (and isn't disabled by *.info=false in qtlogging.ini) by default but qCInfo() isn't (as you'd expect for "info level" logging).
For information, qCInfo() is enabled by default if you use the ECM macro for declaring the logging category, or if you pass QtInfoMsg to the Qt macro. But then again, it seems fine with me to use qInfo() in a test program, no need for categorized output for this.
BTW it is *not* curious that isn't qInfo isn't "disabled by *.info=false in qtlogging.ini", it's rather expected since qInfo() is one layer below categorized logging, just like qDebug and qWarning. qtlogging.ini (and the QT_LOGGING_RULES env vars) are about qC*.
| tests/testkcd.cpp | ||
|---|---|---|
| 64 | There's proper API for setting applications defaults, without the need for env vars. | |
BTW it is *not* curious that isn't qInfo isn't "disabled by *.info=false in qtlogging.ini", it's rather expected since qInfo() is one layer below categorized logging, just like qDebug and qWarning. qtlogging.ini (and the QT_LOGGING_RULES env vars) are about qC*.
I've read that before, yet I notice that qDebug() statements don't print for me if I have *.debug=false in my qtlogging.ini file. Whatever the true reason, I've taken to using qWarning() in my own tests before learning about qInfo() (I have *.warning=true set in qtlogging.ini).
Regardless of that, I meant that AFAIU one typically enables info level messages when even more verbose output than debug output is required. The qC* categories seem to reflect that, IMHO.
There's proper API for setting applications defaults, without the need for env vars.
I only see QLoggingCategory::setFilterRules(), is that what you're referring to? I don't think it'd be proper to use QSettings for a test application, leaving a persistent resource somewhere.
It is rather hard to follow your reasoning...
Yes QSettings is persistent, but QLoggingCategory::setFilterRules() definitely isn't.
You're right about the first point though, qDebug() is associated to the "default" category of qCDebug(), so it's affected by *.debug=false, I hadn't realized that. Surely it must be the same for qInfo() then -- except that *.info=false would be a very stupid thing to do anyway, IMHO. One *wants* the output from command-line tools.