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

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

2017-04-19 Thread Thiago Macieira 
On quarta-feira, 19 de abril de 2017 03:45:46 PDT Lubomir I. Ivanov wrote:
> void MarbleDebug::setEnabled(bool enabled)
> {
> QLoggingCategory::setFilterRules(QString("marble.debug=%1").arg(enabled
> ? "true", "false"));
> }
> 
> and likely remove isEnabled() as it's not needed?

Sounds good, but do we need this setEnabled() in the first place? Is it used 
anywhere?

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
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-19 Thread Kai Koehne 


> -Original Message-
> From: Thiago Macieira [mailto:thi...@kde.org]
> Sent: Tuesday, April 18, 2017 6:36 PM
> To: Stefan Fuchs <sfu...@gmx.de>
> Cc: Lubomir I. Ivanov <neolit...@gmail.com>; Subsurface Mailing List
> <subsurface@subsurface-divelog.org>; Kai Koehne <kai.koe...@qt.io>
> 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-18 Thread Thiago Macieira 
Em terça-feira, 18 de abril de 2017 00:08:57 PDT você escreveu:
> I will try it this morning.
> Two remaining questions:
> - What is the default state now if we don't set it explicitly? Enabled true
> or false? 

Depends on the argument passed in the macro. With QtDebugMsg, it defaults to 
enabled; with QtWarningMsg, it defaults to disabled.

> - Where would be the right place in Subsurface code to put the
> MarbleDebug::setEnabled(false) ?

If we want it disabled by default, just change the macro.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
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-18 Thread Thiago Macieira 
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/MarbleDebug
> .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?

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
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-18 Thread Stefan Fuchs 
Hello Lubomir,


Am 18.04.2017 um 16:00 schrieb Lubomir I. Ivanov:
> i must admit i followed Thiago's suggestions blindly.
> Thiago, do you have an idea why this happens?
>
> the macro:
> #define mDebug qCDebug(Marble::loggingCategory)
>
> expands calls like this one:
> mDebug() << "innerRing: size" << p()->inner.size();
>
> to:
> qCDebug(Marble::loggingCategory)() << "innerRing: size" <<
> p()->inner.size();
>
> is it complaining about the extra()?
>
> Yes! Yes! Thats it ;-)
>
> Changed to:
> #define mDebug() qCDebug(Marble::loggingCategory)
>
> and it compiles.
Good news: Some parts work if I comment out what still gives an error
(see below). Logging works but is always on for marble.

Bad news:
The function
void MarbleDebug::setEnabled(bool enabled)
{
loggingCategory().setEnabled(QtDebugMsg, enabled);
}
still doesn' compile. Error:
/home/stefan/Entwicklung/Subsurface/marble-source/src/lib/marble/MarbleDebug.cpp:27:53:
error: passing 'const QLoggingCategory' as 'this' argument of 'void
QLoggingCategory::setEnabled(QtMsgType, bool)' discards qualifiers
[-fpermissive]
 loggingCategory().setEnabled(QtDebugMsg, enabled);

Also this version:
void MarbleDebug::setEnabled(bool enabled)
{
loggingCategory.setEnabled(QtDebugMsg, enabled);
}
doesn't compile. Error:
/home/stefan/Entwicklung/Subsurface/marble-source/src/lib/marble/MarbleDebug.cpp:
In static member function 'static void
Marble::MarbleDebug::setEnabled(bool)':
/home/stefan/Entwicklung/Subsurface/marble-source/src/lib/marble/MarbleDebug.cpp:27:21:
error: request for member 'setEnabled' in 'Marble::loggingCategory',
which is of non-class type 'const QLoggingCategory&()'
 loggingCategory.setEnabled(QtDebugMsg, enabled);


Stefan

-- 

Stefan Fuchs
E-Mail: sfu...@gmx.de 

