Differential D29791 Diff 83054 kcmkwin/kwinrules/rulesmodel.cpp

Changeset View

Standalone View

View Options

kcmkwin/kwinrules/rulesmodel.cpp

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Line(s)98 case IconNameRole:
99 return rule->iconName(); 99​ return rule->iconName();
100 case DescriptionRole: 100​ case DescriptionRole:
101 return rule->description(); 101​ return rule->description();
102 case SectionRole: 102​ case SectionRole:
103 return rule->section(); 103​ return rule->section();
104 case EnabledRole: 104​ case EnabledRole:
105 return rule->isEnabled(); 105​ return rule->isEnabled();
106 case SelectableRole: 106​ case SelectableRole:
107 return !rule->hasFlag(RuleItem::AlwaysEnabled); 107​ return !rule->hasFlag(RuleItem::AlwaysEnabled) && !rule->hasFlag(RuleItem::SuggestionOnly);
108 case ValueRole: 108​ case ValueRole:
109 return rule->value(); 109​ return rule->value();
110 case TypeRole: 110​ case TypeRole:
111 return rule->type(); 111​ return rule->type();
112 case PolicyRole: 112​ case PolicyRole:
113 return rule->policy(); 113​ return rule->policy();
114 case PolicyModelRole: 114​ case PolicyModelRole:
115 return rule->policyModel(); 115​ return rule->policyModel();
Show All 16 Lines125{
132 switch (role) { 132​ switch (role) {
133 case EnabledRole: 133​ case EnabledRole:
134 if (value.toBool() == rule->isEnabled()) { 134​ if (value.toBool() == rule->isEnabled()) {
135 return true; 135​ return true;
136 } 136​ }
137 rule->setEnabled(value.toBool()); 137​ rule->setEnabled(value.toBool());
138 break; 138​ break;
139 case ValueRole: 139​ case ValueRole:
140​ if (rule->key() == "wmclasshelper") {
141​ setData(this->index(1), value, RulesModel::ValueRole); // wmclass := value
mevenUnsubmitted
Done

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

meven: Make those index values constants or at least less hard coded.
iasensioAuthorUnsubmitted
Not Done

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.

iasensio: Good advice. Now, it uses a `indexOf()`method to get the index of a rule. Also, to keep the…
142​ setData(this->index(2), true, RulesModel::ValueRole); // wmclasscomplete := true
143​ break;
144​ }
140 if (value == rule->value()) { 145​ if (value == rule->value()) {
141 return true; 146​ return true;
142 } 147​ }
143 rule->setValue(value); 148​ rule->setValue(value);
144 break; 149​ break;
145 case PolicyRole: 150​ case PolicyRole:
146 if (value.toInt() == rule->policy()) { 151​ if (value.toInt() == rule->policy()) {
147 return true; 152​ return true;
Show All 19 Lines171 if (rule->hasFlag(RuleItem::AffectsWarning)) {
167 emit warningMessageChanged(); 172​ emit warningMessageChanged();
168 } 173​ }
169 174​
170 return true; 175​ return true;
171} 176​}
172 177​
173RuleItem *RulesModel::addRule(RuleItem *rule) 178​RuleItem *RulesModel::addRule(RuleItem *rule)
174{ 179​{
175 m_ruleList << rule; 180​ m_ruleList << rule;
mevenUnsubmitted
Not Done

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

meven: Could you `for (const RuleItem *rule : qAsConst(m_ruleList)) {` c++ style
broulikUnsubmitted
Not Done

Or you could use std::find_if

broulik: Or you could use `std::find_if`
iasensioAuthorUnsubmitted
Not Done

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.

iasensio: Here we needed the position in the array, not the item itself, to build the `index` within the…
176 m_rules.insert(rule->key(), rule); 181​ m_rules.insert(rule->key(), rule);
177 182​
178 return rule; 183​ return rule;
179} 184​}
180 185​
181bool RulesModel::hasRule(const QString& key) const 186​bool RulesModel::hasRule(const QString& key) const
182{ 187​{
183 return m_rules.contains(key); 188​ return m_rules.contains(key);
Show All 11 Lines198{
195 if (!desc.isEmpty()) { 200​ if (!desc.isEmpty()) {
196 return desc; 201​ return desc;
197 } 202​ }
198 return defaultDescription(); 203​ return defaultDescription();
199} 204​}
200 205​
201void RulesModel::setDescription(const QString &description) 206​void RulesModel::setDescription(const QString &description)
202{ 207​{
203 setData(index(0, 0), description, RulesModel::ValueRole); 208​ setData(index(0), description, RulesModel::ValueRole);
204} 209​}
205 210​
206QString RulesModel::defaultDescription() const 211​QString RulesModel::defaultDescription() const
207{ 212​{
208 const QString wmclass = m_rules["wmclass"]->value().toString(); 213​ const QString wmclass = m_rules["wmclass"]->value().toString();
209 const QString title = m_rules["title"]->isEnabled() ? m_rules["title"]->value().toString() : QString(); 214​ const QString title = m_rules["title"]->isEnabled() ? m_rules["title"]->value().toString() : QString();
210 215​
211 if (!title.isEmpty()) { 216​ if (!title.isEmpty()) {
212 return i18n("Window settings for %1", title); 217​ return i18n("Window settings for %1", title);
213 } 218​ }
214 if (!wmclass.isEmpty()) { 219​ if (!wmclass.isEmpty()) {
215 return i18n("Settings for %1", wmclass); 220​ return i18n("Settings for %1", wmclass);
216 } 221​ }
217 222​
218 return i18n("New window settings"); 223​ return i18n("New window settings");
219} 224​}
220 225​
221QString RulesModel::warningMessage() const 226​QString RulesModel::warningMessage() const
222{ 227​{
223 if (wmclassWarning()) { 228​ if (wmclassWarning()) {
broulikUnsubmitted
Done

Compare with QLatin1String

broulik: Compare with `QLatin1String`
224 return i18n("You have specified the window class as unimportant.\n" 229​ return i18n("You have specified the window class as unimportant.\n"
225 "This means the settings will possibly apply to windows from all applications." 230​ "This means the settings will possibly apply to windows from all applications."
226 " If you really want to create a generic setting, it is recommended" 231​ " If you really want to create a generic setting, it is recommended"
227 " you at least limit the window types to avoid special window types."); 232​ " you at least limit the window types to avoid special window types.");
228 } 233​ }
229 234​
230 return QString(); 235​ return QString();
231} 236​}
232 237​
233 ​
234bool RulesModel::wmclassWarning() const 238​bool RulesModel::wmclassWarning() const
235{ 239​{
236 const bool no_wmclass = !m_rules["wmclass"]->isEnabled() 240​ const bool no_wmclass = !m_rules["wmclass"]->isEnabled()
237 || m_rules["wmclass"]->policy() == Rules::UnimportantMatch; 241​ || m_rules["wmclass"]->policy() == Rules::UnimportantMatch;
238 const bool alltypes = !m_rules["types"]->isEnabled() 242​ const bool alltypes = !m_rules["types"]->isEnabled()
239 || (m_rules["types"]->value() == 0) 243​ || (m_rules["types"]->value() == 0)
240 || (m_rules["types"]->value() == NET::AllTypesMask) 244​ || (m_rules["types"]->value() == NET::AllTypesMask)
241 || ((m_rules["types"]->value().toInt() | (1 << NET::Override)) == 0x3FF); 245​ || ((m_rules["types"]->value().toInt() | (1 << NET::Override)) == 0x3FF);
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Line(s)285{
283 if (description.isEmpty()) { 287​ if (description.isEmpty()) {
284 m_rules["description"]->setValue(defaultDescription()); 288​ m_rules["description"]->setValue(defaultDescription());
285 } 289​ }
286 290​
287 for (const RuleItem *rule : qAsConst(m_ruleList)) { 291​ for (const RuleItem *rule : qAsConst(m_ruleList)) {
288 KConfigSkeletonItem *configItem = settings->findItem(rule->key()); 292​ KConfigSkeletonItem *configItem = settings->findItem(rule->key());
289 KConfigSkeletonItem *configPolicyItem = settings->findItem(rule->policyKey()); 293​ KConfigSkeletonItem *configPolicyItem = settings->findItem(rule->policyKey());
290 294​
291 Q_ASSERT (configItem); 295​ if (!configItem) {
296​ continue;
297​ }
292 298​
293 if (rule->isEnabled()) { 299​ if (rule->isEnabled()) {
294 configItem->setProperty(rule->value()); 300​ configItem->setProperty(rule->value());
295 if (configPolicyItem) { 301​ if (configPolicyItem) {
296 configPolicyItem->setProperty(rule->policy()); 302​ configPolicyItem->setProperty(rule->policy());
297 } 303​ }
298 } else { 304​ } else {
299 if (configPolicyItem) { 305​ if (configPolicyItem) {
▲ Show 20 Lines • Show All 65 Lines • ▼ Show 20 Line(s)352{
365 wmclass->setFlag(RuleItem::AffectsWarning); 371​ wmclass->setFlag(RuleItem::AffectsWarning);
366 372​
367 auto wmclasscomplete = addRule(new RuleItem(QLatin1String("wmclasscomplete"), 373​ auto wmclasscomplete = addRule(new RuleItem(QLatin1String("wmclasscomplete"),
368 RulePolicy::NoPolicy, RuleItem::Boolean, 374​ RulePolicy::NoPolicy, RuleItem::Boolean,
369 i18n("Match whole window class"), i18n("Window matching"), 375​ i18n("Match whole window class"), i18n("Window matching"),
370 QIcon::fromTheme("window"))); 376​ QIcon::fromTheme("window")));
371 wmclasscomplete->setFlag(RuleItem::AlwaysEnabled); 377​ wmclasscomplete->setFlag(RuleItem::AlwaysEnabled);
372 378​
379​ // Helper item to store the detected whole window class when detecting properties
380​ auto wmclasshelper = addRule(new RuleItem(QLatin1String("wmclasshelper"),
381​ RulePolicy::NoPolicy, RuleItem::String,
382​ i18n("Window whole class"), i18n("Window matching"),
broulikUnsubmitted
Done

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

broulik: "whole class" reads odd imho but I'm not a native speaker
iasensioAuthorUnsubmitted
Not Done

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

iasensio: You are right, it should say "whole window class", like the previous property.
383​ QIcon::fromTheme("window")));
384​ wmclasshelper->setFlag(RuleItem::SuggestionOnly);
385​
373 auto types = addRule(new RuleItem(QLatin1String("types"), 386​ auto types = addRule(new RuleItem(QLatin1String("types"),
374 RulePolicy::NoPolicy, RuleItem::FlagsOption, 387​ RulePolicy::NoPolicy, RuleItem::FlagsOption,
375 i18n("Window types"), i18n("Window matching"), 388​ i18n("Window types"), i18n("Window matching"),
376 QIcon::fromTheme("window-duplicate"))); 389​ QIcon::fromTheme("window-duplicate")));
377 types->setOptionsData(windowTypesModelData()); 390​ types->setOptionsData(windowTypesModelData());
378 types->setFlag(RuleItem::AlwaysEnabled); 391​ types->setFlag(RuleItem::AlwaysEnabled);
379 types->setFlag(RuleItem::AffectsWarning); 392​ types->setFlag(RuleItem::AffectsWarning);
380 393​
▲ Show 20 Lines • Show All 232 Lines • ▼ Show 20 Line(s)624 addRule(new RuleItem(QLatin1String("blockcompositing"),
613 i18n("Block compositing"), i18n("Appearance & Fixes"), 626​ i18n("Block compositing"), i18n("Appearance & Fixes"),
614 QIcon::fromTheme("composite-track-on"))); 627​ QIcon::fromTheme("composite-track-on")));
615} 628​}
616 629​
617 630​
618const QHash<QString, QString> RulesModel::x11PropertyHash() 631​const QHash<QString, QString> RulesModel::x11PropertyHash()
619{ 632​{
620 static const auto propertyToRule = QHash<QString, QString> { 633​ static const auto propertyToRule = QHash<QString, QString> {
621 /* The original detection dialog allows to choose depending on "Match complete window class": ​
622 * if Match Complete == false: wmclass = "resourceClass" ​
623 * if Match Complete == true: wmclass = "resourceName" + " " + "resourceClass" ​
624 */ ​
625 { "resourceName", "wmclass" }, 634​ { "resourceName", "wmclass" },
626 { "caption", "title" }, 635​ { "caption", "title" },
627 { "role", "windowrole" }, 636​ { "role", "windowrole" },
628 { "clientMachine", "clientmachine" }, 637​ { "clientMachine", "clientmachine" },
629 { "x11DesktopNumber", "desktop" }, 638​ { "x11DesktopNumber", "desktop" },
630 { "maximizeHorizontal", "maximizehoriz" }, 639​ { "maximizeHorizontal", "maximizehoriz" },
631 { "maximizeVertical", "maximizevert" }, 640​ { "maximizeVertical", "maximizevert" },
632 { "minimized", "minimize" }, 641​ { "minimized", "minimize" },
Show All 23 Lines657{
656 m_rules["maxsize"]->setSuggestedValue(size, forceValue); 665​ m_rules["maxsize"]->setSuggestedValue(size, forceValue);
657 666​
658 NET::WindowType window_type = static_cast<NET::WindowType>(info.value("type", 0).toInt()); 667​ NET::WindowType window_type = static_cast<NET::WindowType>(info.value("type", 0).toInt());
659 if (window_type == NET::Unknown) { 668​ if (window_type == NET::Unknown) {
660 window_type = NET::Normal; 669​ window_type = NET::Normal;
661 } 670​ }
662 m_rules["types"]->setSuggestedValue(1 << window_type, forceValue); 671​ m_rules["types"]->setSuggestedValue(1 << window_type, forceValue);
663 672​
673​ // Store "complete window class" as "resourceName" + " " + "resourceClass"
674​ // Do not force the value, we want it only as a suggested value for the user to select
675​ const QString wmcompleteclass = QStringLiteral("%1 %2").arg(info.value("resourceName").toString())
676​ .arg(info.value("resourceClass").toString());
677​ m_rules["wmclasshelper"]->setSuggestedValue(wmcompleteclass);
678​
664 const auto ruleForProperty = x11PropertyHash(); 679​ const auto ruleForProperty = x11PropertyHash();
665 for (QString &property : info.keys()) { 680​ for (QString &property : info.keys()) {
666 if (!ruleForProperty.contains(property)) { 681​ if (!ruleForProperty.contains(property)) {
667 continue; 682​ continue;
668 } 683​ }
669 const QString ruleKey = ruleForProperty.value(property, QString()); 684​ const QString ruleKey = ruleForProperty.value(property, QString());
670 Q_ASSERT(hasRule(ruleKey)); 685​ Q_ASSERT(hasRule(ruleKey));
671 686​
▲ Show 20 Lines • Show All 148 Lines • Show Last 20 Lines