Improve code of dir/dev/download views

* Reduce indentation * Remove unused #include <QCursor> * Map context menu position to viewport * Use const when possible
2019-07-27 11:11:36 +02:00 · 2019-07-27 11:11:36 +02:00 · 9f9b9124a4
parent 03d15d5bfb
commit 9f9b9124a4
3 changed files with 148 additions and 142 deletions
--- a/tray/gui/devview.cpp
+++ b/tray/gui/devview.cpp
@ -4,7 +4,6 @@
 #include "../../model/syncthingdevicemodel.h"

 #include <QClipboard>
-#include <QCursor>
 #include <QGuiApplication>
 #include <QHeaderView>
 #include <QMenu>
@ -27,75 +26,80 @@ DevView::DevView(QWidget *parent)
 void DevView::mouseReleaseEvent(QMouseEvent *event)
 {
    QTreeView::mouseReleaseEvent(event);
-    if (const auto *devModel = qobject_cast<SyncthingDeviceModel *>(model())) {
-        const QPoint pos(event->pos());
-        const QModelIndex clickedIndex(indexAt(event->pos()));
-        if (clickedIndex.isValid() && clickedIndex.column() == 1 && !clickedIndex.parent().isValid()) {
-            if (const SyncthingDev *devInfo = devModel->devInfo(clickedIndex)) {
-                const QRect itemRect(visualRect(clickedIndex));
-                if (pos.x() > itemRect.right() - 17) {
-                    emit pauseResumeDev(*devInfo);
-                }
-            }
-        }
+    const auto *const devModel = qobject_cast<const SyncthingDeviceModel *>(model());
+    if (!devModel) {
+        return;
+    }
+    const QPoint pos(event->pos());
+    const QModelIndex clickedIndex(indexAt(event->pos()));
+    if (!clickedIndex.isValid() || clickedIndex.column() != 1 || clickedIndex.parent().isValid()) {
+        return;
+    }
+    const SyncthingDev *const devInfo = devModel->devInfo(clickedIndex);
+    if (!devInfo) {
+        return;
+    }
+    const QRect itemRect(visualRect(clickedIndex));
+    if (pos.x() > itemRect.right() - 17) {
+        emit pauseResumeDev(*devInfo);
    }
 }

 void DevView::showContextMenu(const QPoint &position)
 {
-    if (selectionModel() && selectionModel()->selectedRows(0).size() == 1) {
-        QMenu menu;
-        if (selectionModel()->selectedRows(0).at(0).parent().isValid()) {
-            connect(
-                menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
-                    tr("Copy value")),
-                &QAction::triggered, this, &DevView::copySelectedItem);
-        } else {
-            connect(
-                menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
-                    tr("Copy name")),
-                &QAction::triggered, this, &DevView::copySelectedItem);
-            connect(
-                menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
-                    tr("Copy ID")),
-                &QAction::triggered, this, &DevView::copySelectedItemId);
-        }
-        menu.exec(mapToGlobal(position));
+    if (!selectionModel() || selectionModel()->selectedRows(0).size() != 1) {
+        return;
    }
