Differential D28152 Diff 78313 kcmkwin/kwinrules/kcmrules.cpp

Changeset View

Standalone View

View Options

kcmkwin/kwinrules/kcmrules.cpp

  • This file was added.
1​/*
2​ * Copyright (c) 2004 Lubos Lunak <l.lunak@kde.org>
3​ * Copyright (c) 2020 Ismael Asensio <isma.af@gmail.com>
4​ *
5​ * This program is free software; you can redistribute it and/or
6​ * modify it under the terms of the GNU General Public License as
7​ * published by the Free Software Foundation; either version 2 of
8​ * the License or (at your option) version 3 or any later version
9​ * accepted by the membership of KDE e.V. (or its successor approved
10​ * by the membership of KDE e.V.), which shall act as a proxy
11​ * defined in Section 14 of version 3 of the license.
12​ *
13​ * This program is distributed in the hope that it will be useful,
14​ * but WITHOUT ANY WARRANTY; without even the implied warranty of
15​ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16​ * GNU General Public License for more details.
17​ *
18​ * You should have received a copy of the GNU General Public License
19​ * along with this program. If not, see <http://www.gnu.org/licenses/>.
20​ */
21​
22​#include "kcmrules.h"
23​
24​#include <QDBusConnection>
25​#include <QDBusMessage>
broulikUnsubmitted
Done

Don't include the entire module, just include whatever QDBus class you're using, but it appears unused in this file anyway?

broulik: Don't include the entire module, just include whatever `QDBus` class you're using, but it…
iasensioAuthorUnsubmitted
Not Done

It's used in the save() method, to make kwin reload the configuration

