| kfunk | |
| apol |
| KDevelop |
Arguments from project option were not applied or applied
incorrectly since D7752. This patch fixes sime problem when setting
custom arguments to the parser.
Also, since we take arguments from the build system, warning as error
became effective on the parser. I added Wno-error at the end of the
default parser arguments.
| No Linters Available |
| No Unit Test Coverage |
I've almost had the same patch locally. So +1 in general.
I'm just wondering if we should rather append the "extraArguments" *after* the project-specific ones. My idea: If e.g. the CMake manager dictates -std=c++14, with my patch this would be part of extraArguments and then the standard version in use by libclang.
With your patch, we would use the -std=... argument used from the parser settings, which might not match what the user wants. IMO we should try to get everything working 'out of the box' as much as possible.
For the record, my patch:
diff --git a/plugins/custom-definesandincludes/definesandincludesmanager.cpp b/plugins/custom-definesandincludes/definesandincludesmanager.cpp index de317ae977..02b5f48f12 100644 --- a/plugins/custom-definesandincludes/definesandincludesmanager.cpp +++ b/plugins/custom-definesandincludes/definesandincludesmanager.cpp @@ -356,16 +356,18 @@ QString DefinesAndIncludesManager::parserArguments(KDevelop::ProjectBaseItem* it Q_ASSERT(QThread::currentThread() == qApp->thread()); + auto cfg = item->project()->projectConfiguration().data(); + const auto parserArguments = findConfigForItem(m_settings->readPaths(cfg), item).parserArguments; + auto arguments = argumentsForPath(item->path(), parserArguments); + auto buildManager = item->project()->buildSystemManager(); - if ( buildManager ) { - const auto args = buildManager->extraArguments(item); - if (!args.isEmpty()) - return args; + if (buildManager) { + const auto extraArguments = buildManager->extraArguments(item); + if (!extraArguments.isEmpty()) + arguments += ' ' + extraArguments; } - auto cfg = item->project()->projectConfiguration().data(); - const auto arguments = findConfigForItem(m_settings->readPaths(cfg), item).parserArguments; - return argumentsForPath(item->path(), arguments); + return arguments; } QString DefinesAndIncludesManager::parserArguments(const QString& path) const
| plugins/custom-definesandincludes/definesandincludesmanager.cpp | ||
|---|---|---|
| 366 | Shouldn't it get a ' ' in case it starts and ends with options? | |
Seems reasonable @kfunk . I'll update this patch accordingly. Although, I think there should be an option to "override" buildsystem arguments for the parser, to enable more fine grained option about what warnings the parse emit or not.
Since the intent of adding -Wno-error was to override the -Werror from the build system, it no longer make sense to add it. I'll remove it from this patch.
Also, it raises the question whether extra arguments for the build system is doing well on windows. Compiler arguments are quite different there.
+1 if this improves the situation, but in general I was suspecting such issues while reviewing the initial changeset. I suspect we'll see more issues in the future, and I have no clear idea on how to handle that :(