D8492: Make SourceFormatter plugins enablable, hide actions with no plugins

Friedrich W. H. Kossebau noreply at phabricator.kde.org
Sun Nov 19 22:39:54 UTC 2017


kossebau added inline comments.

INLINE COMMENTS

> mwolff wrote in sourceformatterselectionedit.cpp:181
> 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`.
Would you prefer thus another dedicated loop for that, instead of one integrated?

> mwolff wrote in sourceformatterselectionedit.cpp:204
> 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?

> mwolff wrote in sourceformatterplugin.cpp:43
> why not simply make hasFormatters public, i.e. add it to the sourceFormatterController API?

I did not dare to also propose changes to the abstract interface yet. But followed your suggestion, added together with signal.

REPOSITORY
  R32 KDevelop

REVISION DETAIL
  https://phabricator.kde.org/D8492

To: kossebau, #kdevelop, kfunk, mwolff
Cc: mwolff, kfunk, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171119/c0eaa552/attachment.html>


More information about the KDevelop-devel mailing list