----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119896/#review65189 -----------------------------------------------------------
Ship it! You don't really need a ship it from me :) Feel free to work in master. Baloo is in one of these nice stages right now that we do not have to maintain ABI or API compatibilty, so this doesn't really need to be in a branch. src/naturalqueryparser/naturalfilequeryparser.h <https://git.reviewboard.kde.org/r/119896/#comment45552> Could you please move this to the bottom with the other private stuff. It's just coding convention. src/naturalqueryparser/pass_propertyinfo.cpp <https://git.reviewboard.kde.org/r/119896/#comment45554> It's a little risky doing at(0) and at(1) before checking the size. Typically the size is supposed to be > 1 at this point, but still I've run into issues. Maybe a simple assert if nothing else? src/queryparser/autotests/queryparsertest.h <https://git.reviewboard.kde.org/r/119896/#comment45566> We might need to split these tests out at some point as well and test the "Natural" and "normal" part differently. src/queryparser/autotests/queryparsertest.cpp <https://git.reviewboard.kde.org/r/119896/#comment45564> Interesting test. It's interesting cause we don't actually support any of the >, <, >= or <= operators on integers. We only do it for ratings and even then it's .. hackish. You know why this is - we're just a big lookup table. Maybe you want to add a different kind of test? Also, should we be allowing these kind of queries if we do not support it? src/queryparser/queryparser_export.h <https://git.reviewboard.kde.org/r/119896/#comment45563> This file can now be auto generated by cmake magic. Have a look at how other parts of baloo do it. - Vishesh Handa On Aug. 22, 2014, 9:14 a.m., Denis Steckelmacher wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119896/ > ----------------------------------------------------------- > > (Updated Aug. 22, 2014, 9:14 a.m.) > > > Review request for Baloo. > > > Repository: baloo > > > Description > ------- > > This patch is quite big because it cleans up several things that were needed > in order to add support for KFileMetaData file properties: > > * QueryParser is fully ported to KF5 > * QueryParser is renamed to NaturalQueryParser to avoid a clash with the > Xapian query parser > * NaturalQueryParser is split into NaturalQueryParser (recognition of > numbers, sizes, date-times and tags) and NaturalFileQueryParser (file size, > created/modified at, and file properties). This will ease the development of > query parsers targetted specifically to e-mails, contacts, etc, when need > will be. > * Use KFileMetaData to discover the names of valid file properties, so that > the pattern "$1 =|is|: $2" matches only when $1 is a valid property name. > This also allows auto-completion of the property names (added in a patch to > baloo-widgets) > > This patch is the diff between kde:baloo, branch frameworks, and kde:baloo, > branch naturalqueryparser. You can checkout naturalqueryparser or browse this > branch on QuickGit if you want a more detailed view of this patch. > > > Diffs > ----- > > src/CMakeLists.txt f441852 > src/naturalqueryparser/CMakeLists.txt PRE-CREATION > src/naturalqueryparser/autotests/CMakeLists.txt PRE-CREATION > src/naturalqueryparser/naturalfilequeryparser.h PRE-CREATION > src/naturalqueryparser/naturalfilequeryparser.cpp PRE-CREATION > src/naturalqueryparser/naturalqueryparser_p.h PRE-CREATION > src/naturalqueryparser/pass_propertyinfo.h PRE-CREATION > src/naturalqueryparser/pass_propertyinfo.cpp PRE-CREATION > src/queryparser/CMakeLists.txt 4abd4ff > src/queryparser/Messages.sh > src/queryparser/autotests/CMakeLists.txt 68bd9f1 > src/queryparser/autotests/queryparsertest.h 5e823fd > src/queryparser/autotests/queryparsertest.cpp a7f1684 > src/queryparser/completionproposal.h a172acd > src/queryparser/completionproposal.cpp > src/queryparser/pass_comparators.h > src/queryparser/pass_comparators.cpp > src/queryparser/pass_dateperiods.h > src/queryparser/pass_dateperiods.cpp > src/queryparser/pass_datevalues.h > src/queryparser/pass_datevalues.cpp > src/queryparser/pass_decimalvalues.h > src/queryparser/pass_decimalvalues.cpp > src/queryparser/pass_filenames.h > src/queryparser/pass_filenames.cpp > src/queryparser/pass_filesize.h > src/queryparser/pass_filesize.cpp > src/queryparser/pass_numbers.h > src/queryparser/pass_numbers.cpp 0ab1b2b > src/queryparser/pass_periodnames.h > src/queryparser/pass_periodnames.cpp > src/queryparser/pass_properties.h > src/queryparser/pass_properties.cpp > src/queryparser/pass_splitunits.h > src/queryparser/pass_splitunits.cpp > src/queryparser/pass_subqueries.h > src/queryparser/pass_subqueries.cpp > src/queryparser/pass_typehints.h > src/queryparser/pass_typehints.cpp > src/queryparser/patternmatcher.h c491784 > src/queryparser/patternmatcher.cpp 681afa2 > src/queryparser/queryparser.h 8e61434 > src/queryparser/queryparser.cpp 3c4c263 > src/queryparser/queryparser_export.h 9c83b4c > src/queryparser/utils.h > src/queryparser/utils.cpp > > Diff: https://git.reviewboard.kde.org/r/119896/diff/ > > > Testing > ------- > > The testsuite of NaturalQueryParser passes, and a new unit test has been > added for file properties. The Baloo testsuite passes except the "scheduler" > test, I don't know if it is normal or a regression (my system has no kded5 > nor any KF5 runtime component). > > > Thanks, > > Denis Steckelmacher > >
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<
