Differential D29791 Diff 83170 kcmkwin/kwinrules/rulesmodel.cpp

Changeset View

Standalone View

View Options

kcmkwin/kwinrules/rulesmodel.cpp

Show First 20 Lines • Show All 101 Lines • ▼ Show 20 Line(s)101 case IconNameRole:
102 return rule->iconName(); 102​ return rule->iconName();
103 case DescriptionRole: 103​ case DescriptionRole:
104 return rule->description(); 104​ return rule->description();
105 case SectionRole: 105​ case SectionRole:
106 return rule->section(); 106​ return rule->section();
107 case EnabledRole: 107​ case EnabledRole:
108 return rule->isEnabled(); 108​ return rule->isEnabled();
109 case SelectableRole: 109​ case SelectableRole:
110 return !rule->hasFlag(RuleItem::AlwaysEnabled); 110​ return !rule->hasFlag(RuleItem::AlwaysEnabled) && !rule->hasFlag(RuleItem::SuggestionOnly);
111 case ValueRole: 111​ case ValueRole:
112 return rule->value(); 112​ return rule->value();
113 case TypeRole: 113​ case TypeRole:
114 return rule->type(); 114​ return rule->type();
115 case PolicyRole: 115​ case PolicyRole:
116 return rule->policy(); 116​ return rule->policy();
117 case PolicyModelRole: 117​ case PolicyModelRole:
118 return rule->policyModel(); 118​ return rule->policyModel();
Show All 16 Lines128{
135 switch (role) { 135​ switch (role) {
136 case EnabledRole: 136​ case EnabledRole:
137 if (value.toBool() == rule->isEnabled()) { 137​ if (value.toBool() == rule->isEnabled()) {
138 return true; 138​ return true;
139 } 139​ }
140 rule->setEnabled(value.toBool()); 140​ rule->setEnabled(value.toBool());
141 break; 141​ break;
142 case ValueRole: 142​ case ValueRole:
143​ if (rule->hasFlag(RuleItem::SuggestionOnly)) {
144​ processSuggestion(rule->key(), 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…
145​ }
143 if (value == rule->value()) { 146​ if (value == rule->value()) {
144 return true; 147​ return true;
145 } 148​ }
146 rule->setValue(value); 149​ rule->setValue(value);
147 break; 150​ break;
148 case PolicyRole: 151​ case PolicyRole:
149 if (value.toInt() == rule->policy()) { 152​ if (value.toInt() == rule->policy()) {
150 return true; 153​ return true;
Show All 17 Lines
168 } 171​ }
169 if (rule->hasFlag(RuleItem::AffectsWarning)) { 172​ if (rule->hasFlag(RuleItem::AffectsWarning)) {
170 emit warningMessageChanged(); 173​ emit warningMessageChanged();
171 } 174​ }
172 175​
173 return true; 176​ return true;
174} 177​}
175 178​
179​QModelIndex RulesModel::indexOf(const QString& key) const
180​{
181​ const QModelIndexList indexes = match(index(0), RulesModel::KeyRole, key, 1, Qt::MatchFixedString);
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…
182​ if (indexes.isEmpty()) {
183​ return QModelIndex();
184​ }
185​ return indexes.at(0);
186​}
187​
176RuleItem *RulesModel::addRule(RuleItem *rule) 188​RuleItem *RulesModel::addRule(RuleItem *rule)
177{ 189​{
178 m_ruleList << rule; 190​ m_ruleList << rule;
179 m_rules.insert(rule->key(), rule); 191​ m_rules.insert(rule->key(), rule);
180 192​
181 return rule; 193​ return rule;
182} 194​}
183 195​
Show All 14 Lines208{
198 if (!desc.isEmpty()) { 210​ if (!desc.isEmpty()) {
199 return desc; 211​ return desc;
200 } 212​ }
201 return defaultDescription(); 213​ return defaultDescription();
202} 214​}
203 215​
204void RulesModel::setDescription(const QString &description) 216​void RulesModel::setDescription(const QString &description)
205{ 217​{
206 setData(index(0, 0), description, RulesModel::ValueRole); 218​ setData(indexOf("description"), description, RulesModel::ValueRole);
207} 219​}
208 220​
209QString RulesModel::defaultDescription() const 221​QString RulesModel::defaultDescription() const
210{ 222​{
211 const QString wmclass = m_rules["wmclass"]->value().toString(); 223​ const QString wmclass = m_rules["wmclass"]->value().toString();
212 const QString title = m_rules["title"]->isEnabled() ? m_rules["title"]->value().toString() : QString(); 224​ const QString title = m_rules["title"]->isEnabled() ? m_rules["title"]->value().toString() : QString();
213 225​
214 if (!title.isEmpty()) { 226​ if (!title.isEmpty()) {
215 return i18n("Window settings for %1", title); 227​ return i18n("Window settings for %1", title);
216 } 228​ }
217 if (!wmclass.isEmpty()) { 229​ if (!wmclass.isEmpty()) {
218 return i18n("Settings for %1", wmclass); 230​ return i18n("Settings for %1", wmclass);
219 } 231​ }
220 232​
221 return i18n("New window settings"); 233​ return i18n("New window settings");
222} 234​}
223 235​
236​void RulesModel::processSuggestion(const QString &key, const QVariant &value)
237​{
238​ if (key == QLatin1String("wmclasshelper")) {
broulikUnsubmitted
Done

Compare with QLatin1String

broulik: Compare with `QLatin1String`
239​ setData(indexOf("wmclass"), value, RulesModel::ValueRole);
240​ setData(indexOf("wmclasscomplete"), true, RulesModel::ValueRole);
241​ }
242​}
243​
224QString RulesModel::warningMessage() const 244​QString RulesModel::warningMessage() const
225{ 245​{
226 if (wmclassWarning()) { 246​ if (wmclassWarning()) {
227 return i18n("You have specified the window class as unimportant.\n" 247​ return i18n("You have specified the window class as unimportant.\n"
228 "This means the settings will possibly apply to windows from all applications." 248​ "This means the settings will possibly apply to windows from all applications."
229 " If you really want to create a generic setting, it is recommended" 249​ " If you really want to create a generic setting, it is recommended"
230 " you at least limit the window types to avoid special window types."); 250​ " you at least limit the window types to avoid special window types.");
231 } 251​ }
232 252​
233 return QString(); 253​ return QString();
234} 254​}
235 255​
236 ​
237bool RulesModel::wmclassWarning() const 256​bool RulesModel::wmclassWarning() const
238{ 257​{
239 const bool no_wmclass = !m_rules["wmclass"]->isEnabled() 258​ const bool no_wmclass = !m_rules["wmclass"]->isEnabled()
240 || m_rules["wmclass"]->policy() == Rules::UnimportantMatch; 259​ || m_rules["wmclass"]->policy() == Rules::UnimportantMatch;
241 const bool alltypes = !m_rules["types"]->isEnabled() 260​ const bool alltypes = !m_rules["types"]->isEnabled()
242 || (m_rules["types"]->value() == 0) 261​ || (m_rules["types"]->value() == 0)
243 || (m_rules["types"]->value() == NET::AllTypesMask) 262​ || (m_rules["types"]->value() == NET::AllTypesMask)
244 || ((m_rules["types"]->value().toInt() | (1 << NET::Override)) == 0x3FF); 263​ || ((m_rules["types"]->value().toInt() | (1 << NET::Override)) == 0x3FF);
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Line(s)303{
286 if (description.isEmpty()) { 305​ if (description.isEmpty()) {
287 m_rules["description"]->setValue(defaultDescription()); 306​ m_rules["description"]->setValue(defaultDescription());
288 } 307​ }
289 308​
290 for (const RuleItem *rule : qAsConst(m_ruleList)) { 309​ for (const RuleItem *rule : qAsConst(m_ruleList)) {
291 KConfigSkeletonItem *configItem = settings->findItem(rule->key()); 310​ KConfigSkeletonItem *configItem = settings->findItem(rule->key());
292 KConfigSkeletonItem *configPolicyItem = settings->findItem(rule->policyKey()); 311​ KConfigSkeletonItem *configPolicyItem = settings->findItem(rule->policyKey());
293 312​
294 Q_ASSERT (configItem); 313​ if (!configItem) {
314​ continue;
315​ }
295 316​
296 if (rule->isEnabled()) { 317​ if (rule->isEnabled()) {
297 configItem->setProperty(rule->value()); 318​ configItem->setProperty(rule->value());
298 if (configPolicyItem) { 319​ if (configPolicyItem) {
299 configPolicyItem->setProperty(rule->policy()); 320​ configPolicyItem->setProperty(rule->policy());
300 } 321​ }
301 } else { 322​ } else {
302 if (configPolicyItem) { 323​ if (configPolicyItem) {
▲ Show 20 Lines • Show All 65 Lines • ▼ Show 20 Line(s)370{
368 wmclass->setFlag(RuleItem::AffectsWarning); 389​ wmclass->setFlag(RuleItem::AffectsWarning);
369 390​
370 auto wmclasscomplete = addRule(new RuleItem(QLatin1String("wmclasscomplete"), 391​ auto wmclasscomplete = addRule(new RuleItem(QLatin1String("wmclasscomplete"),
371 RulePolicy::NoPolicy, RuleItem::Boolean, 392​ RulePolicy::NoPolicy, RuleItem::Boolean,
372 i18n("Match whole window class"), i18n("Window matching"), 393​ i18n("Match whole window class"), i18n("Window matching"),
373 QIcon::fromTheme("window"))); 394​ QIcon::fromTheme("window")));
374 wmclasscomplete->setFlag(RuleItem::AlwaysEnabled); 395​ wmclasscomplete->setFlag(RuleItem::AlwaysEnabled);
375 396​
397​ // Helper item to store the detected whole window class when detecting properties
398​ auto wmclasshelper = addRule(new RuleItem(QLatin1String("wmclasshelper"),
399​ RulePolicy::NoPolicy, RuleItem::String,
400​ i18n("Whole window 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.
401​ QIcon::fromTheme("window")));
402​ wmclasshelper->setFlag(RuleItem::SuggestionOnly);
403​
376 auto types = addRule(new RuleItem(QLatin1String("types"), 404​ auto types = addRule(new RuleItem(QLatin1String("types"),
377 RulePolicy::NoPolicy, RuleItem::FlagsOption, 405​ RulePolicy::NoPolicy, RuleItem::FlagsOption,
378 i18n("Window types"), i18n("Window matching"), 406​ i18n("Window types"), i18n("Window matching"),
379 QIcon::fromTheme("window-duplicate"))); 407​ QIcon::fromTheme("window-duplicate")));
380 types->setOptionsData(windowTypesModelData()); 408​ types->setOptionsData(windowTypesModelData());
381 types->setFlag(RuleItem::AlwaysEnabled); 409​ types->setFlag(RuleItem::AlwaysEnabled);
382 types->setFlag(RuleItem::AffectsWarning); 410​ types->setFlag(RuleItem::AffectsWarning);
383 411​
▲ Show 20 Lines • Show All 236 Lines • ▼ Show 20 Line(s)646 addRule(new RuleItem(QLatin1String("blockcompositing"),
620 i18n("Block compositing"), i18n("Appearance & Fixes"), 648​ i18n("Block compositing"), i18n("Appearance & Fixes"),
621 QIcon::fromTheme("composite-track-on"))); 649​ QIcon::fromTheme("composite-track-on")));
622} 650​}
623 651​
624 652​
625const QHash<QString, QString> RulesModel::x11PropertyHash() 653​const QHash<QString, QString> RulesModel::x11PropertyHash()
626{ 654​{
627 static const auto propertyToRule = QHash<QString, QString> { 655​ static const auto propertyToRule = QHash<QString, QString> {
628 /* The original detection dialog allows to choose depending on "Match complete window class": ​
629 * if Match Complete == false: wmclass = "resourceClass" ​
630 * if Match Complete == true: wmclass = "resourceName" + " " + "resourceClass" ​
631 */ ​
632 { "resourceName", "wmclass" }, 656​ { "resourceName", "wmclass" },
633 { "caption", "title" }, 657​ { "caption", "title" },
634 { "role", "windowrole" }, 658​ { "role", "windowrole" },
635 { "clientMachine", "clientmachine" }, 659​ { "clientMachine", "clientmachine" },
636 { "x11DesktopNumber", "desktop" }, 660​ { "x11DesktopNumber", "desktop" },
637 { "maximizeHorizontal", "maximizehoriz" }, 661​ { "maximizeHorizontal", "maximizehoriz" },
638 { "maximizeVertical", "maximizevert" }, 662​ { "maximizeVertical", "maximizevert" },
639 { "minimized", "minimize" }, 663​ { "minimized", "minimize" },
Show All 23 Lines679{
663 m_rules["maxsize"]->setSuggestedValue(size, forceValue); 687​ m_rules["maxsize"]->setSuggestedValue(size, forceValue);
664 688​
665 NET::WindowType window_type = static_cast<NET::WindowType>(info.value("type", 0).toInt()); 689​ NET::WindowType window_type = static_cast<NET::WindowType>(info.value("type", 0).toInt());
666 if (window_type == NET::Unknown) { 690​ if (window_type == NET::Unknown) {
667 window_type = NET::Normal; 691​ window_type = NET::Normal;
668 } 692​ }
669 m_rules["types"]->setSuggestedValue(1 << window_type, forceValue); 693​ m_rules["types"]->setSuggestedValue(1 << window_type, forceValue);
670 694​
695​ // Store "complete window class" as "resourceName" + " " + "resourceClass"
696​ // Do not force the value, we want it only as a suggested value for the user to select
697​ const QString wmcompleteclass = QStringLiteral("%1 %2").arg(info.value("resourceName").toString())
698​ .arg(info.value("resourceClass").toString());
699​ m_rules["wmclasshelper"]->setSuggestedValue(wmcompleteclass);
700​
671 const auto ruleForProperty = x11PropertyHash(); 701​ const auto ruleForProperty = x11PropertyHash();
672 for (QString &property : info.keys()) { 702​ for (QString &property : info.keys()) {
673 if (!ruleForProperty.contains(property)) { 703​ if (!ruleForProperty.contains(property)) {
674 continue; 704​ continue;
675 } 705​ }
676 const QString ruleKey = ruleForProperty.value(property, QString()); 706​ const QString ruleKey = ruleForProperty.value(property, QString());
677 Q_ASSERT(hasRule(ruleKey)); 707​ Q_ASSERT(hasRule(ruleKey));
678 708​
▲ Show 20 Lines • Show All 175 Lines • Show Last 20 Lines