+    QMenu menu;
+    if (selectionModel()->selectedRows(0).at(0).parent().isValid()) {
+        connect(menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
+                    tr("Copy value")),
+            &QAction::triggered, this, &DevView::copySelectedItem);
+    } else {
+        connect(menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
+                    tr("Copy name")),
+            &QAction::triggered, this, &DevView::copySelectedItem);
+        connect(menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
+                    tr("Copy ID")),
+            &QAction::triggered, this, &DevView::copySelectedItemId);
+    }
+    menu.exec(viewport()->mapToGlobal(position));
 }

 void DevView::copySelectedItem()
 {
-    if (selectionModel() && selectionModel()->selectedRows(0).size() == 1) {
-        const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
-        QString text;
-        if (selectedIndex.parent().isValid()) {
-            // dev attribute
-            text = model()->data(model()->index(selectedIndex.row(), 1, selectedIndex.parent())).toString();
-        } else {
-            // dev name/id
-            text = model()->data(selectedIndex).toString();
-        }
-        if (!text.isEmpty()) {
-            QGuiApplication::clipboard()->setText(text);
-        }
+    if (!selectionModel() || selectionModel()->selectedRows(0).size() != 1) {
+        return;
+    }
+    const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
+    QString text;
+    if (selectedIndex.parent().isValid()) {
+        // dev attribute
+        text = model()->data(model()->index(selectedIndex.row(), 1, selectedIndex.parent())).toString();
+    } else {
+        // dev name/id
+        text = model()->data(selectedIndex).toString();
+    }
+    if (!text.isEmpty()) {
+        QGuiApplication::clipboard()->setText(text);
    }
 }

 void DevView::copySelectedItemId()
 {
-    if (selectionModel() && selectionModel()->selectedRows(0).size() == 1) {
-        const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
-        QString text;
-        if (selectedIndex.parent().isValid()) {
-            // dev attribute: should be handled by copySelectedItemId()
-        } else {
-            // dev name/id
-            text = model()->data(model()->index(0, 1, selectedIndex)).toString();
-        }
-        if (!text.isEmpty()) {
-            QGuiApplication::clipboard()->setText(text);
-        }
+    if (!selectionModel() || selectionModel()->selectedRows(0).size() != 1) {
+        return;
+    }
+    const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
+    QString text;
+    if (selectedIndex.parent().isValid()) {
+        // dev attribute: should be handled by copySelectedItemId()
+    } else {
+        // dev name/id
+        text = model()->data(model()->index(0, 1, selectedIndex)).toString();
+    }
+    if (!text.isEmpty()) {
+        QGuiApplication::clipboard()->setText(text);
    }
 }
 } // namespace QtGui
--- a/tray/gui/dirview.cpp
+++ b/tray/gui/dirview.cpp
@ -6,7 +6,6 @@
 #include "../../widgets/misc/direrrorsdialog.h"

 #include <QClipboard>
-#include <QCursor>
 #include <QGuiApplication>
 #include <QHeaderView>
 #include <QMenu>
@ -72,59 +71,59 @@ void DirView::mouseReleaseEvent(QMouseEvent *event)

 void DirView::showContextMenu(const QPoint &position)
 {
-    if (selectionModel() && selectionModel()->selectedRows(0).size() == 1) {
-        QMenu menu;
-        if (selectionModel()->selectedRows(0).at(0).parent().isValid()) {
-            connect(
-                menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
-                    tr("Copy value")),
-                &QAction::triggered, this, &DirView::copySelectedItem);
-        } else {
-            connect(
-                menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
-                    tr("Copy label/ID")),
-                &QAction::triggered, this, &DirView::copySelectedItem);
-            connect(
-                menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
-                    tr("Copy path")),
-                &QAction::triggered, this, &DirView::copySelectedItemPath);
-        }
-        menu.exec(mapToGlobal(position));
+    if (!selectionModel() || selectionModel()->selectedRows(0).size() != 1) {
+        return;
    }
