WIP: [Libtaskmanager] Add support for regular expression mapping
ClosedPublic

Authored by broulik on May 24 2016, 3:03 PM.

Details

Summary

This allows more complicated rules, useful for matching Chrome webapps to their
respective desktop file where we need to change crx_foo to chrome-foo-default.

Test Plan
  • Chrome app shows proper icon instead of generic Chrome icon
  • Chrome app can be added as launcher and started independently of Chrome

They are still grouped as Chrome windows and I just noticed that sometimes the grouper gets confused and uses the webapp icon as group icon (well, it just uses the icon of the first item in the group or so)

There can be multiple rewrite rules, it looks for [Rewrite Rules][window class class] and then iterates all groups within, name doesn't matter, I used [1] for clarity. It iterates until it finds a service, the iterations aren't cumulative.

The keys:

  • Property - which property of the window to work with, either ClassName or ClassClass (the default)
  • Identifier - which field to query for in KService (default DesktopEntryName)
  • Match - the Regular Expression, the match named "match" will be used, or the first capture as fallback
  • Target - the rewritten rule, %1 is substituted for the match of the regexp

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik updated this revision to Diff 3967.May 24 2016, 3:03 PM
broulik retitled this revision from to [Libtaskmanager] Add launcher mapping for Google Chrome apps.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik added reviewers: Plasma, hein.
broulik set the repository for this revision to R120 Plasma Workspace.
Restricted Application added a project: Plasma. · View Herald TranscriptMay 24 2016, 3:03 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
This comment was removed by broulik.
hein edited edge metadata.May 24 2016, 6:14 PM

I won't accept this patch, sorry. I'm not willing to add special cases for individual applications, especially to compiled code.

Would you instead accept an extension of the taskmanagerrulesrc mechanism in the style of:

[Rewrite Rules][google-chrome]
crx_$1=chrome-$1-default

?

hein requested changes to this revision.May 25 2016, 9:31 AM
hein edited edge metadata.

Basically yes, but couldn't we use QRegExp syntax instead of coming up with a DSL? Then we could also precompile the regexes when the rc file changes (if we want to make the impl that complex) in case we hit a hotpath.

This revision now requires changes to proceed.May 25 2016, 9:31 AM
broulik updated this revision to Diff 3989.May 25 2016, 11:04 AM
broulik retitled this revision from [Libtaskmanager] Add launcher mapping for Google Chrome apps to WIP: [Libtaskmanager] Add support for regular expression mapping.
broulik updated this object.
broulik edited the test plan for this revision. (Show Details)
broulik edited edge metadata.
hein added a comment.May 25 2016, 11:13 AM

I'd prefer a stricter approach that rejects incomplete rules, e.g. instead of falling back to classClass if Property is not set, skip the rule. Reasoning: The behavior of the heuristic is already magic and obtuse enough, and debugging what happens is not made any easier if interpreting rules requires context knowledge or looking at code. These rules are written so rarely the format doesn't need to be lazy-typing-friendly.

broulik updated this revision to Diff 3991.May 25 2016, 11:36 AM
broulik edited edge metadata.

Parse config strict:

  • Property is required (either ClassClass or ClassName)
  • Identifier is required (eg. DesktopEntryName)
  • only use named regexp match "match"
hein accepted this revision.May 27 2016, 12:09 AM
hein edited edge metadata.

I'll merge this into the new lib to be shipped with 5.7.

This revision is now accepted and ready to land.May 27 2016, 12:09 AM
hein requested changes to this revision.Jun 6 2016, 6:48 PM
hein edited edge metadata.

Kai, could you rebase this against the new lib? The heuristic is in XWindowsTasksModel now. It's the one slice of code still similar to the old lib so it shouldn't be much work.

This revision now requires changes to proceed.Jun 6 2016, 6:48 PM

Will do, once I figure out what I forgot to update: plasmashell: symbol lookup error: /usr/lib/x86_64-linux-gnu/qt5/qml/org/kde/plasma/private/taskmanager/libtaskmanagerplugin.so: undefined symbol: _ZN11TaskManager12GroupManagerC1EP7QObject :)

hein added a comment.Jun 6 2016, 7:01 PM

Looks like you forgot to update plasma-desktop. TaskManager::GroupManager is a class from the old libtaskmanager, which was renamed to legacylibtaskmanager in the plasma-workspace changes. So it looks like you're trying to run the old applet against an updated p-w.

broulik updated this revision to Diff 4254.Jun 6 2016, 7:48 PM
broulik edited edge metadata.
  • Rebase ontop of new libtaskmanager - was a straight forward port :)

Cool side-effect: Chome webapp windows are no longer grouped to other Chrome browser windows, making them behave like real applications.

hein accepted this revision.Jun 6 2016, 7:50 PM
hein edited edge metadata.
This revision is now accepted and ready to land.Jun 6 2016, 7:50 PM
This revision was automatically updated to reflect the committed changes.