On lundi 19 avril 2021 02:54:18 CEST Ted Felix wrote:
> On 4/18/21 3:48 PM, Philip Leishman wrote:
> > I still think qCDebug is a good way to go. I just don't know how to do
> > it without changing many hundreds of lines of code in almost all files.
>
> Before we embark on this project, I would like to see a proof of
> concept that shows that we can correctly preserve the behavior of the
> current RG_* macros with whatever the replacement is.
Sounds good, feature comparison is always a good plan before going further.
> Here are the requirements based on the existing macros:
>
> RG_DEBUG happens only in a debug build. It can be turned off with
> RG_NO_DEBUG_PRINT.
qDebug/qCDebug can be turned off at compile time with QT_NO_DEBUG_OUTPUT,
which we could decide to set in release builds.
On the other hand, given the ability to define all categories as "off by
default", it's customary to not ever set that, so that you can ask an end user
with a hard to reproduce problem, to enable some debug category and send you
the output. But that's your choice, we can certainly replicate the current
solution if you prefer.
More importantly than what happens in release builds:
while we can certainly replicate "RG_DEBUG is on by default", the main benefit
of qCDebug is to let that be OFF by default, with an easy way (config file or
environment variable) to turn ON specific categories. I recommend going that
way, otherwise there's no point in changing anything.
> RG_WARNING happens in both builds and is always on by default.
Yes.
> RG_INFO happens only in a debug build and is always on by default.
> (This one is barely used at all and can probably be dropped.)
qCInfo exists as well, but usually the point is that it happens in both debug
and release (and is on by default indeed). It's useful for stuff like printing
"Application already running, exiting" on stderr at launch. You want "release
mode" users to see that too.
Otherwise it would be pretty much like qCDebug/RG_DEBUG...
Of course for this use case std::cerr does the job too, it's just more painful
to use with QString.
> The proof of concept could easily be run from main(). It already
> issues RG_INFO and RG_DEBUG messages. It would just need a test
> RG_WARNING added:
>
> RG_WARNING << "main(): testing RG_WARNING";
>
> Then replace everything in main.cpp with the new approach and see if
> it can meet my requirements above.
OK, here you go.
The attached diff defines RG_DEBUG and similar at the top of the file, so the
debug statements don't have to be changed. This makes the diff minimal to
read, but it requires a bunch of #defines on top, and doesn't use standard Qt
API, so I was rather thinking of a one-time search/replace in the file.
A benefit of keeping those defines however is that it makes it easy to move
code between files, or to write "RG_DEBUG" without thinking.
The downside of RG_DEBUG however is that it forces to keep the current idea of
"one category per file", while categories are even more useful when they are
orthogonal to files.
(The #undef are because of the current Debug.h included by other headers, they
would go away once Debug.h goes away)
Output of the program, using a pretty basic QT_MESSAGE_PATTERN set to
%{category} %{if-info}INFO: %{endif}%{if-warning}WARNING: %{endif}%{message}
$ ./rosegarden
rosegarden.main INFO: Unbundling examples...
rosegarden.main INFO: Unbundling templates...
rosegarden.main INFO: Unbundling libraries (device files)...
rosegarden.main INFO: Creating RosegardenMainWindow instance...
rosegarden.main WARNING: ABORTING THE PROGRAM HERE, THIS IS JUST A TEST...
And to see all debug output:
$ export QT_LOGGING_RULES="rosegarden.*=true"
$ ./rosegarden
rosegarden.main System Locale: "en_US"
rosegarden.main Qt translations path: "/d/qt/5/inst/translations"
rosegarden.main Qt translations loaded successfully.
rosegarden.main RG Translation: trying to load :locale/ "en_US"
rosegarden.main RG Translations loaded successfully.
rosegarden.main Loaded application icon " "rg-rwb-rose3-16x16" "
rosegarden.main Loaded application icon " "rg-rwb-rose3-32x32" "
rosegarden.main Loaded application icon " "rg-rwb-rose3-48x48" "
rosegarden.main Loaded application icon " "rg-rwb-rose3-64x64" "
rosegarden.main Loaded application icon " "rg-rwb-rose3-128x128" "
rosegarden.main INFO: Unbundling examples...
rosegarden.main INFO: Unbundling templates...
rosegarden.main INFO: Unbundling libraries (device files)...
rosegarden.main INFO: Creating RosegardenMainWindow instance...
rosegarden.main WARNING: ABORTING THE PROGRAM HERE, THIS IS JUST A TEST...
> Once I see that, I will be more likely to be on board. Otherwise I
> would rather that we leave things as-is. The current RG_* macros work
> fine for me and I see no benefit to changing them. All I see is time
> wasted fiddling with debug logging that already works.
Well, sure, if the goal is to use qCDebug to do *exactly the same thing* as
the current solution, then nothing is gained, and my time is better spent
elsewhere.
IMHO the biggest benefit is to cut down on all the "noise" by default and let
people enable the debug categories they're interested in.
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Working on KDE Frameworks 5
diff --git i/src/gui/application/main.cpp w/src/gui/application/main.cpp
index 349141327..1fa8608d8 100644
--- i/src/gui/application/main.cpp
+++ w/src/gui/application/main.cpp
@@ -13,11 +13,8 @@
COPYING included with this distribution for more information.
*/
-#define RG_MODULE_STRING "[main]"
-
#include "misc/ConfigGroups.h"
#include "misc/Strings.h"
-#include "misc/Debug.h"
#include "gui/application/RosegardenMainWindow.h"
#include "document/RosegardenDocument.h"
#include "gui/widgets/StartupLogo.h"
@@ -56,6 +53,7 @@
#include <QtGui>
#include <QPixmapCache>
#include <QStringList>
+#include <QLoggingCategory>
#include <sound/SoundDriverFactory.h>
#include <sys/time.h>
@@ -326,6 +324,15 @@ The main Sequencer state machine is a good starting point and clearly
visible at the bottom of rosegarden/sequencer/main.cpp.
*/
+
+Q_LOGGING_CATEGORY(DEBUG_MAIN, "rosegarden.main", QtInfoMsg)
+#undef RG_DEBUG
+#define RG_DEBUG qCDebug(DEBUG_MAIN)
+#undef RG_WARNING
+#define RG_WARNING qCWarning(DEBUG_MAIN)
+#undef RG_INFO
+#define RG_INFO qCInfo(DEBUG_MAIN)
+
// -----------------------------------------------------------------
static void usage()
@@ -608,6 +615,8 @@ int main(int argc, char *argv[])
SoundDriverFactory::setSoundEnabled(!nosound);
RG_INFO << "Creating RosegardenMainWindow instance...";
+ RG_WARNING << "ABORTING THE PROGRAM HERE, THIS IS JUST A TEST...";
+ return 1;
RosegardenMainWindow *mainWindow =
new RosegardenMainWindow(!nosound, startLogo);
_______________________________________________
Rosegarden-devel mailing list
Rosegarden-devel@lists.sourceforge.net - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel
