Skip to content

Fix parsing and tests #58

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.

Sign up for GitHub

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

Merged
Be-ing merged 2 commits into from
Sep 18, 2021
Merged

Fix parsing and tests #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67ce15e
Fix parsing of numeric operators in search queries
uklotzde Sep 18, 2021
a9c43b1
Fix parsing of duration seconds in search queries
uklotzde Sep 18, 2021
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
12 changes: 10 additions & 2 deletions src/library/searchquery.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,16 @@
#include "util/db/sqllikewildcards.h"

namespace {
const QRegularExpression kDurationRegex(QStringLiteral("^(\\d*)(m|:)?([0-6]?\\d)?s?$"));
const QRegularExpression kNumericOperatorRegex(QStringLiteral("^(>|>=|=|<|<=)(.*)$"));
const QRegularExpression kDurationRegex(QStringLiteral("^(\\d+)(m|:)?([0-5]?\\d)?s?$"));
Copy link
Copy Markdown
Owner

@Be-ing Be-ing Sep 18, 2021

Choose a reason for hiding this comment

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

Was the change from 0-6 to 0-5 intentional?

Copy link
Copy Markdown
Author

@uklotzde uklotzde Sep 18, 2021

Choose a reason for hiding this comment

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

Sure, :00-:59 is the valid range. I also added a test that checks that :60 is not accepted.

Copy link
Copy Markdown
Author

@uklotzde uklotzde Sep 18, 2021

Choose a reason for hiding this comment

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

The regex was wrong and no one ever noticed.


// The ordering of operator alternatives separated by '|' is crucial to avoid incomplete
// partial matches, e.g. by capturing "<" + "=" + <arg> instead of "<=" + <arg>!
//
// See also: https://perldoc.perl.org/perlre
// > Alternatives are tried from left to right, so the first alternative found for which
// > the entire expression matches, is the one that is chosen. This means that alternatives
// > are not necessarily greedy.
const QRegularExpression kNumericOperatorRegex(QStringLiteral("^(<=|>=|=|<|>)(.*)$"));
} // namespace

QVariant getTrackValueForColumn(const TrackPointer& pTrack, const QString& column) {
Expand Down
6 changes: 6 additions & 0 deletions src/test/searchqueryparsertest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,12 @@ TEST_F(SearchQueryParserTest, HumanReadableDurationSearchWithOperators) {
EXPECT_STREQ(
qPrintable(QString("duration >= 63")),
qPrintable(pQuery->toSql()));

// Seconds out of range
pQuery = m_parser.parseQuery("duration:>=1:60", searchColumns, "");
pTrack->setDuration(60);
EXPECT_FALSE(pQuery->match(pTrack));
EXPECT_TRUE(pQuery->toSql().isEmpty());
}

TEST_F(SearchQueryParserTest, HumanReadableDurationSearchwithRangeFilter) {
Expand Down