----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120163/#review66536 -----------------------------------------------------------
Ship it! Great work as always. Think about the stuff below. src/core/advancedqueryparser.cpp <https://git.reviewboard.kde.org/r/120163/#comment46370> I'm wondering if we should just be comparing the QChar with other QChars, but this arguably will be faster since you're comparing one byte each time. I'm not sure. At some point we may want to support <= and >=. But I don't care that much about it right now. These operators are currently only implemented for dates and ratings. src/core/advancedqueryparser.cpp <https://git.reviewboard.kde.org/r/120163/#comment46371> Do you also want to take single quotes into consideration? ' and " src/core/advancedqueryparser.cpp <https://git.reviewboard.kde.org/r/120163/#comment46372> I'm not sure if we should be doing `const auto&` or just `auto`. Could you check? src/core/advancedqueryparser.cpp <https://git.reviewboard.kde.org/r/120163/#comment46374> I'm not entirely sure if we want to be handling both lower and upper case `and+or` or just upper case. word AND word What do you think? src/core/advancedqueryparser.cpp <https://git.reviewboard.kde.org/r/120163/#comment46375> I'm wondering if the default should be "Auto" but it really doesn't make a difference. The backend should handle "contains" for non strings as contains. src/core/autotests/advancedqueryparsertest.cpp <https://git.reviewboard.kde.org/r/120163/#comment46376> Yaye! You also like Coldplay. They are an awesome band. - Vishesh Handa On Sept. 12, 2014, 1:47 p.m., Denis Steckelmacher wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120163/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2014, 1:47 p.m.) > > > Review request for Baloo. > > > Repository: baloo > > > Description > ------- > > This patch rewrites AdvancedQueryParser so that it does not use regular > expressions anymore. This allows some additional features: > > * Some one-character-long comparators are now supported: =, <, > and :. > Adding support for two-chars comparators like <= and >= would slightly > complicate how the lexer works but should be possible. This patch is already > quite complicated and I did not want to make it too big. > * Values without a property are recognized : "key:value value value" = > AND(key:value, "":value, "":value) > * The logical operators AND and OR (case-insensitive) are recognized : "a:b > AND c:d OR e:f" = OR(AND(a:b, c:d), e:f). Logical operators are > left-associative. > * Braces are allowed when left-associativity is not wanted: "a:b AND (c:d OR > e:f)" = AND(a:b, OR(c:d, e:f)) > * Multiple ANDs and ORs are simplified, as shown in the first example (there > is only one AND instead of "AND(AND(a, b), c)" ) > * Because braces, ANDs and ORs can be used, simply fusing all the literals > that have no property into a big query text does not work anymore. For > instance, "baloo AND (kde OR kf5)" is not equivalent to "baloo kde kf5". The > parser therefore keeps the literals separate, as shown in the first example > (there are two terms having no property). > > > Diffs > ----- > > src/core/advancedqueryparser.cpp 3b222ff > src/core/autotests/advancedqueryparsertest.h 26859a2 > src/core/autotests/advancedqueryparsertest.cpp 14d9cc9 > > Diff: https://git.reviewboard.kde.org/r/120163/diff/ > > > Testing > ------- > > The existing tests continue to pass and I've added new ones for AND, OR, > their combination and nested queries. > > > Thanks, > > Denis Steckelmacher > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
