Add subseq operator to match sub-sequences
ClosedPublic

Authored by michaeleden on Jun 22 2018, 5:18 AM.

Details

Summary

Added subseq and ~subseq operators which match subsequences of strings.
This is useful to filter out matches quicker:

$ ktraderclient5 --constraint "'libremath' ~subseq Name" --servicetype 'Application' --short
servicetype is : Application
constraint is : 'libremath' ~subseq Name
got 1 offers.
/nix/store/zq1kgdxk426qgf06nzpd7xqkpk4z5hh2-system-path/share/applications/math.desktop
Test Plan

I'll need help knowing where tests for this will go.

Diff Detail

Repository
R309 KService
Branch
feature/match-subseq
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 669
Build 681: arc lint + arc unit
michaeleden created this revision.Jun 22 2018, 5:18 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptJun 22 2018, 5:18 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
michaeleden requested review of this revision.Jun 22 2018, 5:18 AM

I'm confused, what's the difference with subin?

@dfaure if a user was trying to find LibreOffice 6.0 Writer

with subin they would have type libre to narrow it down to LibreOffice programs, then libreoffice 6.0 writer to get to that application.
with subseq they can type libre to get to LibreOffice programs but then just have librewrite match with LibreOffice 6.0 Writer

This is related to Bug 262837

dfaure requested changes to this revision.Jul 7 2018, 10:37 PM
dfaure added inline comments.
src/services/ktraderparsetree.cpp
443

Nothing is being modified, so this should use constBegin(), constEnd(), and const_iterator.

444

This code (the actual algorithm) should be extracted into a function, and a unittest should be written for it (to make sure all corner cases are correctly handled).

Good opportunity to document the kind of matching being done, too, as you had to explain to me in this review request.

And then once this commit is in, it should be documented on https://techbase.kde.org/Development/Tutorials/Services/Traders, please remember to do so.

This revision now requires changes to proceed.Jul 7 2018, 10:37 PM
michaeleden updated this revision to Diff 37301.Jul 8 2018, 4:53 AM

Respond to @dfaure comments by adding tests and separating subseq func

dfaure accepted this revision.Jul 8 2018, 8:42 AM

Perfect, thanks!

This revision is now accepted and ready to land.Jul 8 2018, 8:42 AM
michaeleden added a comment.EditedJul 8 2018, 3:32 PM

@dfaure I don't believe I have permission to edit the wiki, can I make a revision against it somehow? Nevermind, just had to make an account.

dfaure added a comment.Jul 8 2018, 9:40 PM

Ah, so you don't have a developer account, which means you can't push this commit either, and I should do it for you?

Well, you could also apply for a developer account ;)
https://community.kde.org/Infrastructure/Get_a_Developer_Account#Apply_for_a_KDE_Developer_account (I hope it's still accurate).

I'm tagging 5.48 today though, is there a rush to get this in, or can it wait for 5.49?

michaeleden added a comment.EditedJul 8 2018, 10:21 PM

@dfaure you can land it its no problem. I'm also not in a rush to get it into 5.48 or anything (Bug 262837 is from 2011).
I have a few diffs in review, I think I'll apply for a dev account since I'd like to keep doing this stuff.

Actually D13988 is needed to get this functionality into krunner anyway, and that won't go in today. So it's no rush.

dfaure closed this revision.Jul 9 2018, 7:52 AM