Skip to content

Commit dd96fee

Browse files
chenneschennes
committed
Start: Reviewer and Linter comments
1 parent 49033da commit dd96fee

File tree

6 files changed

+100
-104
lines changed

6 files changed

+100
-104
lines changed

‎src/Mod/Start/App/DisplayedFilesModel.cpp‎

Lines changed: 53 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -26,33 +26,31 @@
2626
#include <boost/algorithm/string/predicate.hpp>
2727
#include <QByteArray>
2828
#include <QCryptographicHash>
29-
#include <QDateTime>
30-
#include <QDir>
31-
#include <QFile>
3229
#include <QFileInfo>
3330
#include <QProcess>
34-
#include <QStandardPaths>
31+
#include <QTimeZone>
32+
#include <QThreadPool>
3533
#include <QUrl>
3634
#endif
3735

3836
#include "DisplayedFilesModel.h"
3937

40-
#include <QThreadPool>
4138

4239
#include "FileUtilities.h"
40+
#include "ThumbnailSource.h"
4341
#include <App/Application.h>
4442
#include <App/ProjectFile.h>
4543
#include <Base/FileInfo.h>
46-
#include <Base/Stream.h>
4744
#include <Base/TimeInfo.h>
45+
#include <Base/Stream.h>
46+
4847

4948
using namespace Start;
5049

5150

