D8634: Custom defines/includes: Improve handling of parser arguments

[Aaron Puchert]

aaronpuchert added a comment.


  I'm going to address the comments I didn't reply to.
  
  One problem remains: in `test_definesandincludes`, some of the methods (`.includes()`, `.defines()`, `.parserArguments()`) are called on directories, which can't really work. (Directories aren't compiled, first of all, and of course they can contain files of different languages.)
  
  Even before this patch, `inode/directory` is mapped to `Utils::Other` in `Utils::languageType()`, for which we return no parser arguments. When asked for includes/defines, we just fail with `Q_UNREACHABLE()`.
  
  I'm not sure how to proceed there. The tests were probably written at a time when only C/C++ were supported, and there was no issue. Of course we can still return the C++ flags/includes/defines, but why should we?

INLINE COMMENTS

> mwolff wrote in settingsmanager.cpp:42
> make this const

Maybe even constexpr?

> mwolff wrote in settingsmanager.cpp:435
> this should probably be a separate patch

Done <https://phabricator.kde.org/D8644>.

> mwolff wrote in settingsmanager.h:42
> rename to `argumentsForLanguage`, or make it an `operator[]() const`

In fact I had this as `operator[]` originally, but the problem was that `get` and `set` have slightly different domains as of now. Meaning that there are some languages for which we don't have a setting, but instead just return an empty string. (See `argumentsForPath` in plugins/custom-definesandincludes/definesandincludesmanager.cpp)

I agree that this is a bit weird, but I'm not sure how to handle it. Do we want to persist options for Objective C as well? And why do we have `Other`?

REPOSITORY
  R32 KDevelop

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

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