Differential D28761 Diff 81081 src/ui/cryptoconfigmodule.cpp

Changeset View

Standalone View

View Options

src/ui/cryptoconfigmodule.cpp

Show First 20 Lines • Show All 59 Lines • ▼ Show 20 Line(s)
60#include <QDesktopWidget> 60​#include <QDesktopWidget>
61#include <QCheckBox> 61​#include <QCheckBox>
62#include <QStyle> 62​#include <QStyle>
63#include <QComboBox> 63​#include <QComboBox>
64#include <QGroupBox> 64​#include <QGroupBox>
65 65​
66#include <memory> 66​#include <memory>
67#include <limits> 67​#include <limits>
68​#include <array>
68 69​
69using namespace Kleo; 70​using namespace Kleo;
70 71​
71namespace 72​namespace
72{ 73​{
73 74​
74class ScrollArea : public QScrollArea 75​class ScrollArea : public QScrollArea
75{ 76​{
▲ Show 20 Lines • Show All 80 Lines • ▼ Show 20 Line(s)149 if (type == Plain) {
156 l->addWidget(s); 157​ l->addWidget(s);
157 vbox = new QWidget(s->viewport()); 158​ vbox = new QWidget(s->viewport());
158 vlay = new QVBoxLayout(vbox); 159​ vlay = new QVBoxLayout(vbox);
159 vlay->setContentsMargins(0, 0, 0, 0); 160​ vlay->setContentsMargins(0, 0, 0, 0);
160 s->setWidget(vbox); 161​ s->setWidget(vbox);
161 addPage(w, configOK ? QString() : i18n("GpgConf Error")); 162​ addPage(w, configOK ? QString() : i18n("GpgConf Error"));
162 } 163​ }
163 164​
164 const QStringList components = config->componentList(); 165​ const QStringList components = sortComponentList(config->componentList());
165 for (QStringList::const_iterator it = components.begin(); it != components.end(); ++it) { 166​ for (QStringList::const_iterator it = components.begin(); it != components.end(); ++it) {
166 //qCDebug(KLEO_UI_LOG) <<"Component" << (*it).toLocal8Bit() <<":"; 167​ //qCDebug(KLEO_UI_LOG) <<"Component" << (*it).toLocal8Bit() <<":";
167 QGpgME::CryptoConfigComponent *comp = config->component(*it); 168​ QGpgME::CryptoConfigComponent *comp = config->component(*it);
168 Q_ASSERT(comp); 169​ Q_ASSERT(comp);
169 if (comp->groupList().empty()) { 170​ if (comp->groupList().empty()) {
170 continue; 171​ continue;
171 } 172​ }
172 173​
▲ Show 20 Lines • Show All 49 Lines • ▼ Show 20 Line(s)218 const QString msg = i18n("The gpgconf tool used to provide the information "
222 components.empty() ? QLatin1String("gpgconf --list-components") : QLatin1String("gpgconf --list-options gpg")); 223​ components.empty() ? QLatin1String("gpgconf --list-components") : QLatin1String("gpgconf --list-options gpg"));
223 QLabel *label = new QLabel(msg, vbox); 224​ QLabel *label = new QLabel(msg, vbox);
224 label->setWordWrap(true); 225​ label->setWordWrap(true);
225 label->setMinimumHeight(fontMetrics().lineSpacing() * 5); 226​ label->setMinimumHeight(fontMetrics().lineSpacing() * 5);
226 vlay->addWidget(label); 227​ vlay->addWidget(label);
227 } 228​ }
228} 229​}
229 230​
231​QStringList Kleo::CryptoConfigModule::sortComponentList(const QStringList &components)
mlaurentUnsubmitted
Done

new line after ')'

mlaurent: new line after ')'
232​{
233​ // components sorting algorithm:
234​ // 1. components with hardcoded order - see below
235​ // 2. other components sorted alphabetically
mlaurentUnsubmitted
Done

QStringLiteral(...) around each element

mlaurent: QStringLiteral(...) around each element
236​ static const std::array<QString, 6> order = {
dvratilUnsubmitted
Done

Make it static, there's no need to initialize this every time. It could also be something more efficient than QStringList, like std::array (since we know the size of the list beforehand).

dvratil: Make it `static`, there's no need to initialize this every time. It could also be something…
andreylegayevAuthorUnsubmitted
Not Done

Makes sense. Done.
I'm not writing on C++ every day and I'm curious is there difference: const static or static const in this particular place ?

andreylegayev: Makes sense. Done. I'm not writing on C++ every day and I'm curious is there difference…
dvratilUnsubmitted
Not Done

Technically there's no difference, the meaning is the same. But the are people who would argue with you endlessly about which way is the truly correct way of writing static const :)

dvratil: Technically there's no difference, the meaning is the same. But the are people who would argue…
237​ QStringLiteral("gpg"),
238​ QStringLiteral("gpgsm"),
239​ QStringLiteral("gpg-agent"),
240​ QStringLiteral("dirmngr"),
241​ QStringLiteral("pinentry"),
242​ QStringLiteral("scdaemon")
243​ };
244​
245​ QStringList result, others;
246​ for (const auto &item : order) {
247​ if (components.contains(item)) {
248​ result.append(item);
mlaurentUnsubmitted
Done

(const auto &item : order)

mlaurent: (const auto &item : order)
249​ }
250​ }
251​ for (const auto &item : components) {
252​ if (!result.contains(item)) {
253​ others.append(item);
254​ }
255​ }
256​ others.sort();
257​ result.append(others);
258​ return result;
259​}
260​
230bool Kleo::CryptoConfigModule::hasError() const 261​bool Kleo::CryptoConfigModule::hasError() const
231{ 262​{
232 return mComponentGUIs.empty(); 263​ return mComponentGUIs.empty();
dvratilUnsubmitted
Not Done

You could just resolve the others list using another for loop and !result.contains() and get rid of all the QSets completely. The order list as well as the components list will always be short enough so that the two QSets (and the cost of constructing and subtracting them) will not be measurably faster than two for loops and a linear check.

dvratil: You could just resolve the `others` list using another `for` loop and `!result.contains()` and…
andreylegayevAuthorUnsubmitted
Not Done

You're right. I didn't think about small amount of items, tried to write universal code.
Added another for. Pls check new version.

andreylegayev: You're right. I didn't think about small amount of items, tried to write universal code. Added…
233} 264​}
234 265​
235void Kleo::CryptoConfigModule::save() 266​void Kleo::CryptoConfigModule::save()
236{ 267​{
237 bool changed = false; 268​ bool changed = false;
238 QList<CryptoConfigComponentGUI *>::Iterator it = mComponentGUIs.begin(); 269​ QList<CryptoConfigComponentGUI *>::Iterator it = mComponentGUIs.begin();
239 for (; it != mComponentGUIs.end(); ++it) { 270​ for (; it != mComponentGUIs.end(); ++it) {
240 if ((*it)->save()) { 271​ if ((*it)->save()) {
▲ Show 20 Lines • Show All 738 Lines • Show Last 20 Lines