Themes commit-by-commit part 1 #7566

zeule · 2017-10-10T10:14:10Z

As per discussion in #6698 I'm splitting that PR onto smaller ones. Here is the first part. No functional changes here, just code formatting and refactoring,

glassez · 2017-10-10T10:46:35Z

src/base/torrentfilter.h

@@ -36,10 +36,7 @@ typedef QSet<QString> QStringSet;

 namespace BitTorrent
 {
-
 class TorrentHandle;


Please indent this line.

glassez · 2017-10-10T10:49:24Z

src/base/preferences.cpp

@@ -1459,10 +1464,9 @@ void Preferences::upgrade()
    QStringList labels = value("TransferListFilters/customLabels").toStringList();
    if (!labels.isEmpty()) {
        QVariantMap categories = value("BitTorrent/Session/Categories").toMap();
-        foreach (const QString &label, labels) {


There was no violation of the coding style, so this is a purely subjective change...

I mean we shouldn't accept such changes. Otherwise, everyone will do some unrelated changes, just formatting some code snippets in its own way.

OK. Reverted.

glassez · 2017-10-10T10:53:24Z

src/base/preferences.cpp

@@ -59,7 +59,7 @@

 Preferences *Preferences::m_instance = 0;

-Preferences::Preferences() {}


Question only for my interest... Why is the new version (with default) better than the former (with empty body)?

Just to give more coding flexibility and less format violation options.

glassez · 2017-10-10T10:54:31Z

src/base/preferences.cpp

@@ -264,6 +264,7 @@ void Preferences::setWinStartup(bool b)
        settings.remove("qBittorrent");
    }
 }
+


Why these blank lines?

@evsh, I used plural form... there are some other similar blank lines.

Chocobo1 · 2017-10-10T12:30:34Z

src/webui/btjson.cpp

@@ -34,6 +34,8 @@
 #include <QElapsedTimer>
 #include <QVariant>

+#include <libtorrent/version.hpp>


you need this later?

https://github.com/qbittorrent/qBittorrent/pull/7566/files#diff-a0b4caf6b479786e86d228562b0049e0R267

Chocobo1 · 2017-10-10T12:31:39Z

src/webui/btjson.cpp

@@ -227,6 +229,52 @@ static const char KEY_LOG_PEER_REASON[] = "reason";

 namespace
 {
+    QString torrentStateToString(BitTorrent::TorrentState state)


can we take const BitTorrent::TorrentState state please?

everything else is fine, just curious.

Chocobo1 · 2017-10-10T12:36:21Z

src/base/preferences.cpp

@@ -740,6 +740,7 @@ void Preferences::useSystemIconTheme(bool enabled)
 {
    setValue("Preferences/Advanced/useSystemIconTheme", enabled);
 }
+


uncrustify did this?

glassez · 2017-10-10T18:56:24Z

src/gui/guiiconprovider.cpp

@@ -117,6 +117,7 @@ QIcon GuiIconProvider::generateDifferentSizes(const QIcon &icon)

    return newIcon;
 }
+


And this one.

glassez · 2017-10-10T19:00:31Z

src/base/preferences.cpp

@@ -1459,10 +1464,9 @@ void Preferences::upgrade()
    QStringList labels = value("TransferListFilters/customLabels").toStringList();
    if (!labels.isEmpty()) {
        QVariantMap categories = value("BitTorrent/Session/Categories").toMap();
-        foreach (const QString &label, labels) {


I mean we shouldn't accept such changes. Otherwise, everyone will do some unrelated changes, just formatting some code snippets in its own way.

--HG-- branch : magao-dev

This is needed to forward declare this type and pass it by value. Conversion from/to QVariant are hanled via Q_DECLARE_METATYPE, while TorrentState::toString() function was used in webui only and as such is moved there.

zeule · 2017-10-11T08:34:26Z

Thanks!

zeule requested review from sledgehammer999, glassez and Chocobo1 October 10, 2017 10:14

glassez requested changes Oct 10, 2017

View reviewed changes

Chocobo1 reviewed Oct 10, 2017

View reviewed changes

glassez requested changes Oct 10, 2017

View reviewed changes

magao and others added 2 commits October 10, 2017 21:28

Follow project coding style. Issue #2192.

19bb6f5

--HG-- branch : magao-dev

Make BitTorrent::TorrentState strongly-typed enum

db35bb5

This is needed to forward declare this type and pass it by value. Conversion from/to QVariant are hanled via Q_DECLARE_METATYPE, while TorrentState::toString() function was used in webui only and as such is moved there.

glassez approved these changes Oct 11, 2017

View reviewed changes

Chocobo1 approved these changes Oct 11, 2017

View reviewed changes

zeule merged commit 7a652c0 into qbittorrent:master Oct 11, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Themes commit-by-commit part 1 #7566

Themes commit-by-commit part 1 #7566

zeule commented Oct 10, 2017

glassez Oct 10, 2017

glassez Oct 10, 2017

glassez Oct 10, 2017

zeule Oct 10, 2017

glassez Oct 10, 2017

zeule Oct 10, 2017 •

edited

glassez Oct 10, 2017

glassez Oct 10, 2017

zeule Oct 10, 2017

Chocobo1 Oct 10, 2017

zeule Oct 10, 2017

Chocobo1 Oct 10, 2017 •

edited

Chocobo1 Oct 10, 2017

glassez Oct 10, 2017

glassez Oct 10, 2017

zeule commented Oct 11, 2017

		@@ -59,7 +59,7 @@

		Preferences *Preferences::m_instance = 0;

		Preferences::Preferences() {}

		@@ -117,6 +117,7 @@ QIcon GuiIconProvider::generateDifferentSizes(const QIcon &icon)

		return newIcon;
		}

Themes commit-by-commit part 1 #7566

Themes commit-by-commit part 1 #7566

Conversation

zeule commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeule Oct 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Chocobo1 Oct 10, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zeule commented Oct 11, 2017

zeule Oct 10, 2017 •

edited

Chocobo1 Oct 10, 2017 •

edited