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

enable search in Browse & Recording views #11014

Merged
merged 5 commits into from
Nov 1, 2022

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Oct 27, 2022

This enables a simple case-insensitive text filter for the Browse & Recording view via their proxymodels.
Edit not so simple anymore since with the last commit the query is not treated as one string but is split into words, so it allows queries like holy "blue cow".

All columns are considered, incl. invisible.
Query is saved per feature only (not for individual folders).

closes #11012

@ronso0 ronso0 added this to the 2.4.0 milestone Oct 27, 2022
@ronso0 ronso0 added the search label Oct 28, 2022
@ronso0
Copy link
Member Author

ronso0 commented Oct 31, 2022

The last commit allows searching for individual words or quoted word groups.
Feels a bit like reinveting the wheel (sql column query) so this basic implementation is all I will do here.

@ronso0 ronso0 force-pushed the browse-enable-search branch from 735bc2a to 04be78d Compare October 31, 2022 23:11
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this first version. I have not yet tested it only some C++ suggestions.

How does the tokenizing compare to our normal search? I think it fails for things like "gimme gimme gimme" ... probably expected

src/library/proxytrackmodel.cpp Outdated Show resolved Hide resolved
if (strData.contains(filter)) {
rowMatches = true;

for (QString token : tokens) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (QString token : tokens) {
for (const auto& token : std::as_const(tokens)) {

Not sure if std::as_const() is strictly required though...

for (QString token : tokens) {
QListIterator<int> iter(filterColumns);
bool tokenMatch = false;
while (iter.hasNext() && !tokenMatch) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can also become a range based loop with a break statement below for early abbort.

@ronso0
Copy link
Member Author

ronso0 commented Nov 1, 2022

How does the tokenizing compare to our normal search? I think it fails for things like "gimme gimme gimme" ... probably expected

Sorry, I don't understand the question, it's the same tokenizer used by WTrackTableView::onSearch > SearchQueryParser::queryIsLessSpecific.
And I don't know how to compare the basic text filter here and the sql filter in BaseTrackCache::filterAndSort performance-wise if that's what you mean.

@daschuer
Copy link
Member

daschuer commented Nov 1, 2022

Never mind, I have just tested it and it works like a charm. Thank you very much.

@daschuer daschuer merged commit 35aad35 into mixxxdj:main Nov 1, 2022
@daschuer daschuer added the changelog This PR should be included in the changelog label Nov 1, 2022
@ronso0 ronso0 deleted the browse-enable-search branch November 1, 2022 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browse changelog This PR should be included in the changelog library recording search
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use search bar in computer folders
2 participants