Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refectoring that aims to fix Rekordbox crash on macOS #10608 #11230

Merged
merged 27 commits into from
Feb 12, 2023
Merged
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
009dba1
Make use of unique_ptr and std::move
daschuer Jan 24, 2023
c6ea8ab
Use std::move in BaseSqlTableModel::setTable
daschuer Jan 25, 2023
b6c82af
Inmprove initialization of library columns
daschuer Jan 24, 2023
adcb323
Move BaseTrackCache::setSearchColumns to constructor, to create the o…
daschuer Jan 25, 2023
734d410
Use QStringLiteral for Library columns
daschuer Jan 22, 2023
6cfdc9c
Use std::unique_ptr when creating the library tree
daschuer Jan 24, 2023
3260e9c
Reset m_pLastRightClickedItem before removing treeItems.
daschuer Jan 24, 2023
83973e1
Clear LastRightClickIndex when removing tree items.
daschuer Jan 24, 2023
01d6667
Clean up search columns of the Serato and Rekordbox feature to match …
daschuer Jan 26, 2023
9257b80
remove unused m_searchColumnIndices
daschuer Jan 27, 2023
849215e
Remove unused include
daschuer Jan 27, 2023
e79baef
Browsetablemodel: Simplify adding search columns. Not yet enabled.
daschuer Jan 27, 2023
fecfc14
make future result const before iteration
daschuer Jan 29, 2023
3fcfa16
Remove pointless comments
daschuer Jan 30, 2023
5805b17
Make use of SharedPointer::create to avoid a literally new,
daschuer Jan 30, 2023
155fad8
Fix naming of contsnats
daschuer Jan 31, 2023
c21d919
Prefere QModelIndex(); when resetting stored indexes.
daschuer Jan 31, 2023
e7ffb39
use std::make_unique when initalize m_pQueryParser
daschuer Jan 31, 2023
6f7ed40
Improve the comment about QFuture
daschuer Jan 31, 2023
904ad3a
Use iterator-based construction
daschuer Jan 31, 2023
c77e18a
Remove redundant type name
daschuer Jan 31, 2023
a48a5ed
Remove stary comment
daschuer Jan 31, 2023
54e0902
Describe the issue with QFuture and std::unique_ptr() a bit more
daschuer Jan 31, 2023
4f49ed7
Document decorateChild()
daschuer Jan 31, 2023
384d275
use auto in connection with std::make_unique
daschuer Jan 31, 2023
d71b91e
Add todos for introducing a std::span to simplifx the one element case
daschuer Feb 2, 2023
4834e90
Add comments about the risk of dangling pointers
daschuer Feb 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
make future result const before iteration
daschuer committed Jan 29, 2023
commit fecfc149552b6c6be7f8e2578e19e7e0b6059486
4 changes: 1 addition & 3 deletions src/library/basetrackcache.cpp
Original file line number Diff line number Diff line change
@@ -27,8 +27,7 @@ BaseTrackCache::BaseTrackCache(TrackCollection* pTrackCollection,
m_columnCount(columns.size()),
m_columnsJoined(columns.join(",")),
m_columnCache(std::move(columns)),
m_pQueryParser(new SearchQueryParser(pTrackCollection)),
m_searchColumns(std::move(searchColumns)),
m_pQueryParser(new SearchQueryParser(pTrackCollection, std::move(searchColumns))),
Swiftb0y marked this conversation as resolved.
Show resolved Hide resolved
m_bIndexBuilt(false),
m_bIsCaching(isCaching),
m_database(pTrackCollection->database()) {
@@ -470,7 +469,6 @@ void BaseTrackCache::filterAndSort(const QSet<TrackId>& trackIds,
const std::unique_ptr<QueryNode> pQuery =
m_pQueryParser->parseQuery(
searchQuery,
m_searchColumns,
queryFragments.join(" AND "));

QString filter = pQuery->toSql();
2 changes: 0 additions & 2 deletions src/library/basetrackcache.h
Original file line number Diff line number Diff line change
@@ -126,8 +126,6 @@ class BaseTrackCache : public QObject {

const StringCollator m_collator;

QStringList m_searchColumns;

// Temporary storage for filterAndSort()

QVector<TrackId> m_trackOrder;
3 changes: 2 additions & 1 deletion src/library/rekordbox/rekordboxfeature.cpp
Original file line number Diff line number Diff line change
@@ -1459,7 +1459,8 @@ void RekordboxFeature::activateChild(const QModelIndex& index) {

void RekordboxFeature::onRekordboxDevicesFound() {
std::vector<std::unique_ptr<TreeItem>> foundDevices;
for (const auto& pDeviceFound : m_devicesFuture.result()) {
const QList<TreeItem*> result = m_devicesFuture.result();
for (const auto& pDeviceFound : result) {
foundDevices.emplace_back(pDeviceFound);
}

48 changes: 25 additions & 23 deletions src/library/searchqueryparser.cpp
Original file line number Diff line number Diff line change
@@ -7,8 +7,11 @@
constexpr char kNegatePrefix[] = "-";
constexpr char kFuzzyPrefix[] = "~";

SearchQueryParser::SearchQueryParser(TrackCollection* pTrackCollection)
: m_pTrackCollection(pTrackCollection) {
SearchQueryParser::SearchQueryParser(TrackCollection* pTrackCollection, QStringList searchColumns)
: m_pTrackCollection(pTrackCollection),
m_searchCrates(false) {
setSearchColumns(std::move(searchColumns));

m_textFilters << "artist"
<< "album_artist"
<< "album"
@@ -31,7 +34,6 @@ SearchQueryParser::SearchQueryParser(TrackCollection* pTrackCollection)
<< "dateadded"
<< "datetime_added"
<< "date_added";
m_ignoredColumns << "crate";

m_fieldToSqlColumns["artist"] << "artist" << "album_artist";
m_fieldToSqlColumns["album_artist"] << "album_artist";
@@ -66,6 +68,19 @@ SearchQueryParser::SearchQueryParser(TrackCollection* pTrackCollection)
SearchQueryParser::~SearchQueryParser() {
}

void SearchQueryParser::setSearchColumns(QStringList searchColumns) {
m_queryColumns = std::move(searchColumns);

// we need to create a filtered columns list that are handled differently
for (int i = 0; i < m_queryColumns.size(); ++i) {
if (m_queryColumns[i] == "crate") {
m_searchCrates = true;
m_queryColumns.removeAt(i);
break;
}
}
}

QString SearchQueryParser::getTextArgument(QString argument,
QStringList* tokens) const {
// If the argument is empty, assume the user placed a space after an
@@ -116,20 +131,7 @@ QString SearchQueryParser::getTextArgument(QString argument,
}

void SearchQueryParser::parseTokens(QStringList tokens,
QStringList searchColumns,
AndNode* pQuery) const {
// we need to create a filtered columns list that are handled differently
auto queryColumns = QStringList();
queryColumns.reserve(searchColumns.count());

for (const auto& column: qAsConst(searchColumns)) {
if (m_ignoredColumns.contains(column)) {
continue;
}
queryColumns << column;
}


while (tokens.size() > 0) {
QString token = tokens.takeFirst().trimmed();
if (token.length() == 0) {
@@ -224,18 +226,18 @@ void SearchQueryParser::parseTokens(QStringList tokens,
// For untagged strings we search the track fields as well
// as the crate names the track is in. This allows the user
// to use crates like tags
if (searchColumns.contains("crate")) {
if (m_searchCrates) {
std::unique_ptr<OrNode> gNode = std::make_unique<OrNode>();

gNode->addNode(std::make_unique<CrateFilterNode>(
&m_pTrackCollection->crates(), argument));
gNode->addNode(std::make_unique<TextFilterNode>(
m_pTrackCollection->database(), queryColumns, argument));
m_pTrackCollection->database(), m_queryColumns, argument));

pNode = std::move(gNode);
} else {
pNode = std::make_unique<TextFilterNode>(
m_pTrackCollection->database(), queryColumns, argument);
m_pTrackCollection->database(), m_queryColumns, argument);
}
}
}
@@ -248,9 +250,9 @@ void SearchQueryParser::parseTokens(QStringList tokens,
}
}

std::unique_ptr<QueryNode> SearchQueryParser::parseQuery(const QString& query,
const QStringList& searchColumns,
const QString& extraFilter) const {
std::unique_ptr<QueryNode> SearchQueryParser::parseQuery(
const QString& query,
const QString& extraFilter) const {
auto pQuery(std::make_unique<AndNode>());

if (!extraFilter.isEmpty()) {
@@ -259,7 +261,7 @@ std::unique_ptr<QueryNode> SearchQueryParser::parseQuery(const QString& query,

if (!query.isEmpty()) {
QStringList tokens = query.split(" ");
parseTokens(tokens, searchColumns, pQuery.get());
parseTokens(tokens, pQuery.get());
}

return pQuery;
9 changes: 5 additions & 4 deletions src/library/searchqueryparser.h
Original file line number Diff line number Diff line change
@@ -10,29 +10,30 @@

class SearchQueryParser {
public:
explicit SearchQueryParser(TrackCollection* pTrackCollection);
explicit SearchQueryParser(TrackCollection* pTrackCollection, QStringList searchColumns);

virtual ~SearchQueryParser();

void setSearchColumns(QStringList searchColumns);

std::unique_ptr<QueryNode> parseQuery(
const QString& query,
const QStringList& searchColumns,
const QString& extraFilter) const;


private:
void parseTokens(QStringList tokens,
QStringList searchColumns,
AndNode* pQuery) const;

QString getTextArgument(QString argument,
QStringList* tokens) const;

TrackCollection* m_pTrackCollection;
QStringList m_queryColumns;
bool m_searchCrates;
QStringList m_textFilters;
QStringList m_numericFilters;
QStringList m_specialFilters;
QStringList m_ignoredColumns;
QStringList m_allFilters;
QHash<QString, QStringList> m_fieldToSqlColumns;

3 changes: 2 additions & 1 deletion src/library/serato/seratofeature.cpp
Original file line number Diff line number Diff line change
@@ -1063,7 +1063,8 @@ void SeratoFeature::activateChild(const QModelIndex& index) {

void SeratoFeature::onSeratoDatabasesFound() {
std::vector<std::unique_ptr<TreeItem>> foundDatabases;
for (const auto& pDatabaseFound : m_databasesFuture.result()) {
const QList<TreeItem*> result = m_databasesFuture.result();
for (const auto& pDatabaseFound : result) {
foundDatabases.emplace_back(pDatabaseFound);
}

254 changes: 86 additions & 168 deletions src/test/searchqueryparsertest.cpp

Large diffs are not rendered by default.