Differential D27395 Diff 76298 kcms/componentchooser/componentchooseremail.cpp

Changeset View

Standalone View

View Options

kcms/componentchooser/componentchooseremail.cpp

Show First 20 Lines • Show All 111 Lines • ▼ Show 20 Line(s)111{
112 if (index == count() -1) { 112​ if (index == count() -1) {
113 // Other option 113​ // Other option
114 114​
115 KOpenWithDialog dlg(s_mimetype, QString(), this); 115​ KOpenWithDialog dlg(s_mimetype, QString(), this);
116 dlg.setSaveNewApplications(true); 116​ dlg.setSaveNewApplications(true);
117 117​
118 if (dlg.exec() != QDialog::Accepted) { 118​ if (dlg.exec() != QDialog::Accepted) {
119 // restore previous setting 119​ // restore previous setting
120 setCurrentIndex(m_currentIndex); 120​ setCurrentIndex(validLastCurrentIndex());
121 emit changed(false); 121​ emit changed(false);
122 } else { 122​ } else {
123 const auto service = dlg.service(); 123​ const auto service = dlg.service();
124 124​
125 const auto icon = QIcon::fromTheme(!service->icon().isEmpty() ? service->icon() : QStringLiteral("application-x-shellscript")); 125​ const auto icon = QIcon::fromTheme(!service->icon().isEmpty() ? service->icon() : QStringLiteral("application-x-shellscript"));
126 insertItem(count() - 1, icon, service->name() + " (" + KShell::tildeCollapse(service->entryPath()) + ")", service->storageId()); 126​ insertItem(count() - 1, icon, service->name() + " (" + KShell::tildeCollapse(service->entryPath()) + ")", service->storageId());
127 127​
128 // select newly inserted email client 128​ // select newly inserted email client
129 setCurrentIndex(count() - 2); 129​ setCurrentIndex(count() - 2);
130 130​
131 emit changed(true); 131​ emit changed(true);
132 return; 132​ return;
133 } 133​ }
134 } else { 134​ } else {
135 emit changed(m_currentIndex != index); 135​ emit changed(m_currentIndex != index);
136 } 136​ }
137} 137​}
138 138​
139void CfgEmailClient::save(KConfig *) 139​void CfgEmailClient::save(KConfig *)
140{ 140​{
141​ if (currentIndex() == count() - 1) {
142​ // no email client installed, nor selected
143​ return;
144​ }
145​
141 const QString &storageId = currentData().toString(); 146​ const QString &storageId = currentData().toString();
142 const KService::Ptr emailClientService = KService::serviceByStorageId(storageId); 147​ const KService::Ptr emailClientService = KService::serviceByStorageId(storageId);
148​ if (!emailClientService) {
149​ // double checking, the selected email client might have been removed
150​ return;
151​ }
143 152​
144 const bool kmailSelected = m_defaultIndex != -1 && currentIndex() == m_defaultIndex; 153​ const bool kmailSelected = m_defaultIndex != -1 && currentIndex() == m_defaultIndex;
145 if (kmailSelected) { 154​ if (kmailSelected) {
146 pSettings->setSetting(KEMailSettings::ClientProgram, QString()); 155​ pSettings->setSetting(KEMailSettings::ClientProgram, QString());
147 pSettings->setSetting(KEMailSettings::ClientTerminal, QStringLiteral("false")); 156​ pSettings->setSetting(KEMailSettings::ClientTerminal, QStringLiteral("false"));
148 } else { 157​ } else {
149 pSettings->setSetting(KEMailSettings::ClientProgram, emailClientService->storageId()); 158​ pSettings->setSetting(KEMailSettings::ClientProgram, emailClientService->storageId());
crossiUnsubmitted
Done

While you are here, can you had a check on this pointer. I don't understand why there is one below but not here.

crossi: While you are here, can you had a check on this pointer. I don't understand why there is one…
mevenAuthorUnsubmitted
Not Done

I would rather do the opposite the check below is for historical reasons from before my refactoring.

meven: I would rather do the opposite the check below is for historical reasons from before my…
mevenAuthorUnsubmitted
Not Done

I meant to remove the check below (which I did).

meven: I meant to remove the check below (which I did).
150 pSettings->setSetting(KEMailSettings::ClientTerminal, emailClientService->terminal() ? QStringLiteral("true") : QStringLiteral("false")); 159​ pSettings->setSetting(KEMailSettings::ClientTerminal, emailClientService->terminal() ? QStringLiteral("true") : QStringLiteral("false"));
151 } 160​ }
152 161​
153 // Save the default email client in mimeapps.list 162​ // Save the default email client in mimeapps.list
154 KSharedConfig::Ptr profile = KSharedConfig::openConfig(QStringLiteral("mimeapps.list"), KConfig::NoGlobals, QStandardPaths::GenericConfigLocation); 163​ KSharedConfig::Ptr profile = KSharedConfig::openConfig(QStringLiteral("mimeapps.list"), KConfig::NoGlobals, QStandardPaths::GenericConfigLocation);
155 if (profile->isConfigWritable(true) && emailClientService) { 164​ if (profile->isConfigWritable(true)) {
ervinUnsubmitted
Done

This actually feels less safe now. What if KService returned a nullptr for some reason?

I understand from the logic of early returns you introduced everywhere that you *expect* something. Still I wonder if that's really guaranteed by KService, we could expect bad config somewhere maybe?

ervin: This actually feels less safe now. What if KService returned a nullptr for some reason? I…
156 165​
157 KSharedConfig::Ptr profile = KSharedConfig::openConfig(QStringLiteral("mimeapps.list"), KConfig::NoGlobals, QStandardPaths::GenericConfigLocation); 166​ KSharedConfig::Ptr profile = KSharedConfig::openConfig(QStringLiteral("mimeapps.list"), KConfig::NoGlobals, QStandardPaths::GenericConfigLocation);
158 167​
159 // Save the default application according to mime-apps-spec 1.0 168​ // Save the default application according to mime-apps-spec 1.0
160 KConfigGroup defaultApp(profile, "Default Applications"); 169​ KConfigGroup defaultApp(profile, "Default Applications");
161 defaultApp.writeXdgListEntry(s_mimetype, {emailClientService->storageId()}); 170​ defaultApp.writeXdgListEntry(s_mimetype, {emailClientService->storageId()});
162 171​
163 KConfigGroup addedApps(profile, "Added Associations"); 172​ KConfigGroup addedApps(profile, "Added Associations");
Show All 13 Lines