5251
namespace
5352
{
54-
55-
std::string humanReadableSize(unsigned int bytes)
53+
std::string humanReadableSize(const unsigned int bytes)
5654
{
5755
static const std::vector<std::string> siPrefix {
5856
"b",
@@ -96,30 +94,29 @@ FileStats fileInfoFromFreeCADFile(const std::string& path)
9694

9795
/// Load the thumbnail image data (if any) that is stored in an FCStd file.
9896
/// \returns The image bytes, or an empty QByteArray (if no thumbnail was stored)
99-
QByteArray loadFCStdThumbnail(const std::string& pathToFCStdFile)
97+
QByteArray loadFCStdThumbnail(const QString& pathToFCStdFile)
10098
{
101-
App::ProjectFile proj(pathToFCStdFile);
102-
if (proj.loadDocument()) {
99+
if (App::ProjectFile proj(pathToFCStdFile.toStdString()); proj.loadDocument()) {
103100
try {
104-
std::string thumbnailFile = getUniquePNG(pathToFCStdFile).toStdString();
101+
const QString thumbnailFile = getUniquePNG(pathToFCStdFile);
105102
if (!useCachedPNG(thumbnailFile, pathToFCStdFile)) {
106-
static std::string thumb = getThumbnailsImage();
107-
if (proj.containsFile(thumb)) {
103+
static const QString thumb = getThumbnailsImage();
104+
if (proj.containsFile(thumb.toStdString())) {
108105
createThumbnailsDir();
109-
Base::FileInfo fi(thumbnailFile);
106+
const Base::FileInfo fi(thumbnailFile.toStdString());
110107
Base::ofstream str(fi, std::ios::out | std::ios::binary);
111-
proj.readInputFileDirect(thumb, str);
108+
proj.readInputFileDirect(thumb.toStdString(), str);
112109
str.close();
113110
}
114111
}
115112

116-
auto inputFile = QFile(QString::fromStdString(thumbnailFile));
117-
if (inputFile.exists()) {
113+
if (auto inputFile = QFile(thumbnailFile); inputFile.exists()) {
118114
inputFile.open(QIODevice::OpenModeFlag::ReadOnly);
119115
return inputFile.readAll();
120116
}
121117
}
122118
catch (...) {
119+
Base::Console().Log("Failed to load thumbnail for %s", pathToFCStdFile.toStdString());
123120
}
124121
}
125122
return {};
@@ -128,12 +125,17 @@ QByteArray loadFCStdThumbnail(const std::string& pathToFCStdFile)
128125
FileStats getFileInfo(const std::string& path)
129126
{
130127
FileStats result;
131-
Base::FileInfo file(path);
128+
const Base::FileInfo file(path);
132129
if (file.hasExtension("FCStd")) {
133130
result = fileInfoFromFreeCADFile(path);
134131
}
135132
else {
136-
file.lastModified();
133+
Base::TimeInfo lastModified = file.lastModified();
134+
std::string lmString = QDateTime::fromSecsSinceEpoch(lastModified.getTime_t())
135+
.toTimeZone(QTimeZone::utc())
136+
.toString(Qt::ISODate)
137+
.toStdString();
138+
result.insert(std::make_pair(DisplayedFilesModelRoles::modifiedTime, lmString));
137139
}
138140
result.insert(std::make_pair(DisplayedFilesModelRoles::path, path));
139141
result.insert(std::make_pair(DisplayedFilesModelRoles::size, humanReadableSize(file.size())));
@@ -145,14 +147,12 @@ bool freecadCanOpen(const QString& extension)
145147
{
146148
std::string ext = extension.toStdString();
147149
auto importTypes = App::GetApplication().getImportTypes();
148-
return std::find_if(importTypes.begin(),
149-
importTypes.end(),
150-
[&ext](const auto& item) {
151-
return boost::iequals(item, ext);
152-
})
150+
return std::ranges::find_if(importTypes,
151+
[&ext](const auto& item) {
152+
return boost::iequals(item, ext);
153+
})
153154
!= importTypes.end();
154155
}
155-
156156
} // namespace
157157

158158
DisplayedFilesModel::DisplayedFilesModel(QObject* parent)
@@ -166,15 +166,14 @@ int DisplayedFilesModel::rowCount(const QModelIndex& parent) const
166166
return static_cast<int>(_fileInfoCache.size());
167167
}
168168

169-
QVariant DisplayedFilesModel::data(const QModelIndex& index, int roleAsInt) const
169+
QVariant DisplayedFilesModel::data(const QModelIndex& index, int role) const
170170
{
171-
int row = index.row();
171+
const int row = index.row();
172172
if (row < 0 || row >= static_cast<int>(_fileInfoCache.size())) {
173173
return {};
174174
}
175-
auto mapEntry = _fileInfoCache.at(row);
176-
auto role = static_cast<DisplayedFilesModelRoles>(roleAsInt);
177-
switch (role) {
175+
const auto mapEntry = _fileInfoCache.at(row);
176+
switch (const auto roleAsType = static_cast<DisplayedFilesModelRoles>(role)) {
178177
case DisplayedFilesModelRoles::author: // NOLINT(bugprone-branch-clone)
179178
[[fallthrough]];
180179
case DisplayedFilesModelRoles::baseName:
@@ -192,32 +191,34 @@ QVariant DisplayedFilesModel::data(const QModelIndex& index, int roleAsInt) cons
192191
case DisplayedFilesModelRoles::path:
193192
[[fallthrough]];
194193
case DisplayedFilesModelRoles::size:
195-
if (mapEntry.find(role) != mapEntry.end()) {
196-
return QString::fromStdString(mapEntry.at(role));
197-
}
198-
else {
199-
return {};
194+
if (mapEntry.contains(roleAsType)) {
195+
return QString::fromStdString(mapEntry.at(roleAsType));
200196
}
197+
return {};
201198
case DisplayedFilesModelRoles::image: {
202-
auto path = QString::fromStdString(mapEntry.at(DisplayedFilesModelRoles::path));
203-
if (_imageCache.contains(path)) {
199+
if (const auto path =
200+
QString::fromStdString(mapEntry.at(DisplayedFilesModelRoles::path));
201+
_imageCache.contains(path)) {
204202
return _imageCache[path];
205203
}
206204
break;
207205
}
208206
default:
209207
break;
210208
}
211-
switch (roleAsInt) {
209+
switch (role) {
212210
case Qt::ItemDataRole::ToolTipRole:
213211
return QString::fromStdString(mapEntry.at(DisplayedFilesModelRoles::path));
212+
default:
213+
// No other role gets handled
214+
break;
214215
}
215216
return {};
216217
}
217218

218219
void DisplayedFilesModel::addFile(const QString& filePath)
219220
{
220-
QFileInfo qfi(filePath);
221+
const QFileInfo qfi(filePath);
221222
if (!qfi.isReadable()) {
222223
return;
223224
}
@@ -228,13 +229,12 @@ void DisplayedFilesModel::addFile(const QString& filePath)
228229

229230
_fileInfoCache.emplace_back(getFileInfo(filePath.toStdString()));
230231
if (qfi.suffix().toLower() == QLatin1String("fcstd")) {
231-
auto thumbnail = loadFCStdThumbnail(filePath.toStdString());
232-
if (!thumbnail.isEmpty()) {
232+
if (const auto thumbnail = loadFCStdThumbnail(filePath); !thumbnail.isEmpty()) {
233233
_imageCache.insert(filePath, thumbnail);
234234
}
235235
}
236236
else {
237-
auto runner = new ThumbnailSource(filePath);
237+
const auto runner = new ThumbnailSource(filePath);
238238
connect(runner->signals(),
239239
&ThumbnailSourceSignals::thumbnailAvailable,
240240
this,
@@ -251,16 +251,16 @@ void DisplayedFilesModel::clear()
251251
QHash<int, QByteArray> DisplayedFilesModel::roleNames() const
252252
{
253253
static QHash<int, QByteArray> nameMap {
254-
std::make_pair(int(DisplayedFilesModelRoles::author), "author"),
255-
std::make_pair(int(DisplayedFilesModelRoles::baseName), "baseName"),
256-
std::make_pair(int(DisplayedFilesModelRoles::company), "company"),
257-
std::make_pair(int(DisplayedFilesModelRoles::creationTime), "creationTime"),
258-
std::make_pair(int(DisplayedFilesModelRoles::description), "description"),
259-
std::make_pair(int(DisplayedFilesModelRoles::image), "image"),
260-
std::make_pair(int(DisplayedFilesModelRoles::license), "license"),
261-
std::make_pair(int(DisplayedFilesModelRoles::modifiedTime), "modifiedTime"),
262-
std::make_pair(int(DisplayedFilesModelRoles::path), "path"),
263-
std::make_pair(int(DisplayedFilesModelRoles::size), "size"),
254+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::author), "author"),
255+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::baseName), "baseName"),
256+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::company), "company"),
257+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::creationTime), "creationTime"),
258+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::description), "description"),
259+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::image), "image"),
260+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::license), "license"),
261+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::modifiedTime), "modifiedTime"),
262+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::path), "path"),
263+
std::make_pair(static_cast<int>(DisplayedFilesModelRoles::size), "size"),
264264
};
265265
return nameMap;
266266
}

