> On Sept. 15, 2014, 10:12 a.m., Vishesh Handa wrote: > > src/core/advancedqueryparser.cpp, line 57 > > <https://git.reviewboard.kde.org/r/120163/diff/1/?file=311772#file311772line57> > > > > Do you also want to take single quotes into consideration? ' and "
I wouldn't. I first implemented strings delimited with " and ', but there is too big of a risk that the user types "let's try this parser" or something else with an unbalanced single quote. The double quote is explicit and users know that they have to be closed. Escape sequences (using \ ) are not a complete solution because most users will not think of escaping single quotes. > On Sept. 15, 2014, 10:12 a.m., Vishesh Handa wrote: > > src/core/advancedqueryparser.cpp, line 125 > > <https://git.reviewboard.kde.org/r/120163/diff/1/?file=311772#file311772line125> > > > > I'm not sure if we should be doing `const auto&` or just `auto`. Could > > you check? You're right, I had to use const auto& (it always bothers me that auto is not smart enought to restrict the type as much as possible) > On Sept. 15, 2014, 10:12 a.m., Vishesh Handa wrote: > > src/core/advancedqueryparser.cpp, line 138 > > <https://git.reviewboard.kde.org/r/120163/diff/1/?file=311772#file311772line138> > > > > 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? I've changed the code so that AND and OR are accepted, but not their lower-case counterpart. It's what Google and Xapian do, I think, and it avoids bad surprises like showing too many results for "Linux or Windows". > On Sept. 15, 2014, 10:12 a.m., Vishesh Handa wrote: > > src/core/advancedqueryparser.cpp, line 151 > > <https://git.reviewboard.kde.org/r/120163/diff/1/?file=311772#file311772line151> > > > > 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. It's not very clean, but "Auto" is already used by my code to store the fact that no comparator has been encountered. If ":" has to be "Auto", then I have to use a custom enum with the values of Term::Comparator plus "NoComparator". > On Sept. 15, 2014, 10:12 a.m., Vishesh Handa wrote: > > src/core/autotests/advancedqueryparsertest.cpp, line 122 > > <https://git.reviewboard.kde.org/r/120163/diff/1/?file=311774#file311774line122> > > > > Yaye! You also like Coldplay. They are an awesome band. Hehe. The main reason why I used these words is that I like consistent unit tests, so I simply reused whatever you used to test the parser in the original unit tests. - Denis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120163/#review66536 ----------------------------------------------------------- On Sept. 16, 2014, 5:13 p.m., Denis Steckelmacher wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120163/ > ----------------------------------------------------------- > > (Updated Sept. 16, 2014, 5:13 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 <<
