Use subseq matching for service runner
ClosedPublic

Authored by michaeleden on Jul 8 2018, 6:27 PM.

Details

Summary

This changes krunner to use sub-sequence matching fixing bug 262837.
Some usage examples are shown below:

BUG: 262837

Depends on D13670

Diff Detail

Repository
R120 Plasma Workspace
Branch
feat/app-name-subseq
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 682
Build 694: arc lint + arc unit
michaeleden created this revision.Jul 8 2018, 6:27 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 8 2018, 6:27 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
michaeleden requested review of this revision.Jul 8 2018, 6:27 PM
michaeleden edited the summary of this revision. (Show Details)Jul 8 2018, 6:29 PM
michaeleden edited the summary of this revision. (Show Details)
ngraham added a subscriber: ngraham.

This fixes B262837 (how do you show this in phab?) and relies on D13670

Like so: :)

BUG: 262837

Depends on D13670

See https://community.kde.org/Infrastructure/Phabricator#Add_special_keywords

After you change the Summary section using the web interface, you'll need to run arc amend to pull down that change into your local branch.

Thanks a lot for the patch, BTW! This will be a nice improvement.

plasma-workspace does compile without D13670 and KRunner seems to work, but it gives me a whole lotta console spew:

kf5.kservice.services: Parsing "exist Exec and ( (exist Keywords and 'scover' ~subin Keywords) or (exist GenericName and 'scover' ~subseq GenericName) or (exist Name and 'scover' ~subseq Name) or ('scover' ~subseq Exec) or (exist Comment and 'scover' ~subseq Comment) )" gave: syntax error

We should probably bump the KF5 version requirement in CMakeLists.txt here to require 5.48.

@ngraham this didn't make it into 5.48 but it did make it into the commit after that. Should I put 5.49 even though its not tagged yet?

Nah, let's wait. We'll have to see what the dependency plans are for Plasma; I'm not sure whether or not we'll be depending on Frameworks 5.49 for Plasma 5.14. Might wanna ask one of the Plasma folks.

cfeck added a subscriber: cfeck.Jul 31 2018, 12:53 AM

According to https://community.kde.org/Schedules/Plasma_5 the Plasma 5.14 release is planned to depend on Frameworks 5.50.

ngraham accepted this revision.Jul 31 2018, 2:51 AM

Fantastic. This works great for me, but let's get some more opinions too.

This revision is now accepted and ready to land.Jul 31 2018, 2:51 AM

@ngraham cool! So I should change the deps for KF 5.50 unless someone else disagrees?

Please only change dependency to a released version otherwise this will break the build for everyone. Feel free to bump to 5.49 once it's released (next week?)

Apparently you have to tell sysadmin two weeks in advance. [1]

[1] https://mail.kde.org/pipermail/plasma-devel/2018-August/087983.html

meven added a subscriber: meven.EditedDec 2 2020, 8:00 PM

There no blockers anymore, providing there no conflict, we could land this as-is after a rebase.

Still applies cleanly and seems to fix the bug. Landing.

ngraham closed this revision.Dec 3 2020, 3:28 PM

Manually merged because arc land doesn't seem to want to work anymore. You can see it at //invent.kde.org/plasma/plasma-workspace/commit/9f2abd0a54d51d9234a5a9489d1342b261429fa3.

Thanks Michael!