Differential D27682 Diff 77563 kcmkwin/kwinrules/ruleslist.cpp

Changeset View

Standalone View

View Options

kcmkwin/kwinrules/ruleslist.cpp

Show All 16 Lines
17 */ 17​ */
18 18​
19#include "ruleslist.h" 19​#include "ruleslist.h"
20 20​
21#include <QDebug> 21​#include <QDebug>
22#include <kconfig.h> 22​#include <kconfig.h>
23#include <QFileDialog> 23​#include <QFileDialog>
24 24​
25​#include "../../rules.h"
26​#include "rulesettings.h"
25#include "ruleswidget.h" 27​#include "ruleswidget.h"
26 28​
27namespace KWin 29​namespace KWin
28{ 30​{
29 31​
30KCMRulesList::KCMRulesList(QWidget* parent) 32​KCMRulesList::KCMRulesList(QWidget* parent)
31 : QWidget(parent) 33​ : QWidget(parent)
34​ , 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{ 35​{
33 setupUi(this); 36​ setupUi(this);
34 // connect both current/selected, so that current==selected (stupid QListBox :( ) 37​ // connect both current/selected, so that current==selected (stupid QListBox :( )
35 connect(rules_listbox, SIGNAL(itemChanged(QListWidgetItem*)), 38​ connect(rules_listbox, SIGNAL(itemChanged(QListWidgetItem*)),
36 SLOT(activeChanged())); 39​ SLOT(activeChanged()));
37 connect(rules_listbox, SIGNAL(itemSelectionChanged()), 40​ connect(rules_listbox, SIGNAL(itemSelectionChanged()),
38 SLOT(activeChanged())); 41​ SLOT(activeChanged()));
39 connect(new_button, SIGNAL(clicked()), 42​ connect(new_button, SIGNAL(clicked()),
Show All 11 Lines
51 connect(import_button, SIGNAL(clicked()), 54​ connect(import_button, SIGNAL(clicked()),
52 SLOT(importClicked())); 55​ SLOT(importClicked()));
53 connect(rules_listbox, SIGNAL(itemDoubleClicked(QListWidgetItem*)), 56​ connect(rules_listbox, SIGNAL(itemDoubleClicked(QListWidgetItem*)),
54 SLOT(modifyClicked())); 57​ SLOT(modifyClicked()));
55 load(); 58​ load();
56} 59​}
57 60​
58KCMRulesList::~KCMRulesList() 61​KCMRulesList::~KCMRulesList()
59{ 62​{
60 for (QVector< Rules* >::Iterator it = rules.begin(); 63​ qDeleteAll(m_rules);
ervinUnsubmitted
Done

or better yet use qDeleteAll(m_rules)

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

Would need a qAsConst around m_rules

ervin: Would need a qAsConst around m_rules
216 ++it) 216​ for (const auto rule : qAsConst(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(); ```
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); 217​ rules_listbox->addItem(rule->description);
230 } 218​ }
231 if (rules.count() > 0) 219​
220​ if (m_rules.count() > 0)
232 rules_listbox->setCurrentItem(rules_listbox->item(0)); 221​ rules_listbox->setCurrentItem(rules_listbox->item(0));
233 else 222​ else
234 rules_listbox->setCurrentItem(nullptr); 223​ rules_listbox->setCurrentItem(nullptr);
235 activeChanged(); 224​ activeChanged();
236} 225​}
237 226​
238void KCMRulesList::save() 227​void KCMRulesList::save()
239{ 228​{
240 KConfig cfg(QLatin1String("kwinrulesrc")); 229​ m_settings.setRules(m_rules);
241 QStringList groups = cfg.groupList(); 230​ m_settings.save();
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(); ```
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} 231​}
256 232​
257void KCMRulesList::defaults() 233​void KCMRulesList::defaults()
258{ 234​{
259 load(); 235​ load();
260} 236​}
261 237​
262} // namespace 238​} // namespace
263 239​