Pkgconfig available at Windows and Mac, but not installed by default and have no a standart place. On Windows pkgconfig may be in MinGW or Cygwin installations, on Mac in macports or in homebrew for example. It's depends from user configuration and wishes. So this functionality is optional and high specialized, but functionality of common-definesandincludes is common and generic. In most cases user cannot disable common-definesandincludes because it's a dependency for cmakebuilder, makebuilder and some other significant plugins.

Continuing from a few thoughts I launched on the original T10209, mainly aimed at keeping the project configuration dialog's left side-bar as unencumbered as possible. I think the current plugin could be merged into the customdefinesandincludes plugin because it provides a programmatic way to add include paths and/or defines.

You'll get more feedback on this when you present your patch via a differential (aka "patch") review.

Already https://phabricator.kde.org/D17794
Sorry, it was my misunderstanding. May i suggest to add link to these instructions to this page :)

This plugin may be useful for any c/c++ project. For example indexer for CMake project KDevelop (sources of ide) don't see all required includes for me.

This plugin may be useful for any c/c++ project. For example indexer for CMake project KDevelop (sources of ide) don't see all required includes for me. I've added paths to includes of QT, KDE and some other manually. And it was much easier than trying to fix automation.

Fix typo

That could be useful for project managers that lack proper build system integration (IOW, there shouldn't be much use for it with the CMake and QMake project managers?)

In D17760#381314, @kossebau wrote:

No time to look in details currently, but IIRC there are some API changes between astyle versions, so version needs to be taken into account instead of just using whatever is provided by the system. Cmp. commit which upgraded the astyle copy recently: 378a6e02cec55390c6dc903d98a035485ec1aa10

Thanks for the patch, @pino, agreed that system things should be preferred (curious myself why there even is a copy).
No time to look in details currently, but IIRC there are some API changes between astyle versions, so version needs to be taken into account instead of just using whatever is provided by the system. Cmp. commit which upgraded the astyle copy recently: 378a6e02cec55390c6dc903d98a035485ec1aa10

Side note: another approach can be to build the astyle plugin only if a system astyle library is found, i.e. make the plugin optional, and get rid of the astyle embedded copy.
I can adopt that approach -- right now I chose the most "conservative" approach.

Remove accidental changes

Looks ok to me! Maybe put it into master?

@zhigalin Thanks for review. @brauch @kfunk If no objections, would push on Monday to 3.5 branch

@brauch @kfunk If no objections, would push on Monday to 3.5 branch.

@mwolff Still "Needs Revision", is the commit message not accurate enough?

I miss the "Ship It" button...

Danke!

@mwolff Done.

See, that's why the commit message should be rephrased :)

@mwolff No, it's not to improve performance. This project will not be built without these changes.
createCompletionContext is an override function, so its definition must correspond with the original one from /usr/include/kdevplatform/language/codecompletion/codecompletionworker.h.

Please rephrase your commit message to indicate what you are fixing. This seems to be a change that fixes a minor performance paper cut, right?

Ping :)

LGTM

No objections?

Yep. please go for it.

Better late than never: I also figured out how to make the plugin's toolview display correctly in KDevelop: https://commits.kde.org/kate/3cd03f408eed2d66ae008fb8349f8b5af24260e9

> ok to have that go in now, thanks!

• Add tests for HeaderGuardAssistant

• QStringLiteral -> i18n

Updated as requested.

undesirable qWarning() removed

Don't worry, no problem.

• Save scratch when run
• Fix scratch removal bug
• Invalidate actions when list is emptied

FIxed and streamlined version.

Also, considering that "session" in `Core::initialize` is user-specified and defaults to an empty string, please do not use a 100% deterministic name,

I think this is a great addition and I see myself using this a lot. So far I've done these things through a set of .cpp files lying around in my $HOME and compiled/ran them using some command-line alias :)

Will push with your version, looks better.
Thanks for the review :)

No, it went beyond that: the original underlying principle was "don't assume or hardcode /usr/local anywhere, use whatever install prefix the user specified".

LGTM in general. If you prefer your version (instead of the one proposed by me) feel free to push directly.

Also, considering that "session" in Core::initialize is user-specified and defaults to an empty string, please do not use a 100% deterministic name, otherwise two different users will have a temporary directory conflict when launching kdevelop. Please use the "XXXXXX" variable part.

Okay, trying to keep this short.

If the problem this patch solves is to have CMAKE_INSTALL_PREFIX to be set for newly-created projects, why not just add a configuration option to the CMake plugin?

Actually, issues with clang temp files is why I started to think about this kind of change but ultimately I realised it didn't seem such a bad idea at all to put all KDevelop-related temporary files in a dedicated location. I often clean out a bunch of KDevelop's own temp files that were left behind, e.g. after a crash. I just didn't mention them because they're negligible in size (and I purge before their numbers really start to grow).

On Linux quite possibly too, I think many packaging systems install into some temporary dir and then copy the deployment out of there for archiving.

• Only configure when config changed
• Better status handling in the settings UI

Hmm, could you remove or comment out:

On Linux quite possibly too, I think many packaging systems install into some temporary dir and then copy the deployment out of there for archiving.

Just some general remarks: Storing the application's install prefix inside the binary itself almost always smells like bad design. Thus the concerns here. You rather want to do this dependent on the kdevelop binary's application path (cf. http://doc.qt.io/qt-5/qcoreapplication.html#applicationDirPath)

I've tested the idea of NOT specifying an install prefix default when none has been configured. To the best I can see this does not lead to invalid cmake invocations (aka -DCMAKE_INSTALL_PREFIX=). Instead, this does exactly what you'd expect: cmake will use whatever is its default for the current platform. The result will be obtained from the CMakeCache later on.
In other words, there doesn't appear to be any reason to hardcode assumptions about CMake's defaults, and not doing that should be a suitable compromise for everyone. It also removes the need for conditional code.

Forgot: Also needs documentation *in* source code why were are doing this. Noone can figure out that the temp dir is changed due to issues with the Clang backend here...

Not usable as-is.

It's quite easy to accidentally start repeated configure jobs, in which case the second one fails - e.g. by clicking "Apply" and then "Ok" in the project settings.