D8634: Custom defines/includes: Improve handling of parser arguments
noreply at phabricator.kde.org
Fri Nov 3 17:53:07 UTC 2017
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?
> 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
> 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`?
To: aaronpuchert, #kdevelop
Cc: mwolff, kdevelop-devel
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the KDevelop-devel