___
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-18 Thread Lubomir I. Ivanov 
On 18 April 2017 at 16:47, Stefan Fuchs  wrote:
> Hello Lubomir,
>
> Am 18.04.2017 um 13:22 schrieb Lubomir I. Ivanov:
>
> here are the updated Marble files and a patch diff for review.
> i can't build Marble to test these changes ATM though.
>
> Looks good, so long as "Marble::category" isn't too generic a name.
>
> ok, i've renamed it to Marable::loggingCategory.
>
> Stefan, could you please try building and running the attached patch?
> you can toggle the debugging with MarbleDebug::setEnabled(true/false)
> e.g. from the Subsurface source code.
>
> Sorry, I'm once again lost...
> I have multiple compile issues like this:
>
> /home/stefan/Entwicklung/Subsurface/marble-source/src/lib/marble/geodata/data/GeoDataPolygon.cpp:
> In member function 'virtual void Marble::GeoDataPolygon::pack(QDataStream&)
> const':
> /home/stefan/Entwicklung/Subsurface/marble-source/src/lib/marble/geodata/data/GeoDataPolygon.cpp:174:16:
> error: no match for call to '(QDebug) ()'
>  mDebug() << "innerRing: size" << p()->inner.size();
>
> Also for:
> https://github.com/Subsurface-divelog/marble/blob/Subsurface-branch/src/lib/marble/geodata/data/GeoDataVec2.cpp#L56
> https://github.com/Subsurface-divelog/marble/blob/Subsurface-branch/src/lib/marble/geodata/data/GeoDataMultiTrack.cpp#L115
> and many others.
>
> But all these files do
> #include "MarbleDebug.h"
>

i must admit i followed Thiago's suggestions blindly.
Thiago, do you have an idea why this happens?

the macro:
#define mDebug qCDebug(Marble::loggingCategory)

expands calls like this one:
mDebug() << "innerRing: size" << p()->inner.size();

to:
qCDebug(Marble::loggingCategory)() << "innerRing: size" << p()->inner.size();

is it complaining about the extra()?

lubomir
--
___
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-18 Thread Dirk Hohndel 
On Tue, Apr 18, 2017 at 02:22:52PM +0300, Lubomir I. Ivanov wrote:
> On 18 April 2017 at 10:08, Stefan Fuchs  wrote:
> > Hello Lubomir,
> >
> >
> > Am 17.04.2017 um 23:53 schrieb Lubomir I. Ivanov:
> >
> > here are the updated Marble files and a patch diff for review.
> > i can't build Marble to test these changes ATM though.
> >
> > Looks good, so long as "Marble::category" isn't too generic a name.
> >
> > ok, i've renamed it to Marable::loggingCategory.
> >
> > Stefan, could you please try building and running the attached patch?
> > you can toggle the debugging with MarbleDebug::setEnabled(true/false)
> > e.g. from the Subsurface source code.
> >
> > I will try it this morning.
> > Two remaining questions:
> > - What is the default state now if we don't set it explicitly? Enabled true
> > or false?
> > - Where would be the right place in Subsurface code to put the
> > MarbleDebug::setEnabled(false) ?
> >
> >
> 
> we already do that in desktop-widgets/globe.cpp:
> Marble::MarbleDebug::setEnabled(verbose);

Once this is tested and works as expected, I'd appreciate a pull request.
I'm super busy at work the next couple of days and prone to miss stuff
otherwise.

Thanks

/D
___
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-18 Thread Stefan Fuchs 
Hello Lubomir,


Am 17.04.2017 um 23:53 schrieb Lubomir I. Ivanov:
>>> here are the updated Marble files and a patch diff for review.
>>> i can't build Marble to test these changes ATM though.
>> Looks good, so long as "Marble::category" isn't too generic a name.
>>
> ok, i've renamed it to Marable::loggingCategory.
>
> Stefan, could you please try building and running the attached patch?
> you can toggle the debugging with MarbleDebug::setEnabled(true/false)
> e.g. from the Subsurface source code.
I will try it this morning.
Two remaining questions:
- What is the default state now if we don't set it explicitly? Enabled
true or false?
- Where would be the right place in Subsurface code to put the
MarbleDebug::setEnabled(false) ?