+    QMenu menu;
+    if (selectionModel()->selectedRows(0).at(0).parent().isValid()) {
+        connect(menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
+                    tr("Copy value")),
+            &QAction::triggered, this, &DirView::copySelectedItem);
+    } else {
+        connect(menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
+                    tr("Copy label/ID")),
+            &QAction::triggered, this, &DirView::copySelectedItem);
+        connect(menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
+                    tr("Copy path")),
+            &QAction::triggered, this, &DirView::copySelectedItemPath);
+    }
+    menu.exec(viewport()->mapToGlobal(position));
 }

 void DirView::copySelectedItem()
 {
-    if (selectionModel() && selectionModel()->selectedRows(0).size() == 1) {
-        const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
-        QString text;
-        if (selectedIndex.parent().isValid()) {
-            // dev attribute
-            text = model()->data(model()->index(selectedIndex.row(), 1, selectedIndex.parent())).toString();
-        } else {
-            // dev label/id
-            text = model()->data(selectedIndex).toString();
-        }
-        if (!text.isEmpty()) {
-            QGuiApplication::clipboard()->setText(text);
-        }
+    if (!selectionModel() || selectionModel()->selectedRows(0).size() != 1) {
+        return;
+    }
+    const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
+    QString text;
+    if (selectedIndex.parent().isValid()) {
+        // dev attribute
+        text = model()->data(model()->index(selectedIndex.row(), 1, selectedIndex.parent())).toString();
+    } else {
+        // dev label/id
+        text = model()->data(selectedIndex).toString();
+    }
+    if (!text.isEmpty()) {
+        QGuiApplication::clipboard()->setText(text);
    }
 }

 void DirView::copySelectedItemPath()
 {
-    if (selectionModel() && selectionModel()->selectedRows(0).size() == 1) {
-        const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
-        QString text;
-        if (selectedIndex.parent().isValid()) {
-            // dev attribute: should be handled by copySelectedItem() only
-        } else {
-            // dev path
-            text = model()->data(model()->index(1, 1, selectedIndex)).toString();
-        }
-        if (!text.isEmpty()) {
-            QGuiApplication::clipboard()->setText(text);
-        }
+    if (!selectionModel() || selectionModel()->selectedRows(0).size() != 1) {
+        return;
+    }
+    const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
+    QString text;
+    if (selectedIndex.parent().isValid()) {
+        // dev attribute: should be handled by copySelectedItem() only
+    } else {
+        // dev path
+        text = model()->data(model()->index(1, 1, selectedIndex)).toString();
+    }
+    if (!text.isEmpty()) {
+        QGuiApplication::clipboard()->setText(text);
    }
 }
 } // namespace QtGui
--- a/tray/gui/downloadview.cpp
+++ b/tray/gui/downloadview.cpp
@ -4,7 +4,6 @@
 #include "../../model/syncthingdownloadmodel.h"

 #include <QClipboard>
-#include <QCursor>
 #include <QGuiApplication>
 #include <QHeaderView>
 #include <QMenu>
@ -27,60 +26,64 @@ DownloadView::DownloadView(QWidget *parent)
 void DownloadView::mouseReleaseEvent(QMouseEvent *event)
 {
    QTreeView::mouseReleaseEvent(event);
-    if (const SyncthingDownloadModel *dlModel = qobject_cast<SyncthingDownloadModel *>(model())) {
-        const QPoint pos(event->pos());
-        const QModelIndex clickedIndex(indexAt(event->pos()));
-        if (clickedIndex.isValid() && clickedIndex.column() == 0) {
-            const QRect itemRect(visualRect(clickedIndex));
-            if (pos.x() > itemRect.right() - 17) {
-                if (clickedIndex.parent().isValid()) {
-                    if (pos.y() < itemRect.y() + itemRect.height() / 2) {
-                        if (const SyncthingItemDownloadProgress *progress = dlModel->progressInfo(clickedIndex)) {
-                            emit openItemDir(*progress);
-                        }
-                    }
-                } else if (const SyncthingDir *dir = dlModel->dirInfo(clickedIndex)) {
-                    emit openDir(*dir);
-                }
+    const auto *const dlModel = qobject_cast<const SyncthingDownloadModel *>(model());
+    if (!dlModel) {
+        return;
+    }
+    const QPoint pos(event->pos());
+    const QModelIndex clickedIndex(indexAt(event->pos()));
+    if (!clickedIndex.isValid() || clickedIndex.column() != 0) {
+        return;
+    }
+    const QRect itemRect(visualRect(clickedIndex));
+    if (pos.x() <= itemRect.right() - 17) {
+        return;
+    }
+    if (clickedIndex.parent().isValid()) {
+        if (pos.y() < itemRect.y() + itemRect.height() / 2) {
+            if (const SyncthingItemDownloadProgress *const progress = dlModel->progressInfo(clickedIndex)) {
+                emit openItemDir(*progress);
            }
        }
+    } else if (const SyncthingDir *const dir = dlModel->dirInfo(clickedIndex)) {
+        emit openDir(*dir);
    }
 }

 void DownloadView::showContextMenu(const QPoint &position)
 {
-    if (selectionModel() && selectionModel()->selectedRows(0).size() == 1) {
-        QMenu menu;
-        if (selectionModel()->selectedRows(0).at(0).parent().isValid()) {
-            connect(
-                menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
-                    tr("Copy value")),
-                &QAction::triggered, this, &DownloadView::copySelectedItem);
-        } else {
-            connect(
-                menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
-                    tr("Copy label/ID")),
-                &QAction::triggered, this, &DownloadView::copySelectedItem);
-        }
-        menu.exec(mapToGlobal(position));
+    if (!selectionModel() || selectionModel()->selectedRows(0).size() != 1) {
+        return;
    }
+    QMenu menu;
+    if (selectionModel()->selectedRows(0).at(0).parent().isValid()) {
+        connect(menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
+                    tr("Copy value")),
+            &QAction::triggered, this, &DownloadView::copySelectedItem);
+    } else {
+        connect(menu.addAction(QIcon::fromTheme(QStringLiteral("edit-copy"), QIcon(QStringLiteral(":/icons/hicolor/scalable/actions/edit-copy.svg"))),
+                    tr("Copy label/ID")),
+            &QAction::triggered, this, &DownloadView::copySelectedItem);
+    }
+    menu.exec(viewport()->mapToGlobal(position));
 }

 void DownloadView::copySelectedItem()
 {
-    if (selectionModel() && selectionModel()->selectedRows(0).size() == 1) {
-        const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
-        QString text;
-        if (selectedIndex.parent().isValid()) {
-            // dev attribute
-            text = model()->data(model()->index(selectedIndex.row(), 1, selectedIndex.parent())).toString();
-        } else {
-            // dev label/id
-            text = model()->data(selectedIndex).toString();
-        }
-        if (!text.isEmpty()) {
-            QGuiApplication::clipboard()->setText(text);
-        }
+    if (!selectionModel() || selectionModel()->selectedRows(0).size() != 1) {
+        return;
+    }
+    const QModelIndex selectedIndex = selectionModel()->selectedRows(0).at(0);
+    QString text;
+    if (selectedIndex.parent().isValid()) {
+        // dev attribute
+        text = model()->data(model()->index(selectedIndex.row(), 1, selectedIndex.parent())).toString();
+    } else {
+        // dev label/id
+        text = model()->data(selectedIndex).toString();
+    }
+    if (!text.isEmpty()) {
+        QGuiApplication::clipboard()->setText(text);
    }
 }
 } // namespace QtGui