D8634: Custom defines/includes: Improve handling of parser arguments
Aaron Puchert
noreply at phabricator.kde.org
Mon Nov 27 00:26:04 UTC 2017
aaronpuchert added a comment.
In https://phabricator.kde.org/D8634#163888, @mwolff wrote:
> I figured - couldn't we simply replace `ParserArguments` with e.g. a `QHash<Utils::LanguageType, QString>`? Or, better yet, we make it a `QString[Utils::LanguageType::Other]`?
There is still that flag `parseAmbiguousAsCPP`, but I like the idea of using an array. Maybe we can get rid of the flag, but I wouldn't want to do too much in one change.
> you'd still be able to use brace-initialization
As stated in the commit message, I don't think a bare-bones initializer list of strings is very readable, because it's not clear which arguments belong to which language when reading it. However, I don't see any issue with an initializer list consisting of `std::pair<Utils::LanguageType, QString>`, if you like that.
INLINE COMMENTS
> mwolff wrote in settingsmanager.cpp:59
> rename to `configKey` or such?
That's already used in line 45. I could name it `parserArgumentsKey`.
> mwolff wrote in settingsmanager.cpp:79
> move this down, such that the anon namespace includes all util functions, esp. the ones you are touching.
This closes the `ConfigConstants` namespace, so I guess I'm going to leave it there.
The anonymous namespace is closed on line 266.
REPOSITORY
R32 KDevelop
REVISION DETAIL
https://phabricator.kde.org/D8634
To: aaronpuchert, #kdevelop, apol
Cc: apol, mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20171127/769843db/attachment-0001.html>
More information about the KDevelop-devel
mailing list