Update based on review by Tobias Fella

src/audiomanager.cpp

@@ -336,11 +336,11 @@ bool AudioManager::canGoNext() const
 {
     // TODO: extend with streaming capability
     if (d->m_entry) {
-        int index = DataManager::instance().getQueue().indexOf(d->m_entry->id());
+        int index = DataManager::instance().queue().indexOf(d->m_entry->id());
         if (index >= 0) {
             // check if there is a next track
-            if (index < DataManager::instance().getQueue().count()-1) {
+            if (index < DataManager::instance().queue().count()-1) {
-                Entry* next_entry = DataManager::instance().getEntry(DataManager::instance().getQueue()[index+1]);
+                Entry* next_entry = DataManager::instance().getEntry(DataManager::instance().queue()[index+1]);
                 if (next_entry->enclosure()) {
                     //qDebug() << "Enclosure status" << next_entry->enclosure()->path() << next_entry->enclosure()->status();
                     if (next_entry->enclosure()->status() == Enclosure::Downloaded) {
@@ -357,9 +357,9 @@ void AudioManager::next()
 {
     if (canGoNext()) {
         QMediaPlayer::State previousTrackState = playbackState();
-        int index = DataManager::instance().getQueue().indexOf(d->m_entry->id());
+        int index = DataManager::instance().queue().indexOf(d->m_entry->id());
-        //qDebug() << "Skipping to" << DataManager::instance().getQueue()[index+1];
+        //qDebug() << "Skipping to" << DataManager::instance().queue()[index+1];
-        setEntry(DataManager::instance().getEntry(DataManager::instance().getQueue()[index+1]));
+        setEntry(DataManager::instance().getEntry(DataManager::instance().queue()[index+1]));
         if (previousTrackState == QMediaPlayer::PlayingState) play();
     } else {
         //qDebug() << "Next track cannot be played, changing entry to nullptr";

src/audiomanager.h

@@ -37,15 +37,6 @@ class AudioManager : public QObject
                WRITE setVolume
                NOTIFY volumeChanged)
 
-    /*
-    // The source should not be set directly, but rather through entry
-    // Hence this property is disabled so it cannot be used accidentally in qml
-    Q_PROPERTY(QUrl source
-               READ source
-               WRITE setSource
-               NOTIFY sourceChanged)
-    */
-
     Q_PROPERTY(QMediaPlayer::MediaStatus status
                READ status
                NOTIFY statusChanged)

src/database.cpp

@@ -52,7 +52,7 @@ bool Database::migrateTo1()
     TRUE_OR_RETURN(execute(QStringLiteral("CREATE TABLE IF NOT EXISTS Authors (feed TEXT, id TEXT, name TEXT, uri TEXT, email TEXT);")));
     TRUE_OR_RETURN(execute(QStringLiteral("CREATE TABLE IF NOT EXISTS Enclosures (feed TEXT, id TEXT, duration INTEGER, size INTEGER, title TEXT, type TEXT, url TEXT, playposition INTEGER, downloaded BOOL);"))); //, filename TEXT);")));
     TRUE_OR_RETURN(execute(QStringLiteral("CREATE TABLE IF NOT EXISTS Queue (listnr INTEGER, feed TEXT, id TEXT, playing BOOL);")));
-    TRUE_OR_RETURN(execute(QStringLiteral("CREATE TABLE IF NOT EXISTS Errors (url TEXT, id TEXT, code INTEGER, string TEXT, date INTEGER);")));
+    TRUE_OR_RETURN(execute(QStringLiteral("CREATE TABLE IF NOT EXISTS Errors (url TEXT, id TEXT, code INTEGER, message TEXT, date INTEGER);")));
     TRUE_OR_RETURN(execute(QStringLiteral("PRAGMA user_version = 1;")));
     return true;
 }

src/datamanager.cpp

@@ -62,7 +62,7 @@ DataManager::DataManager()
             query.bindValue(QStringLiteral(":new"), true);
             Database::instance().execute(query);
             while (query.next()) {
-                QString const id = query.value(QStringLiteral("id")).toString();
+                QString id = query.value(QStringLiteral("id")).toString();
                 addToQueue(feedurl, id);
                 if (SettingsManager::self()->autoDownload()) {
                     if (getEntry(id)->hasEnclosure()) {
@@ -107,29 +107,29 @@ DataManager::DataManager()
     //qDebug() << m_queuemap;
 }
 
-Feed* DataManager::getFeed(int const index) const
+Feed* DataManager::getFeed(const int index) const
 {
     return getFeed(m_feedmap[index]);
 }
 
-Feed* DataManager::getFeed(QString const feedurl) const
+Feed* DataManager::getFeed(const QString &feedurl) const
 {
     if (m_feeds[feedurl] == nullptr)
         loadFeed(feedurl);
     return m_feeds[feedurl];
 }
 
-Entry* DataManager::getEntry(int const feed_index, int const entry_index) const
+Entry* DataManager::getEntry(const int feed_index, const int entry_index) const
 {
     return getEntry(m_entrymap[m_feedmap[feed_index]][entry_index]);
 }
 
-Entry* DataManager::getEntry(const Feed* feed, int const entry_index) const
+Entry* DataManager::getEntry(const Feed* feed, const int entry_index) const
 {
     return getEntry(m_entrymap[feed->url()][entry_index]);
 }
 
-Entry* DataManager::getEntry(QString id) const
+Entry* DataManager::getEntry(const QString &id) const
 {
     if (m_entries[id] == nullptr)
         loadEntry(id);
@@ -232,7 +232,7 @@ void DataManager::removeFeed(Feed* feed)
     removeFeed(m_feedmap.indexOf(feed->url()));
 }
 
-void DataManager::removeFeed(const int &index)
+void DataManager::removeFeed(const int index)
 {
     // Get feed pointer
     Feed* feed = getFeed(m_feedmap[index]);
@@ -342,7 +342,7 @@ void DataManager::addFeeds(const QStringList &urls)
     Fetcher::instance().fetch(urls);
 }
 
-Entry* DataManager::getQueueEntry(int const &index) const
+Entry* DataManager::getQueueEntry(int index) const
 {
     return getEntry(m_queuemap[index]);
 }
@@ -352,7 +352,7 @@ int DataManager::queueCount() const
     return m_queuemap.count();
 }
 
-QStringList DataManager::getQueue() const
+QStringList DataManager::queue() const
 {
     return m_queuemap;
 }
@@ -400,7 +400,7 @@ void DataManager::addToQueue(const QString &feedurl, const QString &id)
     Q_EMIT queueEntryAdded(index, id);
 }
 
-void DataManager::moveQueueItem(const int &from, const int &to)
+void DataManager::moveQueueItem(const int from, const int to)
 {
     // First move the items in the internal data structure
     m_queuemap.move(from, to);
@@ -412,7 +412,7 @@ void DataManager::moveQueueItem(const int &from, const int &to)
     Q_EMIT queueEntryMoved(from, to);
 }
 
-void DataManager::removeQueueItem(const int &index)
+void DataManager::removeQueueItem(const int index)
 {
     //qDebug() << m_queuemap;
     // Unset "new" state

src/datamanager.h

@@ -21,12 +21,12 @@ public:
         return _instance;
     }
 
-    Feed* getFeed(int const index) const;
+    Feed* getFeed(const int index) const;
-    Feed* getFeed(QString const feedurl) const;
+    Feed* getFeed(const QString &feedurl) const;
-    Entry* getEntry(int const feed_index, int const entry_index) const;
+    Entry* getEntry(const int feed_index, const int entry_index) const;
-    Entry* getEntry(const Feed* feed, int const entry_index) const;
+    Entry* getEntry(const Feed* feed, const int entry_index) const;
     Entry* getEntry(const EpisodeModel::Type type, const int entry_index) const;
-    Q_INVOKABLE Entry* getEntry(const QString id) const;
+    Q_INVOKABLE Entry* getEntry(const QString &id) const;
     int feedCount() const;
     int entryCount(const int feed_index) const;
     int entryCount(const Feed* feed) const;
@@ -37,21 +37,21 @@ public:
     void addFeed(const QString &url, const bool fetch);
     void addFeeds(const QStringList &urls);
     Q_INVOKABLE void removeFeed(Feed* feed);
-    void removeFeed(const int &index);
+    void removeFeed(const int index);
 
     //Q_INVOKABLE void addEntry(const QString &url);  // TODO: implement these methods
     //Q_INVOKABLE void removeEntry(const QString &url);
     //Q_INVOKABLE void removeEntry(const Feed* feed, const int &index);
 
-    Entry* getQueueEntry(int const &index) const;
+    Entry* getQueueEntry(int index) const;
     int queueCount() const;
-    QStringList getQueue() const;
+    QStringList queue() const;
     Q_INVOKABLE bool entryInQueue(const Entry* entry);
     Q_INVOKABLE bool entryInQueue(const QString &feedurl, const QString &id) const;
     Q_INVOKABLE void addToQueue(const Entry* entry);
     Q_INVOKABLE void addToQueue(const QString &feedurl, const QString &id);
-    Q_INVOKABLE void moveQueueItem(const int &from, const int &to);
+    Q_INVOKABLE void moveQueueItem(const int from, const int to);
-    Q_INVOKABLE void removeQueueItem(const int &index);
+    Q_INVOKABLE void removeQueueItem(const int index);
     Q_INVOKABLE void removeQueueItem(const QString id);
     Q_INVOKABLE void removeQueueItem(Entry* entry);
 

src/entry.cpp

@@ -15,7 +15,7 @@
 #include "datamanager.h"
 #include "fetcher.h"
 
-Entry::Entry(Feed *feed, QString id)
+Entry::Entry(Feed *feed, const QString &id)
     : QObject(nullptr)
     , m_feed(feed)
 {
@@ -127,7 +127,7 @@ QString Entry::baseUrl() const
     return QUrl(m_link).adjusted(QUrl::RemovePath).toString();
 }
 
-void Entry::setRead(const bool read)
+void Entry::setRead(bool read)
 {
     m_read = read;
     Q_EMIT readChanged(m_read);
@@ -142,7 +142,7 @@ void Entry::setRead(const bool read)
     //TODO: can one of the two slots be removed??
 }
 
-void Entry::setNew(const bool state)
+void Entry::setNew(bool state)
 {
     m_new = state;
     Q_EMIT newChanged(m_new);
@@ -213,15 +213,16 @@ QString Entry::cachedImage() const
 {
     // First check for the feed image as fallback
     QString image = m_image;
-    if (image.isEmpty())
+    if (image.isEmpty()) {
         image = m_feed->image();
+    }
 
     if (image.isEmpty()) { // this will only happen if the feed also doesn't have an image
         return QStringLiteral("no-image");
     } else {
         QString imagePath = Fetcher::instance().image(image);
         if (imagePath.isEmpty()) {
-            return imagePath;
+            return QStringLiteral("fetching");
         } else {
             return QStringLiteral("file://") + imagePath;
         }
@@ -233,7 +234,7 @@ bool Entry::queueStatus() const
     return DataManager::instance().entryInQueue(this);
 }
 
-void Entry::setQueueStatus(const bool state)
+void Entry::setQueueStatus(bool state)
 {
     if (state != DataManager::instance().entryInQueue(this)) {
         if (state)

src/entry.h

@@ -40,7 +40,7 @@ class Entry : public QObject
     Q_PROPERTY(bool queueStatus READ queueStatus WRITE setQueueStatus NOTIFY queueStatusChanged)
 
 public:
-    Entry(Feed *feed, QString id);
+    Entry(Feed *feed, const QString &id);
     ~Entry();
 
     QString id() const;
@@ -61,10 +61,10 @@ public:
 
     QString baseUrl() const;
 
-    void setRead(const bool read);
+    void setRead(bool read);
-    void setNew(const bool state);
+    void setNew(bool state);
     void setImage(const QString &url);
-    void setQueueStatus(const bool status);
+    void setQueueStatus(bool status);
 
     Q_INVOKABLE QString adjustedContent(int width, int fontSize);
 

src/episodemodel.cpp

@@ -16,8 +16,6 @@ EpisodeModel::EpisodeModel()
     // maybe even items have been removed.
     connect(&DataManager::instance(), &DataManager::feedEntriesUpdated, this, [this](const QString &url) {
         Q_UNUSED(url)
-        // we have to reset the entire model in case entries are removed or added
-        // because we have no way of knowing where those entries will be added/removed
         beginResetModel();
         endResetModel();
     });

src/error.h

@@ -18,24 +18,24 @@ class Error : public QObject
     Q_PROPERTY(QString url MEMBER url CONSTANT)
     Q_PROPERTY(QString id MEMBER id CONSTANT)
     Q_PROPERTY(int code MEMBER code CONSTANT)
-    Q_PROPERTY(QString string MEMBER string CONSTANT)
+    Q_PROPERTY(QString message MEMBER message CONSTANT)
     Q_PROPERTY(QDateTime date MEMBER date CONSTANT)
     Q_PROPERTY(QString title READ title CONSTANT)
 
 public:
-    Error(const QString url, const QString id, const int code, const QString string, const QDateTime date): QObject(nullptr)
+    Error(const QString url, const QString id, const int code, const QString message, const QDateTime date): QObject(nullptr)
     {
         this->url = url;
         this->id = id;
         this->code = code;
-        this->string = string;
+        this->message = message;
         this->date = date;
     };
 
     QString url;
     QString id;
     int code;
-    QString string;
+    QString message;
     QDateTime date;
 
     QString title() const

src/errorlogmodel.cpp

@@ -25,15 +25,16 @@ ErrorLogModel::ErrorLogModel()
         QString id = query.value(QStringLiteral("id")).toString();
         QString url = query.value(QStringLiteral("url")).toString();
 
-        Error* error = new Error(url, id, query.value(QStringLiteral("code")).toInt(), query.value(QStringLiteral("string")).toString(), QDateTime::fromSecsSinceEpoch(query.value(QStringLiteral("date")).toInt()));
+        Error* error = new Error(url, id, query.value(QStringLiteral("code")).toInt(), query.value(QStringLiteral("message")).toString(), QDateTime::fromSecsSinceEpoch(query.value(QStringLiteral("date")).toInt()));
         m_errors += error;
     }
 }
 
 QVariant ErrorLogModel::data(const QModelIndex &index, int role) const
 {
-    if (role != 0)
+    if (role != 0) {
         return QVariant();
+    }
     return QVariant::fromValue(m_errors[index.row()]);
 }
 
@@ -66,7 +67,7 @@ void ErrorLogModel::monitorErrorMessages(const QString &url, const QString& id,
     query.bindValue(QStringLiteral(":url"), error->url);
     query.bindValue(QStringLiteral(":id"), error->id);
     query.bindValue(QStringLiteral(":code"), error->code);
-    query.bindValue(QStringLiteral(":string"), error->string);
+    query.bindValue(QStringLiteral(":message"), error->message);
     query.bindValue(QStringLiteral(":date"), error->date.toSecsSinceEpoch());
     Database::instance().execute(query);
 }

src/errorlogmodel.h

@@ -31,7 +31,7 @@ public:
 
     Q_INVOKABLE void clearAll();
 
-public Q_SLOTS:
+public:
     void monitorErrorMessages(const QString &url, const QString &id, const int errorCode, const QString &errorString);
 
 private:

src/feed.cpp

@@ -13,7 +13,7 @@
 #include "feed.h"
 #include "fetcher.h"
 
-Feed::Feed(QString const feedurl)
+Feed::Feed(const QString &feedurl)
     : QObject(nullptr)
 {
 
@@ -153,7 +153,7 @@ QString Feed::cachedImage() const
     } else {
         QString imagePath = Fetcher::instance().image(m_image);
         if (imagePath.isEmpty()) {
-            return imagePath;
+            return QStringLiteral("fetching");
         } else {
             return QStringLiteral("file://") + imagePath;
         }

src/feed.h

@@ -39,7 +39,7 @@ class Feed : public QObject
     Q_PROPERTY(EntriesModel *entries MEMBER m_entries CONSTANT)
 
 public:
-    Feed(QString const feedurl);
+    Feed(const QString &feedurl);
 
     ~Feed();
 

src/fetcher.cpp

@@ -221,7 +221,7 @@ void Fetcher::processFeed(Syndication::FeedPtr feed, const QString &url)
     Q_EMIT feedUpdateFinished(url);
 }
 
-bool Fetcher::processEntry(Syndication::ItemPtr entry, const QString &url, const bool &isNewFeed)
+bool Fetcher::processEntry(Syndication::ItemPtr entry, const QString &url, bool isNewFeed)
 {
     //qDebug() << "Processing" << entry->title();
 

src/fetcher.h

@@ -59,7 +59,7 @@ private:
 
     void retrieveFeed(const QString &url);
     void processFeed(Syndication::FeedPtr feed, const QString &url);
-    bool processEntry(Syndication::ItemPtr entry, const QString &url, const bool &isNewFeed);  // returns true if this is a new entry; false if it already existed
+    bool processEntry(Syndication::ItemPtr entry, const QString &url, bool isNewFeed);  // returns true if this is a new entry; false if it already existed
     void processAuthor(const QString &url, const QString &entryId, const QString &authorName, const QString &authorUri, const QString &authorEmail);
     void processEnclosure(Syndication::EnclosurePtr enclosure, Syndication::ItemPtr entry, const QString &feedUrl);
 

src/mpris2/mediaplayer2player.cpp

@@ -357,7 +357,7 @@ void MediaPlayer2Player::setEntry(Entry* entry)
     if (m_audioPlayer) {
         if (m_audioPlayer->entry()) {
             if (m_audioPlayer->entry() == entry) {
-                int queuenr = DataManager::instance().getQueue().indexOf(m_audioPlayer->entry()->id());
+                int queuenr = DataManager::instance().queue().indexOf(m_audioPlayer->entry()->id());
                 //qDebug() << "MPRIS2: Setting entry" << entry->title();
                 m_currentTrackId = QDBusObjectPath(QLatin1String("/org/kde/alligator/playlist/") + QString::number(queuenr)).path();
 

src/qml/DownloadSwipePage.qml

@@ -52,7 +52,7 @@ Kirigami.Page {
             Controls.TabButton {
                 width: parent.parent.width / parent.count
                 height: tabBarHeight
-                text: i18n("Errors")
+                text: i18n("Error Log")
             }
         }
     }
@@ -73,7 +73,7 @@ Kirigami.Page {
         }
 
         ErrorListPage {
-            title: i18n("Errors")
+            title: i18n("Error Log")
         }
     }
 }

src/qml/EntryListPage.qml

@@ -49,7 +49,6 @@ Kirigami.ScrollablePage {
             iconName: "delete"
             text: i18n("Remove feed")
             onTriggered: {
-                if(pageStack.depth > 1)
                 while(pageStack.depth > 1)
                     pageStack.pop()
                 DataManager.removeFeed(feed)

src/qml/EpisodeListPage.qml

@@ -41,7 +41,7 @@ Kirigami.ScrollablePage {
 
         text: i18n(episodeType === EpisodeModel.All ? i18n("No episodes available")
                  : episodeType === EpisodeModel.New ? i18n("No new episodes")
-                 : episodeType === EpisodeModel.Unread ? i18n("No unread episodes")
+                 : episodeType === EpisodeModel.Unread ? i18n("No unplayed episodes")
                  : episodeType === EpisodeModel.Downloaded ? i18n("No downloaded episodes")
                  : episodeType === EpisodeModel.Downloading ? i18n("No downloads in progress")
                  : i18n("No episodes available"))
@@ -61,8 +61,8 @@ Kirigami.ScrollablePage {
     }
 
     ListView {
-        anchors.fill: parent
         id: episodeList
+        anchors.fill: parent
         visible: count !== 0
         model: episodeType === EpisodeModel.Downloading ? DownloadProgressModel
                                                         : episodeModel

src/qml/ErrorListPage.qml

@@ -33,10 +33,8 @@ Kirigami.ScrollablePage {
                 Kirigami.Icon {
                     source: "data-error"
                     property int size: Kirigami.Units.iconSizes.medium
-                    Layout.minimumHeight: size
+                    Layout.preferredHeight: size
-                    Layout.maximumHeight: size
+                    Layout.preferredWidth: size
-                    Layout.minimumWidth: size
-                    Layout.maximumWidth: size
                 }
                 ColumnLayout {
                     spacing: Kirigami.Units.smallSpacing
@@ -57,7 +55,7 @@ Kirigami.ScrollablePage {
                         opacity: 1
                     }
                     Controls.Label {
-                        text: i18n("Error code: ") + error.code + " ·  " + error.string
+                        text: i18n("Error code: ") + error.code + " ·  " + error.message
                         Layout.fillWidth: true
                         elide: Text.ElideRight
                         font: Kirigami.Theme.smallFont

src/qml/ImageWithFallback.qml

@@ -23,10 +23,9 @@ Item {
     property bool isLoading: false
 
     Loader {
-        //active: !isLoading
         id: imageLoader
         anchors.fill: parent
-        sourceComponent: imageSource === "no-image" ? fallbackImg : (imageSource === "" ? loaderSymbol : realImg )
+        sourceComponent: imageSource === "no-image" ? fallbackImg : (imageSource === "fetching" ? loaderSymbol : realImg )
         opacity: root.imageOpacity
         layer.enabled: (root.absoluteRadius > 0 || root.fractionalRadius > 0)
         layer.effect: OpacityMask {
@@ -122,6 +121,5 @@ Item {
         active: isLoading
         sourceComponent: loaderSymbol
         anchors.fill: parent
-        //anchors.centerIn: parent
     }
 }

src/queuemodel.cpp

@@ -13,19 +13,19 @@
 QueueModel::QueueModel(QObject *parent)
     : QAbstractListModel(parent)
 {
-    connect(&DataManager::instance(), &DataManager::queueEntryMoved, this, [this](const int &from, const int &to_orig) {
+    connect(&DataManager::instance(), &DataManager::queueEntryMoved, this, [this](const int from, const int to_orig) {
         int to = (from < to_orig) ? to_orig + 1 : to_orig;
         beginMoveRows(QModelIndex(), from, from, QModelIndex(), to);
         endMoveRows();
         //qDebug() << "Moved entry" << from << "to" << to;
     });
-    connect(&DataManager::instance(), &DataManager::queueEntryAdded, this, [this](const int &pos, const QString &id) {
+    connect(&DataManager::instance(), &DataManager::queueEntryAdded, this, [this](const int pos, const QString &id) {
         Q_UNUSED(id)
         beginInsertRows(QModelIndex(), pos, pos);
         endInsertRows();
         //qDebug() << "Added entry at pos" << pos;
     });
-    connect(&DataManager::instance(), &DataManager::queueEntryRemoved, this, [this](const int &pos, const QString &id) {
+    connect(&DataManager::instance(), &DataManager::queueEntryRemoved, this, [this](const int pos, const QString &id) {
         Q_UNUSED(id)
         beginRemoveRows(QModelIndex(), pos, pos);
         endRemoveRows();