Best regards
Stefan

-- 

Stefan Fuchs
E-Mail: sfu...@gmx.de 

___
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-17 Thread Thiago Macieira 
On segunda-feira, 17 de abril de 2017 14:53:12 PDT Lubomir I. Ivanov wrote:
> Stefan, could you please try building and running the attached patch?
> you can toggle the debugging with MarbleDebug::setEnabled(true/false)
> e.g. from the Subsurface source code.

Or by setting in your environment:

QT_LOGGING_RULES=marble.debug=true

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
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-17 Thread Thiago Macieira 
On segunda-feira, 17 de abril de 2017 14:41:13 PDT Lubomir I. Ivanov wrote:
> On 18 April 2017 at 00:18, Thiago Macieira  wrote:
> > On segunda-feira, 17 de abril de 2017 14:10:28 PDT Lubomir I. Ivanov 
wrote:
> >> i guess we could just do a:
> >> void MarbleDebug::setEnabled(bool enabled)
> >> {
> >> 
> >> Marble::loggingCategory.setEnabled(enabled);
> >> MarbleDebug::m_enabled = enabled;
> >> 
> >> }
> > 
> > I'd go a little further and drop the m_enabled variable completely. You
> > don't need it, let the bit in the QLoggingCategory variable keep the
> > state.
> > 
> > You'd have:
> > 
> > namespace MarbleDebug {
> > Q_LOGGING_CATEGORY(category, "marble", QtWarningMsg)
> > 
> > void setEnabled(bool enabled)
> > {
> > 
> > category.setEnabled(QtDebugMsg, enabled);
> > 
> > }
> > 
> > bool isEnabled()// do you even need this function?
> > {
> > 
> > return category.isDebugEnabled();
> > 
> > }
> > } //namespace MarbleDebug
> > 
> > This changes to the MarbleDebug namespace too.
> 
> thanks, i think i understand.
> 
> here are the updated Marble files and a patch diff for review.
> i can't build Marble to test these changes ATM though.

Looks good, so long as "Marble::category" isn't too generic a name.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
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-17 Thread Lubomir I. Ivanov 
On 18 April 2017 at 00:18, Thiago Macieira  wrote:
> On segunda-feira, 17 de abril de 2017 14:10:28 PDT Lubomir I. Ivanov wrote:
>> i guess we could just do a:
>> void MarbleDebug::setEnabled(bool enabled)
>> {
>> Marble::loggingCategory.setEnabled(enabled);
>> MarbleDebug::m_enabled = enabled;
>> }
>
> I'd go a little further and drop the m_enabled variable completely. You don't
> need it, let the bit in the QLoggingCategory variable keep the state.
>
> You'd have:
>
> namespace MarbleDebug {
> Q_LOGGING_CATEGORY(category, "marble", QtWarningMsg)
>
> void setEnabled(bool enabled)
> {
> category.setEnabled(QtDebugMsg, enabled);
> }
>
> bool isEnabled()// do you even need this function?
> {
> return category.isDebugEnabled();
> }
> } //namespace MarbleDebug
>
> This changes to the MarbleDebug namespace too.
>

thanks, i think i understand.

here are the updated Marble files and a patch diff for review.
i can't build Marble to test these changes ATM though.

lubomir
--
From 79005f97063620f4f7da0d087a045f4f6da02d70 Mon Sep 17 00:00:00 2001
From: "Lubomir I. Ivanov" 
Date: Tue, 18 Apr 2017 00:36:17 +0300
Subject: [PATCH] MarbleDebug: use a custom QLoggingCategory

