RE: [PATCH] MarbleDebug: don't use a class extending QIODevice as a null device

2017-04-19 Thread Kai Koehne


> -Original Message-
> From: Thiago Macieira [mailto:thi...@kde.org]
> Sent: Tuesday, April 18, 2017 6:36 PM
> To: Stefan Fuchs 
> Cc: Lubomir I. Ivanov ; Subsurface Mailing List
> ; Kai Koehne 
> Subject: Re: [PATCH] MarbleDebug: don't use a class extending QIODevice as
> a null device
> 
> Em terça-feira, 18 de abril de 2017, às 09:23:12 PDT, Stefan Fuchs escreveu:
> > /home/stefan/Entwicklung/Subsurface/marble-
> source/src/lib/marble/Marbl
> > eDebug
> > .cpp:27:53: error: passing 'const QLoggingCategory' as 'this' argument
> > of 'void QLoggingCategory::setEnabled(QtMsgType, bool)' discards
> > qualifiers [-fpermissive] loggingCategory().setEnabled(QtDebugMsg,
> > enabled);
> 
> Hold on. This one is weird, because how can you call that non-const method
> if everything is const?
> 
> Kai, how is one supposed to call QLoggingCategory::setEnabled()? Or are we
> not supposed to?
>
> Or should we skip the Q_LOGGING_CATEGORY macro and deploy our own
> code?

The general idea is that the logging category objects represent a view on the 
general logging configuration, and shouldn't be manipulated directly. That is, 
if you call setEnabled() on one object it will be only changing the exact 
object, and not another object that might represent the same category. Also, a 
change in the logging configuration might overwrite your local change at any 
time.

To avoid this, I suggest to change the logging rules either through custom 
logging rules (QLoggingCategory::setFilterRules(), QT_LOGGING_RULES, 
QT_LOGGING_CONF), or by installing a custom logging filter 
(QLoggingCategory::installFilter()). If you 'just' want to set a default 
logging level (so that e.g. QtDebugMsg messages are not printed for your 
categories) you can also set a level in your Q_LOGGING_CATEGORY macro call.

This is also all documented in 
http://doc.qt.io/qt-5/qloggingcategory.html#details . Please raise it if you 
feel that anything is unclear/missing there.

Regards

Kai
___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface


RE: [PATCH] MarbleDebug: don't use a class extending QIODevice as a null device

2017-04-20 Thread Kai Koehne
> -Original Message-
> From: Thiago Macieira [mailto:thi...@kde.org]
> [...]
> Sounds good, but do we need this setEnabled() in the first place? Is it used
> anywhere?

It's meant to be used by a filter installed by QLoggingCategory::installFilter 
(see the example http://doc.qt.io/qt-5/qloggingcategory.html#installFilter).

https://codereview.qt-project.org/#/c/192152/ aims to clarify this further in 
the documentation.

Regards

Kai

___
subsurface mailing list
subsurface@subsurface-divelog.org
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface