Changeset View
Standalone View
kdevplatform/shell/sourceformatterselectionedit.cpp
Show First 20 Lines • Show All 106 Lines • ▼ Show 20 Line(s) | 84 | { | |||
---|---|---|---|---|---|
107 | d->view->show(); | 107 | d->view->show(); | ||
108 | 108 | | |||
109 | KTextEditor::ConfigInterface *iface = | 109 | KTextEditor::ConfigInterface *iface = | ||
110 | qobject_cast<KTextEditor::ConfigInterface*>(d->view); | 110 | qobject_cast<KTextEditor::ConfigInterface*>(d->view); | ||
111 | if (iface) { | 111 | if (iface) { | ||
112 | iface->setConfigValue(QStringLiteral("dynamic-word-wrap"), false); | 112 | iface->setConfigValue(QStringLiteral("dynamic-word-wrap"), false); | ||
113 | iface->setConfigValue(QStringLiteral("icon-bar"), false); | 113 | iface->setConfigValue(QStringLiteral("icon-bar"), false); | ||
114 | } | 114 | } | ||
115 | | ||||
116 | SourceFormatterController* controller = Core::self()->sourceFormatterControllerInternal(); | ||||
117 | connect(controller, &SourceFormatterController::formatterLoaded, | ||||
118 | this, &SourceFormatterSelectionEdit::addSourceFormatter); | ||||
119 | connect(controller, &SourceFormatterController::formatterUnloading, | ||||
120 | this, &SourceFormatterSelectionEdit::removeSourceFormatter); | ||||
121 | for (auto* formatter : controller->formatters()) { | ||||
mwolff: `auto* `, to make clear this is a pointer. Otherwise it should be `const &` or `const &&` | |||||
122 | addSourceFormatter(formatter); | ||||
123 | } | ||||
115 | } | 124 | } | ||
116 | 125 | | |||
117 | SourceFormatterSelectionEdit::~SourceFormatterSelectionEdit() | 126 | SourceFormatterSelectionEdit::~SourceFormatterSelectionEdit() | ||
118 | { | 127 | { | ||
119 | qDeleteAll(d->formatters); | 128 | qDeleteAll(d->formatters); | ||
120 | } | 129 | } | ||
121 | 130 | | |||
122 | static void selectAvailableStyle(LanguageSettings& lang) | 131 | static void selectAvailableStyle(LanguageSettings& lang) | ||
123 | { | 132 | { | ||
124 | Q_ASSERT(!lang.selectedFormatter->styles.empty()); | 133 | Q_ASSERT(!lang.selectedFormatter->styles.empty()); | ||
125 | lang.selectedStyle = *lang.selectedFormatter->styles.begin(); | 134 | lang.selectedStyle = *lang.selectedFormatter->styles.begin(); | ||
126 | } | 135 | } | ||
127 | 136 | | |||
128 | void SourceFormatterSelectionEdit::loadSettings(const KConfigGroup& config) | 137 | void SourceFormatterSelectionEdit::addSourceFormatter(ISourceFormatter* ifmt) | ||
129 | { | 138 | { | ||
130 | SourceFormatterController* fmtctrl = Core::self()->sourceFormatterControllerInternal(); | 139 | SourceFormatter* formatter; | ||
131 | QList<KDevelop::IPlugin*> plugins = KDevelop::ICore::self()->pluginController()->allPluginsForExtension( QStringLiteral("org.kdevelop.ISourceFormatter") ); | | |||
132 | foreach( KDevelop::IPlugin* plugin, plugins ) | | |||
133 | { | | |||
134 | KDevelop::ISourceFormatter* ifmt = plugin->extension<ISourceFormatter>(); | | |||
135 | auto info = KDevelop::Core::self()->pluginControllerInternal()->pluginInfo( plugin ); | | |||
136 | KDevelop::SourceFormatter* formatter; | | |||
137 | FormatterMap::const_iterator iter = d->formatters.constFind(ifmt->name()); | 140 | FormatterMap::const_iterator iter = d->formatters.constFind(ifmt->name()); | ||
138 | if (iter == d->formatters.constEnd()) { | 141 | if (iter == d->formatters.constEnd()) { | ||
139 | formatter = fmtctrl->createFormatterForPlugin(ifmt); | 142 | formatter = Core::self()->sourceFormatterControllerInternal()->createFormatterForPlugin(ifmt); | ||
140 | d->formatters[ifmt->name()] = formatter; | 143 | d->formatters[ifmt->name()] = formatter; | ||
141 | } else { | 144 | } else { | ||
142 | formatter = iter.value(); | 145 | qCWarning(SHELL) << "formatter plugin" << ifmt->name() << "loading which was already seen before by SourceFormatterSelectionEdit"; | ||
146 | return; | ||||
143 | } | 147 | } | ||
148 | | ||||
144 | foreach ( const SourceFormatterStyle* style, formatter->styles ) { | 149 | foreach ( const SourceFormatterStyle* style, formatter->styles ) { | ||
145 | foreach ( const SourceFormatterStyle::MimeHighlightPair& item, style->mimeTypes() ) { | 150 | foreach ( const SourceFormatterStyle::MimeHighlightPair& item, style->mimeTypes() ) { | ||
146 | QMimeType mime = QMimeDatabase().mimeTypeForName(item.mimeType); | 151 | QMimeType mime = QMimeDatabase().mimeTypeForName(item.mimeType); | ||
147 | if (!mime.isValid()) { | 152 | if (!mime.isValid()) { | ||
148 | qCWarning(SHELL) << "plugin" << info.name() << "supports unknown mimetype entry" << item.mimeType; | 153 | qCWarning(SHELL) << "formatter plugin" << ifmt->name() << "supports unknown mimetype entry" << item.mimeType; | ||
149 | continue; | 154 | continue; | ||
150 | } | 155 | } | ||
151 | QString languageName = item.highlightMode; | 156 | QString languageName = item.highlightMode; | ||
152 | LanguageSettings& l = d->languages[languageName]; | 157 | LanguageSettings& l = d->languages[languageName]; | ||
153 | l.mimetypes.append(mime); | 158 | l.mimetypes.append(mime); | ||
154 | l.formatters.insert( formatter ); | 159 | l.formatters.insert( formatter ); | ||
160 | // init selection if needed | ||||
161 | if (!l.selectedFormatter) { | ||||
162 | l.selectedFormatter = formatter; | ||||
163 | selectAvailableStyle(l); | ||||
155 | } | 164 | } | ||
156 | } | 165 | } | ||
157 | } | 166 | } | ||
158 | 167 | | |||
159 | // Sort the languages, preferring firstly active, then loaded languages | 168 | resetUi(); | ||
160 | QList<QString> sortedLanguages; | 169 | } | ||
161 | 170 | | |||
162 | foreach(const auto language, | 171 | void SourceFormatterSelectionEdit::removeSourceFormatter(ISourceFormatter* ifmt) | ||
163 | KDevelop::ICore::self()->languageController()->activeLanguages() + | | |||
164 | KDevelop::ICore::self()->languageController()->loadedLanguages()) | | |||
165 | { | 172 | { | ||
166 | if (d->languages.contains(language->name()) && !sortedLanguages.contains(language->name())) { | 173 | auto iter = d->formatters.find(ifmt->name()); | ||
167 | sortedLanguages.push_back( language->name() ); | 174 | if (iter == d->formatters.end()) { | ||
175 | qCWarning(SHELL) << "formatter plugin" << ifmt->name() << "unloading which was not seen before by SourceFormatterSelectionEdit"; | ||||
176 | return; | ||||
177 | } | ||||
178 | d->formatters.erase(iter); | ||||
179 | auto formatter = iter.value(); | ||||
180 | | ||||
181 | auto languageIter = d->languages.begin(); | ||||
182 | while (languageIter != d->languages.end()) { | ||||
183 | LanguageSettings& l = languageIter.value(); | ||||
184 | | ||||
185 | l.formatters.remove(formatter); | ||||
186 | if (l.formatters.isEmpty()) { | ||||
187 | languageIter = d->languages.erase(languageIter); | ||||
188 | } else { | ||||
189 | // reset selected formatter if needed | ||||
190 | if (l.selectedFormatter == formatter) { | ||||
191 | l.selectedFormatter = *l.formatters.begin(); | ||||
192 | selectAvailableStyle(l); | ||||
168 | } | 193 | } | ||
194 | ++languageIter; | ||||
169 | } | 195 | } | ||
mwolff: use remove_if + erase | |||||
Idea here is that while iterating over the structure to delete things at the same time also data is updated, cmp. the snippet behind // reset selected formatter if needed. kossebau: Idea here is that while iterating over the structure to delete things at the same time also… | |||||
196 | } | ||||
197 | delete formatter; | ||||
170 | 198 | | |||
171 | foreach (const QString& name, d->languages.keys()) { | 199 | resetUi(); | ||
172 | if( !sortedLanguages.contains( name ) ) | | |||
173 | sortedLanguages.push_back( name ); | | |||
174 | } | 200 | } | ||
175 | 201 | | |||
176 | foreach( const QString& name, sortedLanguages ) | 202 | void SourceFormatterSelectionEdit::loadSettings(const KConfigGroup& config) | ||
177 | { | 203 | { | ||
204 | for (auto languageIter = d->languages.begin(); languageIter != d->languages.end(); ++languageIter) { | ||||
mwolff: this cleanup could be done in a separate patch and committed directly | |||||
What looks like a clean-up though is switching from a QStringList to iterate over to a QMap. With code which is reused from the old version, but actually a left-over from a method whose content is now distributed over multiple methods. The old code used the sorted list of languages also for loading from the config, where though the sorting does not matter, so the new code which just cares for the loading iterates directly over the map. I don't think another intermediate change here helps tracking the changes? kossebau: What looks like a clean-up though is switching from a QStringList to iterate over to a QMap. | |||||
178 | // Pick the first appropriate mimetype for this language | 205 | // Pick the first appropriate mimetype for this language | ||
179 | LanguageSettings& l = d->languages[name]; | 206 | LanguageSettings& l = languageIter.value(); | ||
180 | const QList<QMimeType> mimetypes = l.mimetypes; | 207 | const QList<QMimeType> mimetypes = l.mimetypes; | ||
181 | foreach (const QMimeType& mimetype, mimetypes) { | 208 | foreach (const QMimeType& mimetype, mimetypes) { | ||
182 | QStringList formatterAndStyleName = config.readEntry(mimetype.name(), QString()).split(QStringLiteral("||"), QString::KeepEmptyParts); | 209 | QStringList formatterAndStyleName = config.readEntry(mimetype.name(), QString()).split(QStringLiteral("||"), QString::KeepEmptyParts); | ||
183 | FormatterMap::const_iterator formatterIter = d->formatters.constFind(formatterAndStyleName.first()); | 210 | FormatterMap::const_iterator formatterIter = d->formatters.constFind(formatterAndStyleName.first()); | ||
184 | if (formatterIter == d->formatters.constEnd()) { | 211 | if (formatterIter == d->formatters.constEnd()) { | ||
185 | qCDebug(SHELL) << "Reference to unknown formatter" << formatterAndStyleName.first(); | 212 | qCDebug(SHELL) << "Reference to unknown formatter" << formatterAndStyleName.first(); | ||
186 | Q_ASSERT(!l.formatters.empty()); // otherwise there should be no entry for 'name' | 213 | Q_ASSERT(!l.formatters.empty()); // otherwise there should be no entry for 'name' | ||
187 | l.selectedFormatter = *l.formatters.begin(); | 214 | l.selectedFormatter = *l.formatters.begin(); | ||
Show All 12 Lines | |||||
200 | if (!l.selectedFormatter) { | 227 | if (!l.selectedFormatter) { | ||
201 | Q_ASSERT(!l.formatters.empty()); | 228 | Q_ASSERT(!l.formatters.empty()); | ||
202 | l.selectedFormatter = *l.formatters.begin(); | 229 | l.selectedFormatter = *l.formatters.begin(); | ||
203 | } | 230 | } | ||
204 | if (!l.selectedStyle) { | 231 | if (!l.selectedStyle) { | ||
205 | selectAvailableStyle(l); | 232 | selectAvailableStyle(l); | ||
206 | } | 233 | } | ||
207 | } | 234 | } | ||
235 | | ||||
236 | resetUi(); | ||||
237 | } | ||||
238 | | ||||
239 | void SourceFormatterSelectionEdit::resetUi() | ||||
240 | { | ||||
241 | // Sort the languages, preferring firstly active, then loaded languages | ||||
242 | QList<QString> sortedLanguages; | ||||
243 | | ||||
244 | foreach(const auto language, | ||||
I realize this is old code, but could you do me a favor and clean it up (potentially in a follow-up commit?) It makes my eyes bleed. Instead, do something like // Sort the languages, preferring firstly active, then loaded languages, then any others for (const auto& languages : {ICore::self()->languageController()->activeLanguages(), ICore::self()->languageController()->loadedLanguages()}) { for (const auto* language : languages) { if (d->languages.contains(language->name()) { auto it = std::find(languagesToAdd.begin(), languagesToAdd.end(), language); if (it == languagesToAdd.end()) { return; } languagesToAdd.erase(it); d->ui.cbLanguages->addItem(name); } } } for (const auto &language : languagesToAdd) { d->ui.cbLanguages->addItem(language); } mwolff: I realize this is old code, but could you do me a favor and clean it up (potentially in a… | |||||
245 | KDevelop::ICore::self()->languageController()->activeLanguages() + | ||||
246 | KDevelop::ICore::self()->languageController()->loadedLanguages()) | ||||
247 | { | ||||
248 | if (d->languages.contains(language->name()) && !sortedLanguages.contains(language->name())) { | ||||
249 | sortedLanguages.push_back( language->name() ); | ||||
250 | } | ||||
251 | } | ||||
252 | | ||||
253 | foreach (const QString& name, d->languages.keys()) { | ||||
kfunk: Minor: Odd coding style | |||||
See Milian's comment above. Old code which was moved around (but the diff display manages to blur this :/) Candidate for separate clean-up as requested above by Milian. kossebau: See Milian's comment above. Old code which was moved around (but the diff display manages to… | |||||
254 | if( !sortedLanguages.contains( name ) ) | ||||
255 | sortedLanguages.push_back( name ); | ||||
256 | } | ||||
257 | | ||||
208 | bool b = blockSignals( true ); | 258 | bool b = blockSignals( true ); | ||
209 | d->ui.cbLanguages->blockSignals(!b); | 259 | d->ui.cbLanguages->blockSignals(!b); | ||
210 | d->ui.cbFormatters->blockSignals(!b); | 260 | d->ui.cbFormatters->blockSignals(!b); | ||
211 | d->ui.styleList->blockSignals(!b); | 261 | d->ui.styleList->blockSignals(!b); | ||
212 | d->ui.cbLanguages->clear(); | 262 | d->ui.cbLanguages->clear(); | ||
213 | d->ui.cbFormatters->clear(); | 263 | d->ui.cbFormatters->clear(); | ||
214 | d->ui.styleList->clear(); | 264 | d->ui.styleList->clear(); | ||
215 | foreach( const QString& name, sortedLanguages ) | 265 | foreach( const QString& name, sortedLanguages ) | ||
216 | { | 266 | { | ||
217 | d->ui.cbLanguages->addItem(name); | 267 | d->ui.cbLanguages->addItem(name); | ||
218 | } | 268 | } | ||
219 | if (d->ui.cbLanguages->count() == 0) { | 269 | if (d->ui.cbLanguages->count() == 0) { | ||
220 | d->ui.cbLanguages->setEnabled(false); | 270 | d->ui.cbLanguages->setEnabled(false); | ||
221 | selectLanguage( -1 ); | 271 | selectLanguage( -1 ); | ||
222 | } else | 272 | } else | ||
223 | { | 273 | { | ||
224 | d->ui.cbLanguages->setCurrentIndex(0); | 274 | d->ui.cbLanguages->setCurrentIndex(0); | ||
275 | d->ui.cbLanguages->setEnabled(true); | ||||
225 | selectLanguage( 0 ); | 276 | selectLanguage( 0 ); | ||
226 | } | 277 | } | ||
227 | updatePreview(); | 278 | updatePreview(); | ||
228 | blockSignals( b ); | 279 | blockSignals( b ); | ||
229 | d->ui.cbLanguages->blockSignals(b); | 280 | d->ui.cbLanguages->blockSignals(b); | ||
230 | d->ui.cbFormatters->blockSignals(b); | 281 | d->ui.cbFormatters->blockSignals(b); | ||
231 | d->ui.styleList->blockSignals(b); | 282 | d->ui.styleList->blockSignals(b); | ||
232 | } | 283 | } | ||
▲ Show 20 Lines • Show All 287 Lines • Show Last 20 Lines |
auto* , to make clear this is a pointer. Otherwise it should be const & or const &&