Commit b7744471 authored by Harald Sitter's avatar Harald Sitter 🏳️‍🌈
Browse files

tidy code a bit

- curly brace single line ifs consistently
- default trivial ctors and dtors
- prefer using over typedef
- don't declare multiple vars on the same line
- don't risk detaching containers
- be const when possible
- use ctor init list when possible
- use pragma once
- obey the rule of 5
- use explicit ctors when necessary
- use override where possible

this are partly cpp core guidelines improvements, partly general purpose
clang-tidy improvements, and also to a degree aligning better with how
we write modern code in KDE
parent 332936c2
......@@ -7,8 +7,8 @@
#include "testFileTree.h"
TestFileTree::TestFileTree()
: fl(new File("./autotests/core/dummy.txt", 20))
{
fl = new File("./autotests/core/dummy.txt", 20);
}
TestFileTree::~TestFileTree()
......
......@@ -6,8 +6,7 @@
* SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
***********************************************************************/
#ifndef Config_H
#define Config_H
#pragma once
#include <QObject>
#include <QSet>
......@@ -47,8 +46,6 @@ public:
static const QSet<QByteArray> remoteFsTypes;
};
}
} // namespace Filelight
using Filelight::Config;
#endif
......@@ -5,8 +5,7 @@
* SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
***********************************************************************/
#ifndef DEFINE_H
#define DEFINE_H
#pragma once
#include "Config.h"
#include "version.h"
......@@ -18,5 +17,3 @@
#define APP_NAME "filelight"
#define APP_VERSION FILELIGHT_VERSION_STRING
#define APP_PRETTYNAME "Filelight"
#endif
......@@ -6,16 +6,16 @@
* SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
***********************************************************************/
#ifndef FILETREE_H
#define FILETREE_H
#pragma once
#include <cstdlib>
#include <KFormat>
#include <QByteArray> //qstrdup
#include <stdlib.h>
#include <QByteArray> //qstrdup
typedef quint64 FileSize;
typedef quint64 Dirsize; //**** currently unused
using FileSize = quint64;
using DirSize = quint64; //**** currently unused
class Folder;
......@@ -32,7 +32,7 @@ public:
, m_size(size)
{
}
virtual ~File()
~File() override
{
delete[] m_name;
}
......@@ -99,8 +99,7 @@ protected:
FileSize m_size; // in units of bytes; sum of all children's sizes
private:
File(const File &);
void operator=(const File &);
Q_DISABLE_COPY_MOVE(File)
};
class Folder : public File
......@@ -108,9 +107,8 @@ class Folder : public File
Q_OBJECT
Q_PROPERTY(uint children MEMBER m_children CONSTANT)
public:
Folder(const char *name)
explicit Folder(const char *name)
: File(name, 0)
, m_children(0)
{
} // DON'T pass the full path!
......@@ -130,7 +128,7 @@ public:
Folder *duplicate() const
{
Folder *ret = new Folder(m_name);
auto ret = new Folder(m_name);
for (const File *child : files) {
if (child->isFolder()) {
ret->append(((Folder *)child)->duplicate());
......@@ -200,11 +198,8 @@ private:
files.append(p);
}
uint m_children;
uint m_children = 0;
private:
Folder(const Folder &); // undefined
void operator=(const Folder &); // undefined
Q_DISABLE_COPY_MOVE(Folder)
};
#endif
......@@ -34,11 +34,13 @@ void HistoryAction::setHelpText(const QUrl &url)
void HistoryAction::push(const QUrl &path)
{
if (path.isEmpty())
if (path.isEmpty()) {
return;
}
if (m_list.isEmpty() || (!m_list.isEmpty() && (m_list.last() != path)))
if (m_list.isEmpty() || (!m_list.isEmpty() && (m_list.last() != path))) {
m_list.append(path);
}
setHelpText(path);
setEnabled(true);
......@@ -47,8 +49,9 @@ void HistoryAction::push(const QUrl &path)
QUrl HistoryAction::pop()
{
const QUrl s = m_list.takeLast();
if (!m_list.isEmpty())
if (!m_list.isEmpty()) {
setHelpText(m_list.last());
}
setEnabled();
return s;
......
......@@ -5,8 +5,7 @@
* SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
***********************************************************************/
#ifndef HISTORYACTION_H
#define HISTORYACTION_H
#pragma once
#include <QAction>
#include <QUrl>
......@@ -71,5 +70,3 @@ private Q_SLOTS:
private:
HistoryAction *m_b, *m_f, *m_receiver;
};
#endif
......@@ -27,8 +27,7 @@ QStringList LocalLister::s_remoteMounts;
QStringList LocalLister::s_localMounts;
LocalLister::LocalLister(const QString &path, QList<Folder *> *cachedTrees, ScanManager *parent)
: QThread()
, m_path(path)
: m_path(path)
, m_trees(cachedTrees)
, m_parent(parent)
{
......@@ -36,10 +35,12 @@ LocalLister::LocalLister(const QString &path, QList<Folder *> *cachedTrees, Scan
// TODO empty directories is not ideal as adds to fileCount incorrectly
QStringList list(Config::skipList);
if (!Config::scanAcrossMounts)
if (!Config::scanAcrossMounts) {
list += s_localMounts;
if (!Config::scanRemoteMounts)
}
if (!Config::scanRemoteMounts) {
list += s_remoteMounts;
}
for (const QString &ignorePath : std::as_const(list)) {
if (ignorePath.startsWith(path)) {
......@@ -151,7 +152,8 @@ Folder *LocalLister::scan(const QByteArray &path, const QByteArray &dirname)
void LocalLister::readMounts()
{
for (const QStorageInfo &storage : QStorageInfo::mountedVolumes()) {
const auto volumes = QStorageInfo::mountedVolumes();
for (const QStorageInfo &storage : volumes) {
if (storage.isRoot()) {
continue;
}
......
......@@ -5,8 +5,7 @@
* SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
***********************************************************************/
#ifndef LOCALLISTER_H
#define LOCALLISTER_H
#pragma once
#include <QByteArray>
#include <QMutex>
......@@ -41,8 +40,7 @@ private:
Folder *scan(const QByteArray &, const QByteArray &);
private:
static QStringList s_localMounts, s_remoteMounts; // TODO namespace
static QStringList s_localMounts;
static QStringList s_remoteMounts; // TODO namespace
};
}
#endif
} // namespace Filelight
......@@ -159,7 +159,7 @@ bool MainContext::slotScanUrl(const QUrl &url)
return false;
}
void MainContext::urlAboutToChange()
void MainContext::urlAboutToChange() const
{
// called when part's URL is about to change internally
// the part will then create the Map and emit completed()
......@@ -220,7 +220,7 @@ void MainContext::updateURL(const QUrl &u)
setUrl(u);
}
void MainContext::rescanSingleDir(const QUrl &dirUrl)
void MainContext::rescanSingleDir(const QUrl &dirUrl) const
{
if (m_manager->running()) {
m_manager->abort();
......@@ -241,7 +241,7 @@ void MainContext::setUrl(const QUrl &url)
Q_EMIT urlChanged();
}
void MainContext::configFilelight()
void MainContext::configFilelight() const
{
auto dialog = new SettingsDialog(nullptr);
......
......@@ -59,13 +59,13 @@ public Q_SLOTS:
bool slotScanUrl(const QUrl &);
bool slotScanPath(const QString &);
void urlAboutToChange();
void urlAboutToChange() const;
bool openUrl(const QUrl &);
void configFilelight();
void configFilelight() const;
void updateURL(const QUrl &);
void rescanSingleDir(const QUrl &);
void rescanSingleDir(const QUrl &) const;
private:
void setupActions(QQmlApplicationEngine *engine);
......
......@@ -9,6 +9,7 @@
#include "item.h"
#include <cmath> //::segmentAt()
#include <utility>
#include <QApplication> //sendEvent
#include <QClipboard>
......@@ -44,16 +45,8 @@
RadialMap::Item::Item(QQuickItem *parent)
: QQuickPaintedItem(parent)
, m_tree(nullptr)
, m_focus(nullptr)
, m_map()
, m_rootSegment(nullptr) // TODO we don't delete it, *shrug*
, m_toBeDeleted(nullptr)
{
// setAcceptDrops(true);
// setMinimumSize(350, 250);
// setImplicitWidth(350);
// setImplicitHeight(250);
setAcceptedMouseButtons(Qt::LeftButton | Qt::RightButton);
setAcceptHoverEvents(true);
setFlag(QQuickItem::ItemAcceptsDrops, true);
......@@ -161,8 +154,9 @@ void RadialMap::Item::create(Folder *tree)
void RadialMap::Item::mousePressEvent(QMouseEvent *e)
{
if (!isEnabled())
if (!isEnabled()) {
return;
}
// m_focus is set correctly (I've been strict, I assure you it is correct!)
......@@ -240,13 +234,13 @@ void RadialMap::Item::mousePressEvent(QMouseEvent *e)
QAction *clicked = popup.exec(e->globalPos(), nullptr);
if (openFileManager && clicked == openFileManager) {
KIO::OpenUrlJob *job = new KIO::OpenUrlJob(url, QStringLiteral("inode/directory"), nullptr);
auto *job = new KIO::OpenUrlJob(url, QStringLiteral("inode/directory"), nullptr);
job->setUiDelegate(new KIO::JobUiDelegate(KJobUiDelegate::AutoHandlingEnabled, nullptr));
job->start();
} else if (rescanAction && clicked == rescanAction) {
Q_EMIT rescanRequested(url);
} else if (openTerminal && clicked == openTerminal) {
KTerminalLauncherJob *job = new KTerminalLauncherJob(QString(), this);
auto *job = new KTerminalLauncherJob(QString(), this);
job->setUiDelegate(new KDialogJobUiDelegate(KJobUiDelegate::AutoHandlingEnabled, nullptr));
job->setWorkingDirectory(url.path());
job->start();
......@@ -263,7 +257,7 @@ void RadialMap::Item::mousePressEvent(QMouseEvent *e)
job->setUiDelegate(new KIO::JobUiDelegate(KJobUiDelegate::AutoHandlingEnabled, nullptr));
job->start();
} else if (clicked == copyClipboard) {
QMimeData *mimedata = new QMimeData();
auto *mimedata = new QMimeData();
mimedata->setUrls(QList<QUrl>() << url);
QApplication::clipboard()->setMimeData(mimedata, QClipboard::Clipboard);
} else if (clicked == deleteItem && m_focus->file() != m_tree) {
......@@ -296,8 +290,9 @@ void RadialMap::Item::deleteJobFinished(KJob *job)
m_focus = nullptr;
m_map.make(m_tree, true);
update();
} else
} else {
KMessageBox::error(nullptr, job->errorString(), i18n("Error while deleting"));
}
}
void RadialMap::Item::createFromCache(Folder *tree)
......@@ -324,8 +319,9 @@ void RadialMap::Item::resizeTimeout() // slot
// the segments are about to erased!
// this was a horrid bug, and proves the OO programming should be obeyed always!
m_focus = nullptr;
if (m_tree)
if (m_tree) {
m_map.make(m_tree, true);
}
update();
}
......@@ -379,8 +375,9 @@ void RadialMap::Item::zoomOut() // slot
m_focus = nullptr;
++m_map.m_visibleDepth;
m_map.make(m_tree);
if (m_map.m_visibleDepth > Config::defaultRingDepth)
if (m_map.m_visibleDepth > Config::defaultRingDepth) {
Config::defaultRingDepth = m_map.m_visibleDepth;
}
update();
}
......@@ -469,10 +466,12 @@ void RadialMap::Item::paintExplodedLabels(QPainter &paint) const
return (item1->level > item2->level);
}
if (angle1 > 5760)
if (angle1 > 5760) {
angle1 -= 5760;
if (angle2 > 5760)
}
if (angle2 > 5760) {
angle2 -= 5760;
}
return (angle1 < angle2);
});
......@@ -490,7 +489,7 @@ void RadialMap::Item::paintExplodedLabels(QPainter &paint) const
}
// used in next two steps
bool varySizes;
bool varySizes = 0;
//**** should perhaps use doubles
int *sizes = new int[m_map.m_visibleDepth + 1]; //**** make sizes an array of floats I think instead (or doubles)
......@@ -510,7 +509,7 @@ void RadialMap::Item::paintExplodedLabels(QPainter &paint) const
// determine current range of levels to draw for
uint range = 0;
for (Label *label : list) {
for (const auto &label : std::as_const(list)) {
range = qMax(range, label->level);
//**** better way would just be to assign if nothing is range
......@@ -562,7 +561,8 @@ void RadialMap::Item::paintExplodedLabels(QPainter &paint) const
const bool rightSide = (label->angle < 1440 || label->angle > 4320);
double sinra, cosra;
double sinra;
double cosra;
const double ra = M_PI / 2880 * label->angle; // convert to radians
sincos(ra, &sinra, &cosra);
......@@ -588,7 +588,7 @@ void RadialMap::Item::paintExplodedLabels(QPainter &paint) const
int middleX = targetX - (tan(ra) > 0 ? (startY - targetY) / tan(ra) : 0);
int textY = startY + lineSpacing;
int textX;
int textX = 0;
const int textWidth = fontMetrics.boundingRect(string).width() + LABEL_TEXT_HMARGIN;
if (rightSide) {
if (startX + minTextWidth > width() || textY < fontHeight || middleX < targetX) {
......@@ -649,7 +649,7 @@ void RadialMap::Item::paintExplodedLabels(QPainter &paint) const
// 5. Render labels
QFont font;
for (Label *label : list) {
for (const auto &label : std::as_const(list)) {
if (varySizes) {
//**** how much overhead in making new QFont each time?
// (implicate sharing remember)
......@@ -667,25 +667,27 @@ void RadialMap::Item::paintExplodedLabels(QPainter &paint) const
delete[] sizes;
}
void RadialMap::Item::hoverEnterEvent(QHoverEvent *)
void RadialMap::Item::hoverEnterEvent(QHoverEvent * /*event*/)
{
if (!m_focus)
if (!m_focus) {
return;
}
setCursor(Qt::PointingHandCursor);
Q_EMIT mouseHover(m_focus->file()->displayPath());
update();
}
void RadialMap::Item::hoverLeaveEvent(QHoverEvent *)
void RadialMap::Item::hoverLeaveEvent(QHoverEvent * /*event*/)
{
}
void RadialMap::Item::dropEvent(QDropEvent *e)
{
QList<QUrl> uriList = KUrlMimeData::urlsFromMimeData(e->mimeData());
if (!uriList.isEmpty())
if (!uriList.isEmpty()) {
Q_EMIT giveMeTreeFor(uriList.first());
}
}
void RadialMap::Item::dragEnterEvent(QDragEnterEvent *e)
......@@ -698,8 +700,9 @@ void RadialMap::Item::dragEnterEvent(QDragEnterEvent *e)
bool RadialMap::Item::event(QEvent *e)
{
if (e->type() == QEvent::ApplicationPaletteChange || e->type() == QEvent::PaletteChange)
if (e->type() == QEvent::ApplicationPaletteChange || e->type() == QEvent::PaletteChange) {
m_map.paint();
}
return QQuickPaintedItem::event(e);
}
......@@ -763,7 +766,8 @@ void RadialMap::Item::hoverMoveEvent(QHoverEvent *e)
QFontMetrics fontMetrics(m_tooltip.fontMetrics());
int tooltipWidth = 0;
int tooltipHeight = 0;
for (const QString &part : string.split(QLatin1Char('\n'))) {
const auto parts = string.split(QLatin1Char('\n'));
for (const QString &part : parts) {
tooltipHeight += fontMetrics.height();
tooltipWidth = qMax(tooltipWidth, fontMetrics.horizontalAdvance(part));
}
......@@ -795,8 +799,9 @@ const RadialMap::Segment *RadialMap::Item::segmentAt(QPointF e) const
e -= m_offset;
if (m_map.m_signature.isEmpty())
if (m_map.m_signature.isEmpty()) {
return nullptr;
}
if (e.x() <= m_map.width() && e.y() <= m_map.height()) {
// transform to cartesian coords
......@@ -819,16 +824,19 @@ const RadialMap::Segment *RadialMap::Item::segmentAt(QPointF e) const
uint a = (uint)(acos((double)e.x() / length) * 916.736); // 916.7324722 = #radians in circle * 16
// acos only understands 0-180 degrees
if (e.y() < 0)
if (e.y() < 0) {
a = 5760 - a;
}
for (Segment *segment : m_map.m_signature[depth]) {
if (segment->intersects(a))
if (segment->intersects(a)) {
return segment;
}
}
}
} else
} else {
return m_rootSegment; // hovering over inner circle
}
}
return nullptr;
......
......@@ -100,6 +100,8 @@ private:
Segment *m_rootSegment = nullptr;
const Segment *m_toBeDeleted = nullptr;
QLabel m_tooltip;
Q_DISABLE_COPY_MOVE(Item)
};
} // namespace RadialMap
......@@ -38,8 +38,15 @@ public:
const unsigned int level;
const int angle;
int targetX, targetY, middleX, startY, startX;
int textX, textY, tw, th;
int targetX = 0;
int targetY = 0;
int middleX = 0;
int startY = 0;
int startX = 0;
int textX = 0;
int textY = 0;
int tw = 0;
int th = 0;
QString qs;
};
......
......@@ -8,6 +8,8 @@
#include "filelight_debug.h"
#include <utility>
#ifdef Q_OS_WINDOWS
#include <winrt/Windows.UI.ViewManagement.h>
#pragma comment(lib, "windowsapp")
......@@ -145,12 +147,13 @@ void RadialMap::Map::findVisibleDepth(const Folder *dir, uint currentDepth)
qDebug() << "changing visual depth" << m_visibleDepth << currentDepth;
m_visibleDepth = currentDepth;
}
if (m_visibleDepth >= stopDepth)
if (m_visibleDepth >= stopDepth) {
return;
}
for (File *file : dir->files) {
if (file->isFolder() && file->size() > m_minSize) {
findVisibleDepth((Folder *)file, currentDepth + 1); // if no files greater than min size the depth is still recorded
findVisibleDepth(dynamic_cast<Folder *>(file), currentDepth + 1); // if no files greater than min size the depth is still recorded
}
}
}
......@@ -160,8 +163,9 @@ bool RadialMap::Map::build(const Folder *const dir, const uint depth, uint a_sta
{
// first iteration: dir == m_root
if (dir->children() == 0) // we do fileCount rather than size to avoid chance of divide by zero later
if (dir->children() == 0) { // we do fileCount rather than size to avoid chance of divide by zero later
return false;
}
FileSize hiddenSize = 0;
uint hiddenFileCount = 0;
......@@ -170,21 +174,21 @@ bool RadialMap::Map::build(const Folder *const dir, const uint depth, uint a_sta
if (file->size() < m_limits[depth] * 6) { // limit is half a degree? we want at least 3 degrees
hiddenSize += file->size();
if (file->isFolder()) { //**** considered virtual, but dir wouldn't count itself!
hiddenFileCount += static_cast<const Folder *>(file)->children(); // need to add one to count the dir as well
hiddenFileCount += dynamic_cast<const Folder *>(file)->children(); // need to add one to count the dir as well
}
++hiddenFileCount;
continue;
}
unsigned int a_len = (unsigned int)(5760 * ((double)file->size() / (double)m_root->size()));
auto a_len = (unsigned int)(5760 * ((double)file->size() / (double)m_root->size()));
Segment *s = new Segment(file, a_start, a_len);
auto *s = new Segment(file, a_start, a_len);
m_signature[depth].append(s);
if (file->isFolder()) {
if (depth != m_visibleDepth) {
// recurse
s->m_hasHiddenChildren = build((Folder *)file, depth + 1, a_start, a_start + a_len);
s->m_hasHiddenChildren = build(dynamic_cast<Folder *>(file), depth + 1, a_start, a_start + a_len);
} else {
s->m_hasHiddenChildren = true;
}
......@@ -253,10 +257,15 @@ void RadialMap::Map::colorise()
return;
}
QColor cp, cb;
QColor cp;
QColor cb;
double darkness = 1;
double contrast = (double)Config::contrast / (double)100;
int h, s1, s2, v1, v2;
int h = 0;
int s1 = 0;
int s2 = 0;
int v1 = 0;
int v2 = 0;
QPalette palette;
#ifdef Q_OS_WINDOWS
......@@ -272,7 +281,7 @@ void RadialMap::Map::colorise()
double deltaBlue = (double)(kdeColour[0].blue() - kdeColour[1].blue()) / 2880;
for (uint i = 0; i <= m_visibleDepth; ++i, darkness += 0.04) {
for (Segment *segment : m_signature[i]) {
for (const auto &segment : std::as_const(m_signature[i])) {
switch (Config::scheme) {
case Filelight::KDE: {
// gradient will work by figuring out rgb delta values for 360 degrees
......@@ -280,8 +289,9 @@ void RadialMap::Map::colorise()
int a = segment->start();
if (a > 2880)
if (a > 2880) {
a = 2880 - (a - 2880);
}