From 95931751899e0fd971668260e5432a002f4d9a20 Mon Sep 17 00:00:00 2001 From: RJ Ryan Date: Sat, 28 Jan 2017 13:43:42 -0800 Subject: [PATCH 1/3] Logging updates: * Rename --debugLevel to --logLevel and use English names for the levels. * Add qInfo support when Qt5 is used. * Bypass --logLevel for controller debug statements when --controllerDebug is enabled. --- src/controllers/controllerdebug.h | 10 ++++-- src/util/cmdlineargs.cpp | 29 +++++++++------- src/util/cmdlineargs.h | 6 ++-- src/util/logging.cpp | 58 +++++++++++++++++++++++-------- src/util/logging.h | 14 ++++++-- 5 files changed, 82 insertions(+), 35 deletions(-) diff --git a/src/controllers/controllerdebug.h b/src/controllers/controllerdebug.h index 8c03519da261..42ba24e46679 100644 --- a/src/controllers/controllerdebug.h +++ b/src/controllers/controllerdebug.h @@ -4,6 +4,7 @@ #include #include "util/cmdlineargs.h" +#include "util/logging.h" class ControllerDebug { public: @@ -31,12 +32,15 @@ class ControllerDebug { bool m_enabled; }; -// Usage -// controllerDebug("hello" << "world"); +// Usage: controllerDebug("hello" << "world"); +// +// We prefix every log message with Logging::kControllerDebugPrefix so that we +// can bypass the --logLevel commandline argument when --controllerDebug is +// specified. #define controllerDebug(stream) \ { \ if (ControllerDebug::enabled()) { \ - QDebug(QtDebugMsg) << stream; \ + QDebug(QtDebugMsg) << mixxx::Logging::kControllerDebugPrefix << stream; \ } \ } \ diff --git a/src/util/cmdlineargs.cpp b/src/util/cmdlineargs.cpp index c9eb1b9e47b1..330e00422bb5 100644 --- a/src/util/cmdlineargs.cpp +++ b/src/util/cmdlineargs.cpp @@ -4,7 +4,6 @@ #include "util/version.h" #include "sources/soundsourceproxy.h" -#include "util/logging.h" CmdlineArgs::CmdlineArgs() @@ -13,7 +12,7 @@ CmdlineArgs::CmdlineArgs() m_developer(false), m_safeMode(false), m_settingsPathSet(false), - m_debugLevel(mixxx::kDebugLevelDefault), + m_logLevel(mixxx::Logging::kLogLevelDefault), // We are not ready to switch to XDG folders under Linux, so keeping $HOME/.mixxx as preferences folder. see lp:1463273 #ifdef __LINUX__ m_settingsPath(QDir::homePath().append("/").append(SETTINGS_PATH)) { @@ -54,13 +53,18 @@ bool CmdlineArgs::Parse(int &argc, char **argv) { } else if (argv[i] == QString("--timelinePath") && i+1 < argc) { m_timelinePath = QString::fromLocal8Bit(argv[i+1]); i++; - } else if (argv[i] == QString("--debugLevel") && i+1 < argc) { - bool isInt = false; - int debugLevelRq = QString::fromLocal8Bit(argv[i+1]).toInt(&isInt); - if (isInt) { - m_debugLevel = debugLevelRq; + } else if (argv[i] == QString("--logLevel") && i+1 < argc) { + auto level = QLatin1String(argv[i+1]); + if (level == "debug") { + m_logLevel = mixxx::Logging::LogLevel::Debug; + } else if (level == "info") { + m_logLevel = mixxx::Logging::LogLevel::Info; + } else if (level == "warning") { + m_logLevel = mixxx::Logging::LogLevel::Warning; + } else if (level == "critical") { + m_logLevel = mixxx::Logging::LogLevel::Critical; } else { - fputs("\ndebugLevel argument wasn't a number! Mixxx will only output\n\ + fputs("\nlogLevel argument wasn't 'debug', 'info', 'warning', or 'critical'! Mixxx will only output\n\ warnings and errors to the console unless this is set properly.\n", stdout); } i++; @@ -124,10 +128,11 @@ void CmdlineArgs::printUsage() { \n\ -f, --fullScreen Starts Mixxx in full-screen mode\n\ \n\ ---debugLevel LEVEL Sets the verbosity of command line debug messages\n\ - 0 - Critical/Fatal only\n\ - 1 - Above + Warnings\n\ - 2 - Above + Debug/Developer messages\n\ +--logLevel LEVEL Sets the verbosity of command line logging\n\ + critical - Critical/Fatal only\n\ + warning - Above + Warnings\n\ + info - Above + Informational messages\n\ + debug - Above + Debug/Developer messages\n\ \n\ -h, --help Display this help message and exit", stdout); diff --git a/src/util/cmdlineargs.h b/src/util/cmdlineargs.h index 3691dd0e69f5..32d42b68d978 100644 --- a/src/util/cmdlineargs.h +++ b/src/util/cmdlineargs.h @@ -6,6 +6,8 @@ #include #include +#include "util/logging.h" + // A structure to store the parsed command-line arguments class CmdlineArgs final { public: @@ -23,7 +25,7 @@ class CmdlineArgs final { bool getDeveloper() const { return m_developer; } bool getSafeMode() const { return m_safeMode; } bool getSettingsPathSet() const { return m_settingsPathSet; } - int getDebugLevel() const { return m_debugLevel; } + mixxx::Logging::LogLevel getLogLevel() const { return m_logLevel; } bool getTimelineEnabled() const { return !m_timelinePath.isEmpty(); } const QString& getLocale() const { return m_locale; } const QString& getSettingsPath() const { return m_settingsPath; } @@ -43,7 +45,7 @@ class CmdlineArgs final { bool m_developer; // Developer Mode bool m_safeMode; bool m_settingsPathSet; // has --settingsPath been set on command line ? - int m_debugLevel; // Level of debug message verbosity + mixxx::Logging::LogLevel m_logLevel; // Level of logging message verbosity QString m_locale; QString m_settingsPath; QString m_resourcePath; diff --git a/src/util/logging.cpp b/src/util/logging.cpp index 2f6568da84ae..95f304218ed0 100644 --- a/src/util/logging.cpp +++ b/src/util/logging.cpp @@ -21,7 +21,6 @@ namespace { QFile Logfile; QMutex mutexLogfile; -int debugLevel; // Debug message handler which outputs to both a logfile and prepends the thread // the message came from. @@ -44,10 +43,22 @@ void MessageHandler(QtMsgType type, } else { ba = "[?]: "; } + #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) - ba += input; + bool controllerDebug = strncmp(input, Logging::kControllerDebugPrefix, + strlen(Logging::kControllerDebugPrefix)) == 0; + if (controllerDebug) { + ba += (input + strlen(Logging::kControllerDebugPrefix) + 1); + } else { + ba += input; + } #else - ba += input.toLocal8Bit(); + bool controllerDebug = input.startsWith(QLatin1String(Logging::kControllerDebugPrefix)); + if (controllerDebug) { + ba += input.mid(strlen(Logging::kControllerDebugPrefix) + 1).toLocal8Bit(); + } else { + ba += input.toLocal8Bit(); + } #endif ba += "\n"; @@ -88,42 +99,56 @@ void MessageHandler(QtMsgType type, Logfile.open(QIODevice::WriteOnly | QIODevice::Text); } - debugLevel = CmdlineArgs::Instance().getDebugLevel(); // Get message verbosity + Logging::LogLevel logLevel = CmdlineArgs::Instance().getLogLevel(); switch (type) { - case QtDebugMsg: // debugLevel 2 - if (debugLevel > kDebugLevelDefault) + case QtDebugMsg: + if (logLevel >= Logging::LogLevel::Debug || controllerDebug) { fprintf(stderr, "Debug %s", ba.constData()); + } if (Logfile.isOpen()) { Logfile.write("Debug "); Logfile.write(ba); } break; - case QtWarningMsg: // debugLevel 1 - if (debugLevel > kDebugLevelMin) +#if QT_VERSION >= QT_VERSION_CHECK(5, 5, 0) + case QtInfoMsg: + if (logLevel >= Logging::LogLevel::Info) { + fprintf(stderr, "Info %s", ba.constData()); + } + if (Logfile.isOpen()) { + Logfile.write("Info "); + Logfile.write(ba); + } + break; +#endif + case QtWarningMsg: + if (logLevel >= Logging::LogLevel::Warning) { fprintf(stderr, "Warning %s", ba.constData()); + } if (Logfile.isOpen()) { Logfile.write("Warning "); Logfile.write(ba); } break; - case QtCriticalMsg: // debugLevel 0 (always shown) + case QtCriticalMsg: + // Critical errors are always shown on the console. fprintf(stderr, "Critical %s", ba.constData()); if (Logfile.isOpen()) { Logfile.write("Critical "); Logfile.write(ba); } - break; //NOTREACHED - case QtFatalMsg: // debugLevel 0 (always shown) + break; + case QtFatalMsg: + // Fatal errors are always shown on the console. fprintf(stderr, "Fatal %s", ba.constData()); if (Logfile.isOpen()) { Logfile.write("Fatal "); Logfile.write(ba); } - break; //NOTREACHED - default: // debugLevel unknown, we'll assume Warning - if (debugLevel > 0) - fprintf(stderr, "Unknown %s", ba.constData()); + break; + default: + fprintf(stderr, "Unknown %s", ba.constData()); if (Logfile.isOpen()) { Logfile.write("Unknown "); Logfile.write(ba); @@ -137,6 +162,9 @@ void MessageHandler(QtMsgType type, } // namespace +// static +constexpr Logging::LogLevel Logging::kLogLevelDefault; + // static void Logging::initialize() { #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) diff --git a/src/util/logging.h b/src/util/logging.h index 705e2797c5b9..cec663559416 100644 --- a/src/util/logging.h +++ b/src/util/logging.h @@ -3,11 +3,19 @@ namespace mixxx { -static const int kDebugLevelMin = 0; -static const int kDebugLevelDefault = 1; - class Logging { public: + enum class LogLevel { + Critical = 0, + Warning = 1, + Info = 2, + Debug = 3 + }; + static constexpr LogLevel kLogLevelDefault = LogLevel::Warning; + // Any debug statement starting with this prefix bypasses the --logLevel + // command line flags. + static constexpr const char* kControllerDebugPrefix = "CDBG"; + static void initialize(); static void shutdown(); private: From 5fdcb70325c8bd4001a32c462ec7da63256bf7eb Mon Sep 17 00:00:00 2001 From: RJ Ryan Date: Sat, 28 Jan 2017 22:50:10 -0800 Subject: [PATCH 2/3] Force --logLevel to debug if it is unspecified and --developer is enabled. --- src/util/cmdlineargs.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/util/cmdlineargs.cpp b/src/util/cmdlineargs.cpp index 330e00422bb5..79421e2ca977 100644 --- a/src/util/cmdlineargs.cpp +++ b/src/util/cmdlineargs.cpp @@ -24,6 +24,7 @@ CmdlineArgs::CmdlineArgs() } bool CmdlineArgs::Parse(int &argc, char **argv) { + bool logLevelSet = false; for (int i = 0; i < argc; ++i) { if ( argv[i] == QString("-h") || argv[i] == QString("--h") @@ -54,6 +55,7 @@ bool CmdlineArgs::Parse(int &argc, char **argv) { m_timelinePath = QString::fromLocal8Bit(argv[i+1]); i++; } else if (argv[i] == QString("--logLevel") && i+1 < argc) { + logLevelSet = true; auto level = QLatin1String(argv[i+1]); if (level == "debug") { m_logLevel = mixxx::Logging::LogLevel::Debug; @@ -79,6 +81,13 @@ warnings and errors to the console unless this is set properly.\n", stdout); m_musicFiles += QString::fromLocal8Bit(argv[i]); } } + + // If --logLevel was unspecified and --developer is enabled then set + // logLevel to debug. + if (m_developer && !logLevelSet) { + m_logLevel = mixxx::Logging::LogLevel::Debug; + } + return true; } From a2d8cc89aa62edf8e7e8cb8caae072fe663449cf Mon Sep 17 00:00:00 2001 From: RJ Ryan Date: Sun, 29 Jan 2017 20:43:58 -0800 Subject: [PATCH 3/3] Use controllerDebug for engine.log messages. --- src/controllers/controllerengine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/controllers/controllerengine.cpp b/src/controllers/controllerengine.cpp index 0dcb12ba8917..6645e5139be7 100644 --- a/src/controllers/controllerengine.cpp +++ b/src/controllers/controllerengine.cpp @@ -687,7 +687,7 @@ double ControllerEngine::getDefaultParameter(QString group, QString name) { Output: - -------- ------------------------------------------------------ */ void ControllerEngine::log(QString message) { - qDebug() << message; + controllerDebug(message); } /* -------- ------------------------------------------------------