Web lists-archives.com

Re: Review Request 128664: Nested tags for Baloo




This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128664/

This is quite an interesting change.

Could you please also elaborate on how this will change the tags kioslave? I'm afraid I don't quite follow.


src/kioslaves/tags/kio_tags.cpp (Diff revision 1)
103
            if (resultTag.startsWith(tag, Qt::CaseInsensitive) && resultTag.compare(tag, Qt::CaseInsensitive)) {

QString::compare returns 0 on match or < 0 and > 0 depending on which term is larger. I don't quite understand what is going on over here.


src/kioslaves/tags/kio_tags.cpp (Diff revision 1)
384
TagsProtocol::ParseResult TagsProtocol::parseUrl(const QUrl& url, QString& tag, QString& fileUrl, bool)
395
TagsProtocol::ParseResult TagsProtocol::parseUrl(const QUrl& url, QString& tag, QString& fileUrl, bool)

This code is getting hard to follow. Do you think you could write a simple unit test for just this function?

That way it will be clear what inputs it can accept.

Also I think this function can be made const.


src/lib/searchstore.cpp (Diff revision 1)
PostingIterator* SearchStore::constructQuery(Transaction* tr, const Term& term)
58
    m_prefixes.insert(QByteArray("tag"), QByteArray("TA"));
58
    m_prefixes.insert(QByteArray("tag"), QByteArray("TA"));

I think this line along with the next can now be removed.


src/lib/searchstore.cpp (Diff revision 1)
270
    } else if (property == "tag") {

Please check for both "tag" and "tags".


src/lib/searchstore.cpp (Diff revision 1)
276
            q = EngineQuery(prefix + value.toString().toUtf8(), EngineQuery::Phrase);

Could you please explain what's going on over here? I don't understand it.

From a cursory glance it seems that this code will assert. q is of type EngineQuery::Phrase and is a leaf query so Transaction::PostingIterator will assert it Q_ASSERT(0)


- Vishesh Handa


On August 18th, 2016, 11:56 p.m. UTC, James Smith wrote:

Review request for Baloo and Vishesh Handa.
By James Smith.

Updated Aug. 18, 2016, 11:56 p.m.

Bugs: 334615
Repository: baloo

Description

Index and query each tag as a single full term for generating recursed search results. Represent nested tags as recursed items in the Tags:// KIO Slave.

Testing

Compile, run

Diffs

  • src/file/basicindexingjob.cpp (88bb59a01e5592abb74b1ab345bfc6765d35db57)
  • src/kioslaves/tags/kio_tags.cpp (de2e6d71945632e23a85f831878b4c431360731c)
  • src/lib/searchstore.cpp (060a4fd795ab858eb84526f93f827d09ee85db7c)

View Diff