[kcm/kwinrules] Detect also window complete class
ClosedPublic

Authored by iasensio on Sat, May 16, 1:03 AM.

Details

Summary

When detecting the properties of a window, now it shows and lets the user select the window complete class.
If this property is selected in the overlay, Window class takes the whole class value, and the option Match window whole class is set.

This adds back a feature the old kcm was offering.

BUG: 421542
FIXED-IN: 5.20

Test Plan
  • Detect window properties and pick a firefox window
  • The property selector shows: Window class: navigator and Whole window class: navigator firefox
  • Selecting the latter set the properties as per summary

Diff Detail

Repository
R108 KWin
Branch
wmclasscomplete
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27083
Build 27101: arc lint + arc unit
iasensio created this revision.Sat, May 16, 1:03 AM
Restricted Application added a project: KWin. · View Herald TranscriptSat, May 16, 1:03 AM
Restricted Application added a subscriber: kwin. · View Herald Transcript
iasensio requested review of this revision.Sat, May 16, 1:03 AM
iasensio updated this revision to Diff 82984.Sat, May 16, 1:13 AM
  • Change assertion for simple check
iasensio edited the summary of this revision. (Show Details)Sat, May 16, 1:14 AM
iasensio edited the test plan for this revision. (Show Details)
zzag added a subscriber: zzag.Mon, May 18, 8:53 AM

As such, and introducing a new i18n string, I don't know whether it should be included in 5.19 or 5.20 release.

This is more like 5.20 stuff.

anthonyfieroni added inline comments.
kcmkwin/kwinrules/ruleitem.cpp
56

You don't need to change these.

iasensio updated this revision to Diff 83054.Mon, May 18, 9:55 PM
iasensio marked an inline comment as done.
  • Keep previous line
iasensio edited the summary of this revision. (Show Details)Mon, May 18, 9:58 PM
iasensio edited the test plan for this revision. (Show Details)

ping on this?

ngraham accepted this revision.Tue, May 26, 8:44 PM

LGTM

This revision is now accepted and ready to land.Tue, May 26, 8:44 PM
meven added a subscriber: meven.Wed, May 27, 5:33 AM
meven added inline comments.
kcmkwin/kwinrules/rulesmodel.cpp
144

Make those index values constants or at least less hard coded.

iasensio updated this revision to Diff 83165.Wed, May 27, 7:03 PM
iasensio marked an inline comment as done.
  • Add indexOf() method
  • Delegate suggestions to a different method
iasensio added inline comments.Wed, May 27, 7:04 PM
kcmkwin/kwinrules/rulesmodel.cpp
144

Good advice. Now, it uses a indexOf()method to get the index of a rule.
Also, to keep the philosophy of keeping the setData() method agnostic to specifics of certain rules, I've delegated the internals to another method.

meven accepted this revision.Thu, May 28, 6:07 AM
meven added a subscriber: davidedmundson.

Seems good to me @zzag @davidedmundson ?

meven added inline comments.Thu, May 28, 6:26 AM
kcmkwin/kwinrules/rulesmodel.cpp
181

Could you for (const RuleItem *rule : qAsConst(m_ruleList)) { c++ style

broulik added inline comments.
kcmkwin/kwinrules/rulesmodel.cpp
181

Or you could use std::find_if

238

Compare with QLatin1String

400

"whole class" reads odd imho but I'm not a native speaker

iasensio updated this revision to Diff 83169.Thu, May 28, 9:39 PM
iasensio marked 2 inline comments as done.
  • Address comments
  • Use match() to get the index
iasensio added inline comments.Thu, May 28, 9:39 PM
kcmkwin/kwinrules/rulesmodel.cpp
181

Here we needed the position in the array, not the item itself, to build the index within the model. But in fact, there's the proper match method for that.

400

You are right, it should say "whole window class", like the previous property.

iasensio updated this revision to Diff 83170.Thu, May 28, 9:42 PM
  • Make it const
iasensio closed this revision.Sun, May 31, 10:51 AM