‎src/Mod/Start/App/DisplayedFilesModel.h‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,13 @@
2121
* *
2222
***************************************************************************/
2323

24-
#ifndef FREECAD_START_DISPLAYEDFILESMODEL_H
25-
#define FREECAD_START_DISPLAYEDFILESMODEL_H
24+
#ifndef FREECAD_START_DISPLAYED_FILES_MODEL_H
25+
#define FREECAD_START_DISPLAYED_FILES_MODEL_H
2626

2727
#include <QAbstractListModel>
2828
#include <Base/Parameter.h>
2929

3030
#include "../StartGlobal.h"
31-
#include "ThumbnailSource.h"
3231

3332
namespace Start
3433
{
@@ -70,11 +69,6 @@ class StartExport DisplayedFilesModel: public QAbstractListModel
7069
/// DisplayedFilesModelRoles enumeration
7170
QHash<int, QByteArray> roleNames() const override;
7271

73-
/// Destroy and recreate the cache of info about the files. Should be connected to a signal
74-
/// indicating when some piece of information about the files has changed. Does NOT generate
75-
/// a new list of files, only re-caches the existing ones.
76-
void reCacheFileInfo();
77-
7872
/// Process a new thumbnail produces by some sort of worker thread
7973
void processNewThumbnail(const QString& file, const QByteArray& thumbnail);
8074

@@ -85,4 +79,4 @@ class StartExport DisplayedFilesModel: public QAbstractListModel
8579

8680
} // namespace Start
8781

88-
#endif // FREECAD_START_DISPLAYEDFILESMODEL_H
82+
#endif // FREECAD_START_DISPLAYED_FILES_MODEL_H

‎src/Mod/Start/App/FileUtilities.cpp‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,19 @@
2424
#include "PreCompiled.h"
2525
#ifndef _PreComp_
2626
#include <QCryptographicHash>
27+
#include <QDir>
28+
#include <QFileInfo>
2729
#include <QStandardPaths>
30+
#include <QString>
2831
#include <QUrl>
2932
#endif
3033

