Yes this mail is an excellent description of the benefits of qCDebug.
Thanks David. I agree with everything.

Actually parallel to this I did a little merge request - also with a
minimal change to demonstrate category debugging. And (surprise) it
looks almost exactly like the changes here !!

However I put the debug macros in Debug.h - triggered by defining a
macro. Also I like David's variable names better than mine !

I will open a feature ticket for this - we can have this discussion there.

On 4/21/21 9:44 AM, David Faure wrote:
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.



_______________________________________________
Rosegarden-devel mailing list
Rosegarden-devel@lists.sourceforge.net  - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel

_______________________________________________
Rosegarden-devel mailing list
Rosegarden-devel@lists.sourceforge.net - use the link below to unsubscribe
https://lists.sourceforge.net/lists/listinfo/rosegarden-devel

Reply via email to