From 8e8ac2f3c7d54c3e934ed95ab3448bf4aaf295cc Mon Sep 17 00:00:00 2001 From: Evgeniy Alekseev Date: Thu, 30 Jun 2016 01:59:06 +0300 Subject: [PATCH] some refactoring * fix undefinded behaviour * drop dbus timout, use generic timeout insead * drop load source to own cmake key * update contributing.md --- CONTRIBUTING.md | 9 +- sources/CMakeLists.txt | 1 + sources/awdebug.cpp | 2 + .../plugin/awdataengineaggregator.cpp | 3 +- sources/awesome-widget/plugin/awkeycache.cpp | 6 +- .../awesome-widget/plugin/awkeyoperations.cpp | 82 +++++++++++-------- .../plugin/awkeysaggregator.cpp | 2 +- .../abstractextitemaggregator.cpp | 10 +-- sources/extsysmon/extsysmonaggregator.cpp | 4 +- sources/extsysmonsources/playersource.cpp | 2 +- sources/extsysmonsources/playersource.h | 4 - sources/test/testawkeys.cpp | 1 + sources/test/testdesktopsource.cpp | 5 -- sources/test/testprocessessource.cpp | 5 -- sources/version.h.in | 4 +- 15 files changed, 75 insertions(+), 65 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c024dd9..a58c2ca 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -54,7 +54,14 @@ for more details. To avoid manual labor there is automatic cmake target named ``` * C-like `NULL`, use `nullptr` instead. -* It is highly recommended to avoid implicit casts. +* It is highly recommended to avoid implicit casts. Exception `nullptr` casts to + boolean, e.g.: + + ``` + if (nullptr) + qDebug() << "nullptr is true, wtf"; + ``` + * Abstract classes (which have at least one pure virtual method) are allowed. * Templates are allowed and recommended. Templates usually should be described inside header not source code file. diff --git a/sources/CMakeLists.txt b/sources/CMakeLists.txt index fa0dc2c..8a415f7 100644 --- a/sources/CMakeLists.txt +++ b/sources/CMakeLists.txt @@ -34,6 +34,7 @@ option(BUILD_DEB_PACKAGE "Build deb package" OFF) option(BUILD_RPM_PACKAGE "Build rpm package" OFF) # build details option(BUILD_FUTURE "Build with the features which will be marked as stable later" OFF) +option(BUILD_LOAD "Build with additional load" OFF) option(BUILD_TESTING "Build with additional test abilities" OFF) # generate changelog diff --git a/sources/awdebug.cpp b/sources/awdebug.cpp index 6397383..09e4ea5 100644 --- a/sources/awdebug.cpp +++ b/sources/awdebug.cpp @@ -47,6 +47,7 @@ const QStringList getBuildData() metadata.append(QString(" AWEUAPI: %1").arg(AWEUAPI)); metadata.append(QString(" AWEWAPI: %1").arg(AWEWAPI)); metadata.append(QString(" AWEFAPI: %1").arg(AWEFAPI)); + metadata.append(QString(" REQUEST_TIMEOUT: %1").arg(REQUEST_TIMEOUT)); metadata.append(QString(" TIME_KEYS: %1").arg(TIME_KEYS)); metadata.append(QString(" STATIC_KEYS: %1").arg(STATIC_KEYS)); // cmake properties @@ -89,6 +90,7 @@ const QStringList getBuildData() QString(" CPPCHECK_EXECUTABLE: %1").arg(CPPCHECK_EXECUTABLE)); // additional functions metadata.append(QString(" PROP_FUTURE: %1").arg(PROP_FUTURE)); + metadata.append(QString(" PROP_LOAD: %1").arg(PROP_LOAD)); metadata.append(QString(" PROP_TEST: %1").arg(PROP_TEST)); return metadata; diff --git a/sources/awesome-widget/plugin/awdataengineaggregator.cpp b/sources/awesome-widget/plugin/awdataengineaggregator.cpp index a8b7d96..9148b73 100644 --- a/sources/awesome-widget/plugin/awdataengineaggregator.cpp +++ b/sources/awesome-widget/plugin/awdataengineaggregator.cpp @@ -42,7 +42,6 @@ void AWDataEngineAggregator::clear() disconnectSources(); m_dataEngines.clear(); delete m_consumer; - m_consumer = nullptr; } @@ -82,7 +81,7 @@ void AWDataEngineAggregator::dropSource(const QString source) { qCDebug(LOG_AW) << "Source" << source; - // FIXME there is no possibility to check to which dataengine source + // HACK there is no possibility to check to which dataengine source // connected we will try to disconnect it from systemmonitor and extsysmon m_dataEngines[QString("systemmonitor")]->disconnectSource(source, parent()); m_dataEngines[QString("extsysmon")]->disconnectSource(source, parent()); diff --git a/sources/awesome-widget/plugin/awkeycache.cpp b/sources/awesome-widget/plugin/awkeycache.cpp index 11fdcd4..a668908 100644 --- a/sources/awesome-widget/plugin/awkeycache.cpp +++ b/sources/awesome-widget/plugin/awkeycache.cpp @@ -38,8 +38,8 @@ bool AWKeyCache::addKeyToCache(const QString type, const QString key) cache.beginGroup(type); QStringList cachedValues; - for (auto key : cache.allKeys()) - cachedValues.append(cache.value(key).toString()); + for (auto number : cache.allKeys()) + cachedValues.append(cache.value(number).toString()); if (type == QString("hdd")) { QStringList allDevices @@ -51,6 +51,7 @@ bool AWKeyCache::addKeyToCache(const QString type, const QString key) if (cachedValues.contains(device)) continue; qCInfo(LOG_AW) << "Found new key" << device << "for type" << type; + cachedValues.append(device); cache.setValue( QString("%1").arg(cache.allKeys().count(), 3, 10, QChar('0')), device); @@ -63,6 +64,7 @@ bool AWKeyCache::addKeyToCache(const QString type, const QString key) if (cachedValues.contains(device)) continue; qCInfo(LOG_AW) << "Found new key" << device << "for type" << type; + cachedValues.append(device); cache.setValue( QString("%1").arg(cache.allKeys().count(), 3, 10, QChar('0')), device); diff --git a/sources/awesome-widget/plugin/awkeyoperations.cpp b/sources/awesome-widget/plugin/awkeyoperations.cpp index 811dff0..ea14d0c 100644 --- a/sources/awesome-widget/plugin/awkeyoperations.cpp +++ b/sources/awesome-widget/plugin/awkeyoperations.cpp @@ -191,43 +191,53 @@ QString AWKeyOperations::infoByKey(QString key) const QString stripped = key; stripped.remove(QRegExp(QString("\\d+"))); + QString output; - QString output = QString("(none)"); - - // FIXME undefined behaviour - if (key.startsWith(QString("bar"))) - return graphicalItems->itemByTag(key, stripped)->uniq(); - else if (key.startsWith(QString("custom"))) - return extScripts->itemByTag(key, stripped)->uniq(); - else if (key.contains(QRegExp(QString("^hdd[rw]")))) - return QString("%1").arg( - m_devices[QString("disk")] - [key.remove(QRegExp(QString("hdd[rw]"))).toInt()]); - else if (key.contains(QRegExp( - QString("^hdd([0-9]|mb|gb|freemb|freegb|totmb|totgb)")))) - return QString("%1").arg( - m_devices[QString("mount")] - [key - .remove(QRegExp(QString( - "^hdd([0-9]|mb|gb|freemb|freegb|totmb|totgb)"))) - .toInt()]); - else if (key.startsWith(QString("hddtemp"))) - return QString("%1").arg( - m_devices[QString("hdd")][key.remove(QString("hddtemp")).toInt()]); - else if (key.contains(QRegExp(QString("^(down|up)[0-9]")))) - return QString("%1").arg( - m_devices[QString("net")] - [key.remove(QRegExp(QString("^(down|up)"))).toInt()]); - else if (key.startsWith(QString("pkgcount"))) - return extUpgrade->itemByTag(key, stripped)->uniq(); - else if (key.contains(QRegExp(QString("(^|perc)(ask|bid|price)(chg|)")))) - return extQuotes->itemByTag(key, stripped)->uniq(); - else if (key.contains(QRegExp( - QString("(weather|weatherId|humidity|pressure|temperature)")))) - return extWeather->itemByTag(key, stripped)->uniq(); - else if (key.startsWith(QString("temp"))) - return QString("%1").arg( - m_devices[QString("temp")][key.remove(QString("temp")).toInt()]); + if (key.startsWith(QString("bar"))) { + AbstractExtItem *item = graphicalItems->itemByTag(key, stripped); + if (item) + output = item->uniq(); + } else if (key.startsWith(QString("custom"))) { + AbstractExtItem *item = extScripts->itemByTag(key, stripped); + if (item) + output = item->uniq(); + } else if (key.contains(QRegExp(QString("^hdd[rw]")))) { + output = m_devices[QString("disk")] + [key.remove(QRegExp(QString("hdd[rw]"))).toInt()]; + } else if (key.contains(QRegExp( + QString("^hdd([0-9]|mb|gb|freemb|freegb|totmb|totgb)")))) { + output + = m_devices[QString("mount")] + [key + .remove(QRegExp(QString( + "^hdd([0-9]|mb|gb|freemb|freegb|totmb|totgb)"))) + .toInt()]; + } else if (key.startsWith(QString("hddtemp"))) { + output + = m_devices[QString("hdd")][key.remove(QString("hddtemp")).toInt()]; + } else if (key.contains(QRegExp(QString("^(down|up)[0-9]")))) { + output = m_devices[QString("net")] + [key.remove(QRegExp(QString("^(down|up)"))).toInt()]; + } else if (key.startsWith(QString("pkgcount"))) { + AbstractExtItem *item = extUpgrade->itemByTag(key, stripped); + if (item) + output = item->uniq(); + } else if (key.contains( + QRegExp(QString("(^|perc)(ask|bid|price)(chg|)")))) { + AbstractExtItem *item = extQuotes->itemByTag(key, stripped); + if (item) + output = item->uniq(); + } else if (key.contains(QRegExp(QString( + "(weather|weatherId|humidity|pressure|temperature)")))) { + AbstractExtItem *item = extWeather->itemByTag(key, stripped); + if (item) + output = item->uniq(); + } else if (key.startsWith(QString("temp"))) { + output + = m_devices[QString("temp")][key.remove(QString("temp")).toInt()]; + } else { + output = QString("(none)"); + } return output; } diff --git a/sources/awesome-widget/plugin/awkeysaggregator.cpp b/sources/awesome-widget/plugin/awkeysaggregator.cpp index 735f716..9fdb952 100644 --- a/sources/awesome-widget/plugin/awkeysaggregator.cpp +++ b/sources/awesome-widget/plugin/awkeysaggregator.cpp @@ -515,7 +515,7 @@ QStringList AWKeysAggregator::registerSource(const QString &source, } else if (source.startsWith(QString("lmsensors/"))) { // temperature int index = m_devices[QString("temp")].indexOf(source); - // FIXME on DE initialization there are no units key + // HACK on DE initialization there are no units key if (units.isEmpty()) return QStringList() << QString("temp%1").arg(index); if (index > -1) { diff --git a/sources/awesomewidgets/abstractextitemaggregator.cpp b/sources/awesomewidgets/abstractextitemaggregator.cpp index 90704cf..cba2f6e 100644 --- a/sources/awesomewidgets/abstractextitemaggregator.cpp +++ b/sources/awesomewidgets/abstractextitemaggregator.cpp @@ -66,7 +66,7 @@ void AbstractExtItemAggregator::copyItem() .arg(QStandardPaths::writableLocation( QStandardPaths::GenericDataLocation)) .arg(m_type); - if ((source == nullptr) || (fileName.isEmpty())) { + if ((!source) || (fileName.isEmpty())) { qCWarning(LOG_LIB) << "Nothing to copy"; return; } @@ -83,7 +83,7 @@ void AbstractExtItemAggregator::copyItem() void AbstractExtItemAggregator::deleteItem() { AbstractExtItem *source = itemFromWidget(); - if (source == nullptr) { + if (!source) { qCWarning(LOG_LIB) << "Nothing to delete"; return; }; @@ -98,7 +98,7 @@ void AbstractExtItemAggregator::deleteItem() void AbstractExtItemAggregator::editItem() { AbstractExtItem *source = itemFromWidget(); - if (source == nullptr) { + if (!source) { qCWarning(LOG_LIB) << "Nothing to edit"; return; }; @@ -128,7 +128,7 @@ QString AbstractExtItemAggregator::getName() AbstractExtItem *AbstractExtItemAggregator::itemFromWidget() { QListWidgetItem *widgetItem = ui->listWidget->currentItem(); - if (widgetItem == nullptr) + if (!widgetItem) return nullptr; AbstractExtItem *found = nullptr; @@ -139,7 +139,7 @@ AbstractExtItem *AbstractExtItemAggregator::itemFromWidget() found = item; break; } - if (found == nullptr) + if (!found) qCWarning(LOG_LIB) << "Could not find item by name" << widgetItem->text(); diff --git a/sources/extsysmon/extsysmonaggregator.cpp b/sources/extsysmon/extsysmonaggregator.cpp index 18a7130..050a748 100644 --- a/sources/extsysmon/extsysmonaggregator.cpp +++ b/sources/extsysmon/extsysmonaggregator.cpp @@ -148,10 +148,10 @@ void ExtSysMonAggregator::init(const QHash config) = new WeatherSource(this, QStringList()); for (auto source : weatherItem->sources()) m_map[source] = weatherItem; -#ifdef BUILD_TESTING +#ifdef BUILD_LOAD // additional load source AbstractExtSysMonSource *loadItem = new LoadSource(this, QStringList()); for (auto source : loadItem->sources()) m_map[source] = loadItem; -#endif /* BUILD_TESTING */ +#endif /* BUILD_LOAD */ } diff --git a/sources/extsysmonsources/playersource.cpp b/sources/extsysmonsources/playersource.cpp index a686d8c..e082006 100644 --- a/sources/extsysmonsources/playersource.cpp +++ b/sources/extsysmonsources/playersource.cpp @@ -345,7 +345,7 @@ QVariantHash PlayerSource::getPlayerMprisInfo(const QString mpris) const QString("/org/mpris/MediaPlayer2"), QString(""), QString("Get")); request.setArguments(args); QDBusMessage response - = bus.call(request, QDBus::BlockWithGui, DBUS_CALL_TIMEOUT); + = bus.call(request, QDBus::BlockWithGui, REQUEST_TIMEOUT); if ((response.type() != QDBusMessage::ReplyMessage) || (response.arguments().isEmpty())) { qCWarning(LOG_ESS) << "Error message" << response.errorMessage(); diff --git a/sources/extsysmonsources/playersource.h b/sources/extsysmonsources/playersource.h index 48fa39b..39d666b 100644 --- a/sources/extsysmonsources/playersource.h +++ b/sources/extsysmonsources/playersource.h @@ -24,10 +24,6 @@ #include "abstractextsysmonsource.h" -#ifndef DBUS_CALL_TIMEOUT -#define DBUS_CALL_TIMEOUT 3000 -#endif /* DBUS_CALL_TIMEOUT */ - class QProcess; class PlayerSource : public AbstractExtSysMonSource diff --git a/sources/test/testawkeys.cpp b/sources/test/testawkeys.cpp index b20acc6..84d7beb 100644 --- a/sources/test/testawkeys.cpp +++ b/sources/test/testawkeys.cpp @@ -95,6 +95,7 @@ void TestAWKeys::test_pattern() plugin->initKeys(pattern, interval, 0, false); QSignalSpy spy(plugin, SIGNAL(needTextToBeUpdated(const QString))); + QVERIFY(spy.wait(5 * interval)); QVERIFY(spy.wait(5 * interval)); QString text = spy.takeFirst().at(0).toString(); diff --git a/sources/test/testdesktopsource.cpp b/sources/test/testdesktopsource.cpp index 4096d06..36831e6 100644 --- a/sources/test/testdesktopsource.cpp +++ b/sources/test/testdesktopsource.cpp @@ -39,11 +39,6 @@ void TestDesktopSource::cleanupTestCase() void TestDesktopSource::test_sources() { QCOMPARE(source->sources().count(), 4); - // FIXME there is segfault here sometimes o_0 - // QVERIFY(std::all_of( - // source->sources().cbegin(), source->sources().cend(), - // [](const QString &src) { return - // src.startsWith(QString("desktop/")); })); } diff --git a/sources/test/testprocessessource.cpp b/sources/test/testprocessessource.cpp index 5119418..fef1bf1 100644 --- a/sources/test/testprocessessource.cpp +++ b/sources/test/testprocessessource.cpp @@ -39,11 +39,6 @@ void TestProcessesSource::cleanupTestCase() void TestProcessesSource::test_sources() { QCOMPARE(source->sources().count(), 3); - // FIXME there is segfault here sometimes o_0 - // QVERIFY(std::all_of( - // source->sources().cbegin(), source->sources().cend(), - // [](const QString &src) { return src.startsWith(QString("ps/")); - // })); } diff --git a/sources/version.h.in b/sources/version.h.in index 4c89f7b..518de5a 100644 --- a/sources/version.h.in +++ b/sources/version.h.in @@ -37,7 +37,7 @@ // formatter api version #define AWEFAPI 1 // network requests timeout, ms -#define REQUEST_TIMEOUT 5000 +#define REQUEST_TIMEOUT 3000 // available time keys #define TIME_KEYS \ "dddd,ddd,dd,d,MMMM,MMM,MM,M,yyyy,yy,hh,h,HH,H,mm,m,ss,s,t,ap,a,AP,A" @@ -51,6 +51,7 @@ "dalbum,dartist,dtitle,salbum,sartist,stitle,pscount,pstotal,ps,desktop," \ "ndesktop,tdesktops,la15,la5,la1" #cmakedefine BUILD_FUTURE +#cmakedefine BUILD_LOAD #cmakedefine BUILD_TESTING // links @@ -94,6 +95,7 @@ #define CPPCHECK_EXECUTABLE "@CPPCHECK_EXECUTABLE@" // additional functions #define PROP_FUTURE "@BUILD_FUTURE@" +#define PROP_LOAD "@BUILD_LOAD@" #define PROP_TEST "@BUILD_TESTING@"