iasensio: It's used in the `save()` method, to make kwin reload the configuration
26​
27​#include <KAboutData>
28​#include <KConfig>
29​#include <KLocalizedString>
30​#include <KPluginFactory>
31​
32​
33​namespace
34​{
35​ const QString s_configFile { QLatin1String("kwinrulesrc") };
36​}
37​
38​namespace KWin
39​{
40​
41​KCMKWinRules::KCMKWinRules(QObject *parent, const QVariantList &arguments)
42​ : KQuickAddons::ConfigModule(parent, arguments)
43​ , m_ruleBook(new RuleBookSettings(this))
44​ , m_rulesModel(new RulesModel(this))
45​{
46​ auto about = new KAboutData(QStringLiteral("kcm_kwinrules_qml"),
47​ i18n("Window Rules"),
48​ QStringLiteral("1.0"),
49​ QString(),
50​ KAboutLicense::GPL);
51​ about->addAuthor(i18n("Ismael Asensio"),
52​ i18n("Author"),
53​ QStringLiteral("isma.af@gmail.com"));
54​ setAboutData(about);
55​
56​ setQuickHelp(i18n("<p><h1>Window-specific Settings</h1> Here you can customize window settings specifically only"
57​ " for some windows.</p>"
58​ " <p>Please note that this configuration will not take effect if you do not use"
59​ " KWin as your window manager. If you do use a different window manager, please refer to its documentation"
60​ " for how to customize window behavior.</p>"));
61​
62​ connect(m_rulesModel, &RulesModel::descriptionChanged, this, [this]{
63​ if (m_editingIndex >=0 && m_editingIndex < m_ruleBook->count()) {
64​ m_rules.at(m_editingIndex)->description = m_rulesModel->description();
65​ emit ruleBookModelChanged();
66​ }
67​ } );
68​ connect(m_rulesModel, &RulesModel::dataChanged, this, &KCMKWinRules::updateNeedsSave);
69​}
70​
71​KCMKWinRules::~KCMKWinRules() {
72​ qDeleteAll(m_rules);
73​}
broulikUnsubmitted
Done

You don't need to delete this manually. You pass this as parent in the constructor and Qt has this parent-child releationship where a parent brutally murders its children on desturction.

broulik: You don't need to `delete` this manually. You pass `this` as `parent` in the constructor and Qt…
74​
75​
76​QStringList KCMKWinRules::ruleBookModel() const
77​{
78​ QStringList ruleDescriptionList;
79​ for (const Rules *rule : qAsConst(m_rules)) {
80​ ruleDescriptionList.append(rule->description);
81​ }
82​ return ruleDescriptionList;
83​}
84​
85​
86​void KCMKWinRules::load()
87​{
88​ m_ruleBook->load();
89​ m_rules = m_ruleBook->rules();
90​
91​ setNeedsSave(false);
broulikUnsubmitted
Done

Why not keep it a QVector? We generally try to move away from QList

broulik: Why not keep it a `QVector`? We generally try to move away from `QList`
92​ emit ruleBookModelChanged();
93​
94​ // Check if current index is no longer valid
95​ if (m_editingIndex >= m_rules.count()) {
96​ m_editingIndex = -1;
97​ pop();
98​ emit editingIndexChanged();
99​ }
100​ // Reset current index for rule editor
101​ if (m_editingIndex > 0) {
102​ m_rulesModel->importFromRules(m_rules.at(m_editingIndex));
103​ }
104​}
105​
106​void KCMKWinRules::save()
107​{
108​ saveCurrentRule();
109​ m_ruleBook->setRules(m_rules);
110​ m_ruleBook->save();
111​
112​ // Notify kwin to reload configuration
113​ QDBusMessage message = QDBusMessage::createSignal("/KWin", "org.kde.KWin", "reloadConfig");
114​ QDBusConnection::sessionBus().send(message);
115​}
116​
117​void KCMKWinRules::updateState()
118​{
119​ m_ruleBook->setCount(m_rules.count());
120​
121​ emit editingIndexChanged();
122​ emit ruleBookModelChanged();
123​
124​ updateNeedsSave();
125​}
126​
127​void KCMKWinRules::updateNeedsSave()
128​{
129​ setNeedsSave(true);
130​ emit needsSaveChanged();
131​}
132​
133​void KCMKWinRules::saveCurrentRule()
134​{
135​ if (m_editingIndex < 0) {
136​ return;
137​ }
138​ if (needsSave()) {
139​ delete(m_rules[m_editingIndex]);
140​ m_rules[m_editingIndex] = m_rulesModel->exportToRules();
141​ }
broulikUnsubmitted
Not Done

Could this be done in-memory without a temp file?

broulik: Could this be done in-memory without a temp file?
iasensioAuthorUnsubmitted
Not Done

Yes, I want to talk to @hchain and see to expand the RuleBookSettings class to work with a list of RuleSettings objects instead of Rules, and avoid this intermediate step.

iasensio: Yes, I want to talk to @hchain and see to expand the `RuleBookSettings` class to work with a…
142​}
143​
144​
145​int KCMKWinRules::editingIndex() const
146​{
147​ return m_editingIndex;
148​}
149​
150​void KCMKWinRules::editRule(int index)
151​{
152​ if (index < 0 || index >= m_rules.count()) {
153​ return;
154​ }
155​ saveCurrentRule();
156​
157​ m_editingIndex = index;
158​ m_rulesModel->importFromRules(m_rules.at(m_editingIndex));
159​
160​ emit editingIndexChanged();
161​
162​ // Show and move to Rules Editor page
163​ if (depth() < 2) {
164​ push(QStringLiteral("RulesEditor.qml"));
165​ }
166​ setCurrentIndex(1);
167​}
168​
169​void KCMKWinRules::createRule()
170​{
171​
172​ m_rules.append(new Rules());
173​ updateState();
broulikUnsubmitted
Done

Looks like you're leaking the rules? Perhaps do a

qDeleteAll(m_ruleList)

in the destructor

broulik: Looks like you're leaking the rules? Perhaps do a ``` qDeleteAll(m_ruleList) ``` in the…
174​
175​ const int newIndex = m_rules.count() - 1;
176​ editRule(newIndex);
177​
178​ saveCurrentRule();
179​}
180​
181​void KCMKWinRules::removeRule(int index)
182​{
183​ if (index < 0 || index >= m_rules.count()) {
184​ return;
broulikUnsubmitted
Done

I think doing

if (index < 0 || index >= m_ruleList.count()) {
    return;
}

is clear enough

broulik: I think doing ``` if (index < 0 || index >= m_ruleList.count()) { return; } ``` is clear…
185​ }
186​
187​ if (m_editingIndex == index) {
188​ m_editingIndex = -1;
189​ pop();
190​ }
191​
192​ delete(m_rules.at(index));
193​ m_rules.removeAt(index);
194​
195​ updateState();
196​}
197​
198​void KCMKWinRules::moveRule(int sourceIndex, int destIndex)
199​{
200​ const int lastIndex = m_rules.count() - 1;
201​ if (sourceIndex == destIndex
202​ || (sourceIndex < 0 || sourceIndex > lastIndex)
203​ || (destIndex < 0 || destIndex > lastIndex)) {
204​ return;
205​ }
206​
207​ m_rules.move(sourceIndex, destIndex);
208​
209​ if (m_editingIndex == sourceIndex) {
210​ m_editingIndex = destIndex;
211​ emit editingIndexChanged();
212​ } else if (m_editingIndex > sourceIndex && m_editingIndex <= destIndex) {
213​ m_editingIndex -= 1;
214​ emit editingIndexChanged();
215​ } else if (m_editingIndex < sourceIndex && m_editingIndex >= destIndex) {
216​ m_editingIndex += 1;
217​ emit editingIndexChanged();
218​ }
219​
220​ emit ruleBookModelChanged();
221​
222​ updateNeedsSave();
223​}
224​
225​void KCMKWinRules::exportToFile(const QUrl &path, int index)
226​{
227​ Q_ASSERT(index >= 0 && index < m_rules.count());
228​
229​ saveCurrentRule();
230​
231​ const auto config = KSharedConfig::openConfig(path.toLocalFile(), KConfig::SimpleConfig);
232​ RuleSettings settings(config, m_rules.at(index)->description);
233​
234​ settings.setDefaults();
235​ m_rules.at(index)->write(&settings);
236​ settings.save();
237​}
238​
239​void KCMKWinRules::importFromFile(const QUrl &path)
240​{
241​ const auto config = KSharedConfig::openConfig(path.toLocalFile(), KConfig::SimpleConfig);
242​ const QStringList groups = config->groupList();
243​ if (groups.isEmpty()) {
244​ return;
245​ }
246​
247​ for (const QString &groupName : groups) {
broulikUnsubmitted
Done

Don't use this in conjunction with QML! This creates a nested event loop which will cause all sorts of hard to find crashes. You can probably just do the file selection on the QML side, like we do in most other settings modules:

import QtQuick.Dialogs 1.0 as QtDialogs

    Loader {
        id: fileDialogLoader
        active: false
        sourceComponent: QtDialogs.FileDialog {
            title: i18n("Import Rule")
            folder: shortcuts.home
            nameFilters: [ i18n("KWin Rules (*.kwinrule)") ]
            Component.onCompleted: open()
            onAccepted: {
                kcm.importRuleFromFile(fileUrls[0])
                fileDialogLoader.active = false
            }
            onRejected: fileDialogLoader.active = false
        }
    }

This is copied from another KCM and adjusted. It's behind a Loader as creation of this item is pretty heavy, so we want to just do it on demand. And on the import button you just do onClicked: fileDialogLoader.active = true. Perhaps you could share it for open and save, or just use two separate ones. I'm sure you'll figure it out :)

broulik: Don't use this in conjunction with QML! This creates a nested event loop which will cause all…
248​ RuleSettings settings(config, groupName);
249​
250​ const bool remove = settings.deleteRule();
251​ const QString importDescription = settings.description();
252​ if (importDescription.isEmpty()) {
253​ continue;
254​ }
255​
256​ // Try to find a rule with the same description to replace
257​ int newIndex = -2;
258​ for (int index = 0; index < m_rules.count(); index++) {
259​ if (m_rules.at(index)->description == importDescription) {
260​ newIndex = index;
261​ break;
262​ }
263​ }
264​
265​ if (remove) {
266​ removeRule(newIndex);
267​ continue;
268​ }
269​
270​ Rules *newRule = new Rules(&settings);
271​
272​ if (newIndex < 0) {
273​ m_rules.append(newRule);
274​ } else {
275​ delete m_rules[newIndex];
276​ m_rules[newIndex] = newRule;
277​ }
278​
279​ // Reset rule editor if the current rule changed when importing
280​ if (m_editingIndex == newIndex) {
281​ m_rulesModel->importFromRules(m_rules.at(m_editingIndex));
282​ }
283​ }
284​
285​ updateState();
286​}
287​
288​K_PLUGIN_CLASS_WITH_JSON(KCMKWinRules, "kcm_kwinrules_qml.json");
289​
290​} // namespace
291​
292​#include "kcmrules.moc"