---
 src/lib/marble/MarbleDebug.cpp | 22 --
 src/lib/marble/MarbleDebug.h   | 12 +---
 2 files changed, 9 insertions(+), 25 deletions(-)

diff --git a/src/lib/marble/MarbleDebug.cpp b/src/lib/marble/MarbleDebug.cpp
index 5f53f6f..7ee9a4f 100644
--- a/src/lib/marble/MarbleDebug.cpp
+++ b/src/lib/marble/MarbleDebug.cpp
@@ -10,35 +10,21 @@
 
 #include "MarbleDebug.h"
 
-#include 
-#include 
+#include 
 
 namespace Marble
 {
-// bool MarbleDebug::m_enabled = true;
-bool MarbleDebug::m_enabled = false;
 
-QDebug mDebug()
-{
-if ( MarbleDebug::isEnabled() ) {
-return QDebug( QtDebugMsg );
-}
-else {
-static QFile *nullDevice = new QFile(QProcess::nullDevice());
-if ( !nullDevice->isOpen() )
- nullDevice->open(QIODevice::WriteOnly);
- return QDebug( nullDevice );
-}
-}
+Q_LOGGING_CATEGORY(category, "marble", QtDebugMsg)
 
 bool MarbleDebug::isEnabled()
 {
-return MarbleDebug::m_enabled;
+return category.isDebugEnabled();
 }
 
 void MarbleDebug::setEnabled(bool enabled)
 {
-MarbleDebug::m_enabled = enabled;
+category.setEnabled(QtDebugMsg, enabled);
 }
 
 } // namespace Marble
diff --git a/src/lib/marble/MarbleDebug.h b/src/lib/marble/MarbleDebug.h
index 74b71ae..03b2db4 100644
--- a/src/lib/marble/MarbleDebug.h
+++ b/src/lib/marble/MarbleDebug.h
@@ -12,12 +12,15 @@
 #define MARBLE_MARBLEDEBUG_H
 
 #include 
+#include 
 
 #include "marble_export.h"
 
 namespace Marble
 {
 
+Q_DECLARE_LOGGING_CATEGORY(category)
+
 /**
   * a class which takes all the settings and exposes them
   */
@@ -35,15 +38,10 @@ public:
  */
 static void setEnabled(bool enabled);
 
-private:
-static bool m_enabled;
-
 };
 
-/**
-  * a function to replace qDebug() in Marble library code
-  */
-MARBLE_EXPORT QDebug mDebug();
+// logging category
+#define mDebug qCDebug(Marble::category)
 
 } // namespace Marble
 
-- 
1.7.11.msysgit.0

//
// This file is part of the Marble Virtual Globe.
//
// This program is free software licensed under the GNU LGPL. You can
// find a copy of this license in LICENSE.txt in the top directory of
// the source code.
//
// Copyright 2009  Patrick Spendrin 
//

#include "MarbleDebug.h"

#include 

namespace Marble
{

Q_LOGGING_CATEGORY(category, "marble", QtDebugMsg)

bool MarbleDebug::isEnabled()
{
return category.isDebugEnabled();
}

void MarbleDebug::setEnabled(bool enabled)
{
category.setEnabled(QtDebugMsg, enabled);
}

} // namespace Marble
//
// This file is part of the Marble Virtual Globe.
//
// This program is free software licensed under the GNU LGPL. You can
// find a copy of this license in LICENSE.txt in the top directory of
// the source code.
//
// Copyright 2009  Patrick Spendrin 
//

#ifndef MARBLE_MARBLEDEBUG_H
#define MARBLE_MARBLEDEBUG_H

#include 
#include 

#include "marble_export.h"

namespace Marble
{

Q_DECLARE_LOGGING_CATEGORY(category)

/**
  * a class which takes all the settings and exposes them
  */
class MARBLE_EXPORT MarbleDebug
{
public:
/**
 * @brief isEnabled returns whether debug information output is generated
 */
static bool isEnabled();

/**
 * @brief setEnabled Toggle debug information output generation
 * @param enabled Set to true to enable debug output, false to disable
 */
static void setEnabled(bool enabled);

};

// logging category
#define mDebug qCDebug(Marble::category)

} // namespace Marble

