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