Differential D27682 Diff 76837 kcmkwin/kwinrules/ruleslist.cpp

Changeset View

Standalone View

View Options

kcmkwin/kwinrules/ruleslist.cpp

Show All 23 Lines
24 24​
25#include "ruleswidget.h" 25​#include "ruleswidget.h"
26 26​
27namespace KWin 27​namespace KWin
28{ 28​{
29 29​
30KCMRulesList::KCMRulesList(QWidget* parent) 30​KCMRulesList::KCMRulesList(QWidget* parent)
31 : QWidget(parent) 31​ : QWidget(parent)
32​ , m_settings(this)
ervinUnsubmitted
Done

I don't think this is doing what you expect, you didn't declare m_settings as a pointer. I wonder if that couldn't even lead to a double delete... might not be the case a bit by chance (since children are cleaned-up in QObject dtor the KCMRulesList part of the object (including the m_settings member are long gone). Cheer luck ;-)

ervin: I don't think this is doing what you expect, you didn't declare m_settings as a pointer. I…
ervinUnsubmitted
Done

I still think this should be switched to using a pointer, or if not the this should be dropped as parameter.

ervin: I still think this should be switched to using a pointer, or if not the this should be dropped…
32{ 33​{
33 setupUi(this); 34​ setupUi(this);
34 // connect both current/selected, so that current==selected (stupid QListBox :( ) 35​ // connect both current/selected, so that current==selected (stupid QListBox :( )
35 connect(rules_listbox, SIGNAL(itemChanged(QListWidgetItem*)), 36​ connect(rules_listbox, SIGNAL(itemChanged(QListWidgetItem*)),
36 SLOT(activeChanged())); 37​ SLOT(activeChanged()));
37 connect(rules_listbox, SIGNAL(itemSelectionChanged()), 38​ connect(rules_listbox, SIGNAL(itemSelectionChanged()),
38 SLOT(activeChanged())); 39​ SLOT(activeChanged()));
39 connect(new_button, SIGNAL(clicked()), 40​ connect(new_button, SIGNAL(clicked()),
Show All 11 Lines
51 connect(import_button, SIGNAL(clicked()), 52​ connect(import_button, SIGNAL(clicked()),
52 SLOT(importClicked())); 53​ SLOT(importClicked()));
53 connect(rules_listbox, SIGNAL(itemDoubleClicked(QListWidgetItem*)), 54​ connect(rules_listbox, SIGNAL(itemDoubleClicked(QListWidgetItem*)),
54 SLOT(modifyClicked())); 55​ SLOT(modifyClicked()));
55 load(); 56​ load();
56} 57​}
57 58​
58KCMRulesList::~KCMRulesList() 59​KCMRulesList::~KCMRulesList()
59{ 60​{
60 for (QVector< Rules* >::Iterator it = rules.begin(); 61​ qDeleteAll(m_rules);
ervinUnsubmitted
Done

or better yet use qDeleteAll(m_rules)

ervin: or better yet use `qDeleteAll(m_rules)`
61 it != rules.end(); 62​ m_rules.clear();
62 ++it) ​
63 delete *it; ​
64 rules.clear(); ​
65} 63​}
66 64​
67void KCMRulesList::activeChanged() 65​void KCMRulesList::activeChanged()
68{ 66​{
69 QListWidgetItem *item = rules_listbox->currentItem(); 67​ QListWidgetItem *item = rules_listbox->currentItem();
70 int itemRow = rules_listbox->row(item); 68​ int itemRow = rules_listbox->row(item);
71 69​
72 if (item != nullptr) // make current==selected 70​ if (item != nullptr) // make current==selected
Show All 9 Lines
82{ 80​{
83 RulesDialog dlg(this); 81​ RulesDialog dlg(this);
84 Rules* rule = dlg.edit(nullptr, {}, false); 82​ Rules* rule = dlg.edit(nullptr, {}, false);
85 if (rule == nullptr) 83​ if (rule == nullptr)
86 return; 84​ return;
87 int pos = rules_listbox->currentRow() + 1; 85​ int pos = rules_listbox->currentRow() + 1;
88 rules_listbox->insertItem(pos , rule->description); 86​ rules_listbox->insertItem(pos , rule->description);
89 rules_listbox->setCurrentRow(pos, QItemSelectionModel::ClearAndSelect); 87​ rules_listbox->setCurrentRow(pos, QItemSelectionModel::ClearAndSelect);
90 rules.insert(rules.begin() + pos, rule); 88​ m_rules.insert(m_rules.begin() + pos, rule);
91 emit changed(true); 89​ emit changed(true);
92} 90​}
93 91​
94void KCMRulesList::modifyClicked() 92​void KCMRulesList::modifyClicked()
95{ 93​{
96 int pos = rules_listbox->currentRow(); 94​ int pos = rules_listbox->currentRow();
97 if (pos == -1) 95​ if (pos == -1)
98 return; 96​ return;
99 RulesDialog dlg(this); 97​ RulesDialog dlg(this);
100 Rules* rule = dlg.edit(rules[ pos ], {}, false); 98​ Rules *rule = dlg.edit(m_rules[pos], {}, false);
101 if (rule == rules[ pos ]) 99​ if (rule == m_rules[pos])
102 return; 100​ return;
103 delete rules[ pos ]; 101​ delete m_rules[pos];
104 rules[ pos ] = rule; 102​ m_rules[pos] = rule;
105 rules_listbox->item(pos)->setText(rule->description); 103​ rules_listbox->item(pos)->setText(rule->description);
106 emit changed(true); 104​ emit changed(true);
107} 105​}
108 106​
109void KCMRulesList::deleteClicked() 107​void KCMRulesList::deleteClicked()
110{ 108​{
111 int pos = rules_listbox->currentRow(); 109​ int pos = rules_listbox->currentRow();
112 Q_ASSERT(pos != -1); 110​ Q_ASSERT(pos != -1);
113 delete rules_listbox->takeItem(pos); 111​ delete rules_listbox->takeItem(pos);
114 rules.erase(rules.begin() + pos); 112​ m_rules.erase(m_rules.begin() + pos);
115 emit changed(true); 113​ emit changed(true);
116} 114​}
117 115​
118void KCMRulesList::moveupClicked() 116​void KCMRulesList::moveupClicked()
119{ 117​{
120 int pos = rules_listbox->currentRow(); 118​ int pos = rules_listbox->currentRow();
121 Q_ASSERT(pos != -1); 119​ Q_ASSERT(pos != -1);
122 if (pos > 0) { 120​ if (pos > 0) {
123 QListWidgetItem * item = rules_listbox->takeItem(pos); 121​ QListWidgetItem * item = rules_listbox->takeItem(pos);
124 rules_listbox->insertItem(pos - 1 , item); 122​ rules_listbox->insertItem(pos - 1 , item);
125 rules_listbox->setCurrentItem(item, QItemSelectionModel::ClearAndSelect); 123​ rules_listbox->setCurrentItem(item, QItemSelectionModel::ClearAndSelect);
126 Rules* rule = rules[ pos ]; 124​ Rules *rule = m_rules[pos];
127 rules[ pos ] = rules[ pos - 1 ]; 125​ m_rules[pos] = m_rules[pos - 1];
128 rules[ pos - 1 ] = rule; 126​ m_rules[pos - 1] = rule;
129 } 127​ }
130 emit changed(true); 128​ emit changed(true);
131} 129​}
132 130​
133void KCMRulesList::movedownClicked() 131​void KCMRulesList::movedownClicked()
134{ 132​{
135 int pos = rules_listbox->currentRow(); 133​ int pos = rules_listbox->currentRow();
136 Q_ASSERT(pos != -1); 134​ Q_ASSERT(pos != -1);
137 if (pos < int(rules_listbox->count()) - 1) { 135​ if (pos < int(rules_listbox->count()) - 1) {
138 QListWidgetItem * item = rules_listbox->takeItem(pos); 136​ QListWidgetItem * item = rules_listbox->takeItem(pos);
139 rules_listbox->insertItem(pos + 1 , item); 137​ rules_listbox->insertItem(pos + 1 , item);
140 rules_listbox->setCurrentItem(item, QItemSelectionModel::ClearAndSelect); 138​ rules_listbox->setCurrentItem(item, QItemSelectionModel::ClearAndSelect);
141 Rules* rule = rules[ pos ]; 139​ Rules *rule = m_rules[pos];
142 rules[ pos ] = rules[ pos + 1 ]; 140​ m_rules[pos] = m_rules[pos + 1];
143 rules[ pos + 1 ] = rule; 141​ m_rules[pos + 1] = rule;
144 } 142​ }
145 emit changed(true); 143​ emit changed(true);
146} 144​}
147 145​
148void KCMRulesList::exportClicked() 146​void KCMRulesList::exportClicked()
149{ 147​{
150 int pos = rules_listbox->currentRow(); 148​ int pos = rules_listbox->currentRow();
151 Q_ASSERT(pos != -1); 149​ Q_ASSERT(pos != -1);
152 QString path = QFileDialog::getSaveFileName(this, i18n("Export Rules"), QDir::home().absolutePath(), 150​ QString path = QFileDialog::getSaveFileName(this, i18n("Export Rules"), QDir::home().absolutePath(),
153 i18n("KWin Rules (*.kwinrule)")); 151​ i18n("KWin Rules (*.kwinrule)"));
154 if (path.isEmpty()) 152​ if (path.isEmpty())
155 return; 153​ return;
156 KConfig config(path, KConfig::SimpleConfig); 154​ const auto cfg = KSharedConfig::openConfig(path, KConfig::SimpleConfig);
157 KConfigGroup group(&config, rules[pos]->description); 155​ RuleSettings settings(cfg, m_rules[pos]->description);
158 group.deleteGroup(); 156​ settings.setDefaults();
159 rules[pos]->write(group); 157​ m_rules[pos]->write(settings);
160} 158​}
161 159​
162void KCMRulesList::importClicked() 160​void KCMRulesList::importClicked()
163{ 161​{
164 QString path = QFileDialog::getOpenFileName(this, i18n("Import Rules"), QDir::home().absolutePath(), 162​ QString path = QFileDialog::getOpenFileName(this, i18n("Import Rules"), QDir::home().absolutePath(),
165 i18n("KWin Rules (*.kwinrule)")); 163​ i18n("KWin Rules (*.kwinrule)"));
166 if (path.isEmpty()) 164​ if (path.isEmpty())
167 return; 165​ return;
168 KConfig config(path, KConfig::SimpleConfig); 166​ const auto config = KSharedConfig::openConfig(path, KConfig::SimpleConfig);
169 QStringList groups = config.groupList(); 167​ QStringList groups = config->groupList();
170 if (groups.isEmpty()) 168​ if (groups.isEmpty())
171 return; 169​ return;
172 170​
173 int pos = qMax(0, rules_listbox->currentRow()); 171​ int pos = qMax(0, rules_listbox->currentRow());
174 foreach (const QString &group, groups) { 172​ for (const QString &group : groups) {
175 KConfigGroup grp(&config, group); 173​ RuleSettings settings(config, group);
176 const bool remove = grp.readEntry("DeleteRule", false); 174​ const bool remove = settings.deleteRule();
177 Rules* new_rule = new Rules(grp); 175​ Rules *new_rule = new Rules(settings);
178 176​
179 // try to replace existing rule first 177​ // try to replace existing rule first
180 for (int i = 0; i < rules.count(); ++i) { 178​ for (int i = 0; i < m_rules.count(); ++i) {
181 if (rules[i]->description == new_rule->description) { 179​ if (m_rules[i]->description == new_rule->description) {
182 delete rules[i]; 180​ delete m_rules[i];
183 if (remove) { 181​ if (remove) {
184 rules.remove(i); 182​ m_rules.remove(i);
185 delete rules_listbox->takeItem(i); 183​ delete rules_listbox->takeItem(i);
186 delete new_rule; 184​ delete new_rule;
187 pos = qMax(0, rules_listbox->currentRow()); // might have changed! 185​ pos = qMax(0, rules_listbox->currentRow()); // might have changed!
188 } 186​ } else
189 else 187​ m_rules[i] = new_rule;
190 rules[i] = new_rule; ​
191 new_rule = nullptr; 188​ new_rule = nullptr;
192 break; 189​ break;
193 } 190​ }
194 } 191​ }
195 192​
196 // don't add "to be deleted" if not present 193​ // don't add "to be deleted" if not present
197 if (remove) { 194​ if (remove) {
198 delete new_rule; 195​ delete new_rule;
199 new_rule = nullptr; 196​ new_rule = nullptr;
200 } 197​ }
201 198​
202 // plain insertion 199​ // plain insertion
203 if (new_rule) { 200​ if (new_rule) {
204 rules.insert(pos, new_rule); 201​ m_rules.insert(pos, new_rule);
205 rules_listbox->insertItem(pos++, new_rule->description); 202​ rules_listbox->insertItem(pos++, new_rule->description);
206 } 203​ }
207 } 204​ }
208 emit changed(true); 205​ emit changed(true);
209} 206​}
210 207​
211void KCMRulesList::load() 208​void KCMRulesList::load()
212{ 209​{
213 rules_listbox->clear(); 210​ rules_listbox->clear();
214 for (QVector< Rules* >::Iterator it = rules.begin(); 211​ m_settings.loadAllInto(m_rules);
215 it != rules.end(); 212​ for (const auto rule : m_rules) {
ervinUnsubmitted
Not Done

Would need a qAsConst around m_rules

ervin: Would need a qAsConst around m_rules
ervinUnsubmitted
Not Done

With my proposal it'd become:

m_settings->load();
m_rules = m_settings->rules();
ervin: With my proposal it'd become: ``` m_settings->load(); m_rules = m_settings->rules(); ```
216 ++it) ​
217 delete *it; ​
218 rules.clear(); ​
219 KConfig _cfg("kwinrulesrc"); ​
220 KConfigGroup cfg(&_cfg, "General"); ​
221 int count = cfg.readEntry("count", 0); ​
222 rules.reserve(count); ​
223 for (int i = 1; ​
224 i <= count; ​
225 ++i) { ​
226 cfg = KConfigGroup(&_cfg, QString::number(i)); ​
227 Rules* rule = new Rules(cfg); ​
228 rules.append(rule); ​
229 rules_listbox->addItem(rule->description); 213​ rules_listbox->addItem(rule->description);
230 } 214​ }
231 if (rules.count() > 0) 215​
216​ if (m_rules.count() > 0)
232 rules_listbox->setCurrentItem(rules_listbox->item(0)); 217​ rules_listbox->setCurrentItem(rules_listbox->item(0));
233 else 218​ else
234 rules_listbox->setCurrentItem(nullptr); 219​ rules_listbox->setCurrentItem(nullptr);
235 activeChanged(); 220​ activeChanged();
236} 221​}
237 222​
238void KCMRulesList::save() 223​void KCMRulesList::save()
239{ 224​{
240 KConfig cfg(QLatin1String("kwinrulesrc")); 225​ m_settings.saveAllFrom(m_rules);
ervinUnsubmitted
Not Done

With my proposal it'd become:

m_settings->setRules(m_rules);
m_settings->save();
ervin: With my proposal it'd become: ``` m_settings->setRules(m_rules); m_settings->save(); ```
241 QStringList groups = cfg.groupList(); ​
242 for (QStringList::ConstIterator it = groups.constBegin(); ​
243 it != groups.constEnd(); ​
244 ++it) ​
245 cfg.deleteGroup(*it); ​
246 cfg.group("General").writeEntry("count", rules.count()); ​
247 int i = 1; ​
248 for (QVector< Rules* >::ConstIterator it = rules.constBegin(); ​
249 it != rules.constEnd(); ​
250 ++it) { ​
251 KConfigGroup cg(&cfg, QString::number(i)); ​
252 (*it)->write(cg); ​
253 ++i; ​
254 } ​
255} 226​}
256 227​
257void KCMRulesList::defaults() 228​void KCMRulesList::defaults()
258{ 229​{
259 load(); 230​ load();
260} 231​}
261 232​
262} // namespace 233​} // namespace
263 234​