3134
#include "FileUtilities.h"
32-
#include <Base/TimeInfo.h>
3335

34-
using namespace Start;
3536

36-
std::string Start::getThumbnailsImage()
37+
QString Start::getThumbnailsImage()
3738
{
38-
return "thumbnails/Thumbnail.png";
39+
return QLatin1String("thumbnails/Thumbnail.png");
3940
}
4041

4142
QString Start::getThumbnailsName()
@@ -61,35 +62,34 @@ QString Start::getThumbnailsDir()
6162
void Start::createThumbnailsDir()
6263
{
6364
const QString name = getThumbnailsName();
64-
const QDir dir(getThumbnailsParentDir());
65-
if (!dir.exists(name)) {
66-
dir.mkpath(name);
65+
if (const QDir dir(getThumbnailsParentDir()); !dir.exists(name)) {
66+
[[maybe_unused]] bool result = dir.mkpath(name);
6767
}
6868
}
6969

70-
QString Start::getMD5Hash(const std::string& path)
70+
QString Start::getMD5Hash(const QString& path)
7171
{
7272
// Use MD5 hash as specified here:
7373
// https://specifications.freedesktop.org/thumbnail-spec/0.8.0/thumbsave.html
74-
QUrl url(QString::fromStdString(path));
75-
url.setScheme(QString::fromLatin1("file"));
74+
QUrl url(path);
75+
url.setScheme(QStringLiteral("file"));
7676
QCryptographicHash hash(QCryptographicHash::Md5);
7777
hash.addData(url.toEncoded());
78-
QByteArray ba = hash.result().toHex();
78+
const QByteArray ba = hash.result().toHex();
7979
return QString::fromLatin1(ba);
8080
}
8181

82-
QString Start::getUniquePNG(const std::string& path)
82+
QString Start::getUniquePNG(const QString& path)
8383
{
84-
QDir dir = getThumbnailsDir();
85-
QString md5 = getMD5Hash(path) + QLatin1String(".png");
84+
const QDir dir = getThumbnailsDir();
85+
const QString md5 = getMD5Hash(path) + QLatin1String(".png");
8686
return dir.absoluteFilePath(md5);
8787
}
8888

89-
bool Start::useCachedPNG(const std::string& image, const std::string& project)
89+
bool Start::useCachedPNG(const QString& image, const QString& project)
9090
{
91-
Base::FileInfo f1(image);
92-
Base::FileInfo f2(project);
91+
const QFileInfo f1(image);
92+
const QFileInfo f2(project);
9393
if (!f1.exists()) {
9494
return false;
9595
}

‎src/Mod/Start/App/FileUtilities.h‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,15 +24,13 @@
2424
#ifndef FREECAD_FILEUTILITIES_H
2525
#define FREECAD_FILEUTILITIES_H
2626

27-
#include <string>
28-
#include <Base/FileInfo.h>
29-
#include <QString>
30-
#include <QDir>
27+
class QString;
28+
class QDir;
3129

3230
namespace Start
3331
{
3432

35-
std::string getThumbnailsImage();
33+
QString getThumbnailsImage();
3634

3735
QString getThumbnailsName();
3836

@@ -42,11 +40,11 @@ QString getThumbnailsDir();
4240

4341
void createThumbnailsDir();
4442

45-
QString getMD5Hash(const std::string& path);
43+
QString getMD5Hash(const QString& path);
4644

47-
QString getUniquePNG(const std::string& path);
45+
QString getUniquePNG(const QString& path);
4846

49-
bool useCachedPNG(const std::string& image, const std::string& project);
47+
bool useCachedPNG(const QString& image, const QString& project);
5048

5149
} // namespace Start
5250

‎src/Mod/Start/App/PreCompiled.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@
5252
#include <QDir>
5353
#include <QFile>
5454
#include <QFileInfo>
55+
#include <QMutexLocker>
5556
#include <QStandardPaths>
57+
#include <QString>
58+
#include <QThreadPool>
59+
#include <QTimeZone>
5660
#include <QUrl>
5761

5862
#endif // _PreComp_

0 commit comments

Comments
 (0)