#endif
___
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-17 Thread Thiago Macieira 
On segunda-feira, 17 de abril de 2017 14:10:28 PDT Lubomir I. Ivanov wrote:
> i guess we could just do a:
> void MarbleDebug::setEnabled(bool enabled)
> {
> Marble::loggingCategory.setEnabled(enabled);
> MarbleDebug::m_enabled = enabled;
> }

I'd go a little further and drop the m_enabled variable completely. You don't 
need it, let the bit in the QLoggingCategory variable keep the state.

You'd have:

namespace MarbleDebug {
Q_LOGGING_CATEGORY(category, "marble", QtWarningMsg)

void setEnabled(bool enabled)
{
category.setEnabled(QtDebugMsg, enabled);
}

bool isEnabled()// do you even need this function?
{
return category.isDebugEnabled();
}
} //namespace MarbleDebug

This changes to the MarbleDebug namespace too.

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
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-17 Thread Lubomir I. Ivanov 
On 18 April 2017 at 00:02, Lubomir I. Ivanov  wrote:
> On 17 April 2017 at 23:46, Thiago Macieira  wrote:
>> This disables the debug output for the default category, that is, every user
>> of qDebug().
>>
>
> yep, i later figured that might happen.
>
>> I was suggesting turning mDebug() into qCDebug(...).
>>
>> // in a header
>> namespace Marble {
>> Q_DECLARE_LOGGING_CATEGORY(loggingCategory)
>> }
>> #define mDebug  qCDebug(Marble::loggingCategory)
>>
>> // in one source file
>> namespace Marble {
>> Q_LOGGING_CATEGORY(loggingCategory, "marble", QtWarningMsg)
>> }
>>
>
> i've tried returning a qCDebug() from mDebug, but it cannot be done.
> so how can the flag MarbleDebug::isEnabled() be used then to
> conditionally show debug output in Marble?
>

i guess we could just do a:
void MarbleDebug::setEnabled(bool enabled)
{
Marble::loggingCategory.setEnabled(enabled);
MarbleDebug::m_enabled = enabled;
}

lubomir
--
___
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-17 Thread Thiago Macieira 
On segunda-feira, 17 de abril de 2017 13:24:26 PDT Lubomir I. Ivanov wrote:
> On 17 April 2017 at 22:29, Thiago Macieira  wrote:
> > On segunda-feira, 17 de abril de 2017 12:01:48 PDT Lubomir I. Ivanov 
wrote:
> >> QDebug mDebug()
> >> 
> >>  {
> >>  
> >> return QDebug( QtDebugMsg ); // or "return qDebug();"
> >> 
> >> }
> >> 
> >> which will essentially enable debug output for everything in Marble,
> >> until we write a dummy Class with an overloaded << operator to void
> >> all the incoming debug calls.
> > 
> > It would be better to use a category and then simply turn the category
> > off. It can be enabled manually by users with QT_LOGGING_RULES
> > environment variable.
> > 
> > See http://doc.qt.io/qt-5/qloggingcategory.html.
> 
> thanks, for the suggestion, Thiago.
> 
> Stefan, could you please test the attached patch and if it works,
> submit it to Github for approval.

This disables the debug output for the default category, that is, every user 
of qDebug().

I was suggesting turning mDebug() into qCDebug(...).

// in a header
namespace Marble {
Q_DECLARE_LOGGING_CATEGORY(loggingCategory)
}
#define mDebug  qCDebug(Marble::loggingCategory)

// in one source file
namespace Marble {
Q_LOGGING_CATEGORY(loggingCategory, "marble", QtWarningMsg)
}

-- 
Thiago Macieira - thiago (AT) macieira.info - thiago (AT) kde.org
   Software Architect - Intel Open Source Technology Center

