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

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  thanks for the update, some small nits then this is fine to go in.

INLINE COMMENTS

> settingsmanager.cpp:73
> +        return QStringLiteral("parserArgumentsCuda");
> +    default:
> +        Q_UNREACHABLE();

make this Utils::Other, such that we get a compile warning when we extend the enum later

> settingsmanager.cpp:144
>          pathgrp.writeEntry(ConfigConstants::projectPathKey, path.path);
> -        pathgrp.writeEntry(ConfigConstants::parserArgumentsCPP(), path.parserArguments.cppArguments);
> -        pathgrp.writeEntry(ConfigConstants::parserArgumentsC(), path.parserArguments.cArguments);
> +        for (Utils::LanguageType type : configurableLanguageTypes) {
> +            pathgrp.writeEntry(ConfigConstants::parserArgumentsKey(type), path.parserArguments[type]);

auto?

> settingsmanager.cpp:179
>          path.path = pathgrp.readEntry( ConfigConstants::projectPathKey, "" );
> -        path.parserArguments.cppArguments = pathgrp.readEntry(ConfigConstants::parserArgumentsCPP(), defaultArguments().cppArguments);
> -        path.parserArguments.cArguments = pathgrp.readEntry(ConfigConstants::parserArgumentsC(), defaultArguments().cArguments);
> -        path.parserArguments.parseAmbiguousAsCPP = pathgrp.readEntry(ConfigConstants::parseAmbiguousAsCPP(), defaultArguments().parseAmbiguousAsCPP);
> -
> -        if (path.parserArguments.cppArguments.isEmpty()) {
> -            path.parserArguments.cppArguments = defaultArguments().cppArguments;
> +        for (Utils::LanguageType type : configurableLanguageTypes) {
> +            path.parserArguments[type] = pathgrp.readEntry(ConfigConstants::parserArgumentsKey(type), defaultArguments()[type]);

auto?

> settingsmanager.cpp:184
>  
> -        if (path.parserArguments.cArguments.isEmpty()) {
> -            path.parserArguments.cArguments = defaultArguments().cArguments;
> +        for (Utils::LanguageType type : configurableLanguageTypes) {
> +            if (path.parserArguments[type].isEmpty()) {

auto?

> settingsmanager.h:54
> +
> +    /// Is any of arguments empty?
> +    bool isAnyEmpty() const;

missing word: "the"

> parserwidget.cpp:60
> +        return QLatin1String("c99");
> +    default:
> +        Q_UNREACHABLE();

dito, explicitly mention Other here

REPOSITORY
  R32 KDevelop

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

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