___
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-17 Thread Lubomir I. Ivanov 
On 17 April 2017 at 22:29, Thiago Macieira  wrote:
> On segunda-feira, 17 de abril de 2017 12:01:48 PDT Lubomir I. Ivanov wrote:
>> QDebug mDebug()
>>  {
>> return QDebug( QtDebugMsg ); // or "return qDebug();"
>> }
>>
>> which will essentially enable debug output for everything in Marble,
>> until we write a dummy Class with an overloaded << operator to void
>> all the incoming debug calls.
>
> It would be better to use a category and then simply turn the category off. It
> can be enabled manually by users with QT_LOGGING_RULES environment variable.
>
> See http://doc.qt.io/qt-5/qloggingcategory.html.
>

thanks, for the suggestion, Thiago.

Stefan, could you please test the attached patch and if it works,
submit it to Github for approval.

lubomir
--
From 64bac042e3378108eeab1b66a18fa5f63d91e5fa Mon Sep 17 00:00:00 2001
From: "Lubomir I. Ivanov" 
Date: Mon, 17 Apr 2017 23:15:23 +0300
Subject: [PATCH] MarbleDebug: use QLoggingCategory to disable logging via
 mDebug()

Signed-off-by: Lubomir I. Ivanov 
---
 src/lib/marble/MarbleDebug.cpp | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/src/lib/marble/MarbleDebug.cpp b/src/lib/marble/MarbleDebug.cpp
index 5f53f6f..6c00dfd 100644
--- a/src/lib/marble/MarbleDebug.cpp
+++ b/src/lib/marble/MarbleDebug.cpp
@@ -11,7 +11,7 @@
 #include "MarbleDebug.h"
 
 #include 
-#include 
+#include 
 
 namespace Marble
 {
@@ -20,15 +20,8 @@ bool MarbleDebug::m_enabled = false;
 
 QDebug mDebug()
 {
-if ( MarbleDebug::isEnabled() ) {
-return QDebug( QtDebugMsg );
-}
-else {
-static QFile *nullDevice = new QFile(QProcess::nullDevice());
-if ( !nullDevice->isOpen() )
- nullDevice->open(QIODevice::WriteOnly);
- return QDebug( nullDevice );
-}
+QLoggingCategory::defaultCategory()->setEnabled(QtDebugMsg, MarbleDebug::isEnabled());
+return QDebug( QtDebugMsg );
 }
 
 bool MarbleDebug::isEnabled()
-- 
1.7.11.msysgit.0

___
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-17 Thread Stefan Fuchs 
Hello Lubomir,

Am 17.04.2017 um 21:01 schrieb Lubomir I. Ivanov:
> your best bet is to change the function to this:
>
> QDebug mDebug()
>  {
> return QDebug( QtDebugMsg ); // or "return qDebug();"
> }
>
> which will essentially enable debug output for everything in Marble,
> until we write a dummy Class with an overloaded << operator to void
> all the incoming debug calls.
Yes, agreed. I did this already once before following your suggestion
and I now did it again in a little different way by enabling a line Dirk
recently added in MarbleDebug.cpp:

namespace Marble
{
bool MarbleDebug::m_enabled = true;
// bool MarbleDebug::m_enabled = false;

Even with my limited skills I can guess that will give the same result.
And yes: In both cases no more crashes.
And yes: I see lot's of marble debug output now if I start under windows
from the PowerShell.


But still two questions, one maybe stupid:
- Why could it be that eventually in Marble FileLoader::run every ~10th
startup the "different" branch including line 138 is executed? Does this
happen unintentionally? This is executed for d->m_contents.isEmpty()==true.
- Can this change also go into Dirks Subsurface-marble branch? Maybe
also other users are affected?

Best regards
Stefan

-- 

Stefan Fuchs
E-Mail: sfu...@gmx.de 

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