<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/105299/">http://git.reviewboard.kde.org/r/105299/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 4th, 2012, 8:02 a.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmm, what does the stringlist contain? I don't like string's to transport type-like information and features of a language do rather sound like a flags-type. I'm also wondering what this thing does that needs it in the buildsystem manager and not in the language support itself? An implementation for both sides would probably help understanding the intent.
Oh and even if its decided to go into the project interface I'd say it should be in the base interface, not the buildsystem one so project managers which have no buildsystem support can still supply this feature.</pre>
</blockquote>
<p>On July 4th, 2012, 1:02 p.m., <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think he needs it to tell the (c/c++) language support whether it should be analyzed with a C or a C++ parser. The build system support can tell which one should be used.
On the other hand, I agree that string is too generic.</pre>
</blockquote>
<p>On July 4th, 2012, 7:15 p.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hmm, what information does the buildsystem have that the c++ support does not? I mean currently cmake just sees a bunch of C or C++ files. So this would require some special buildsystem plugin to work in the first place. I dislike that very very much, its the wrong approach IMHO. The parser mode for the C++ parser is completely orthogonal to the buildsystem being used. So the parsing mode shouldn't be something decided based on the buildsystem, but rather based on the actual file.
Can C vs. C++ be determined for a file automatically? If not, I'd say a project config page supplied by the cpp-plugin that lets you choose between C and C++ parsing mode is completely sufficient. If there is sufficient demand for having more finegrained control over the type of parsing per folder or file, then the config page needs to get some support for wildcard matching or the like.
If there is indeed a way to find this out for most of the files, then the c++ plugin should do exactly that.</pre>
</blockquote>
<p>On July 4th, 2012, 9:59 p.m., <b>Aleix Pol Gonzalez</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">well, in the system side, it's cmake who decides if gcc or g++ should be used, no? So it would make sense if we did likewise.
Am I missing anything?</pre>
</blockquote>
<p>On July 5th, 2012, 12:02 a.m., <b>Alexandre Courbot</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">As Aleix pointed out, the build manager is ultimately the only entity who knows how a file should be compiled, by which compiler, and with which options. Thus it is the only one who can tell with precision whether a .h file should be interpreted as C or C++, whether a .c file should be parsed as C99 or ANSI C, etc. Language support is not enough.
As for passing this information as a string, this is because language features are a contract between build manager and language support. The information passed is of type "this file is C99, so don't use the C++ features here". Since we are working at the platform level, concepts like C99 do not exist and thus flags would not be suitable either.</pre>
</blockquote>
<p>On July 5th, 2012, 6:25 a.m., <b>Andreas Pakulat</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">While the buildsystem may be able to know the compiler switches, kdevelop's plugins won't. Thats partially due to not being able to execute all the steps that are declared via the buildsystem - running external processes, writing files etc. are problematic to execute just for parsing and it can be that certain compiler switches are only used after such actions have been performed (un)successfully.
In addition to find this level of information from the buildsystem requires already quite some work for CMake and even more for any other managers we have. I won't ever add support for that to the custom buildsystem manager for example.
So, while such a fully automatic solution might be really nice to have, I think its ultimately not going to be fully working or only for the combination of 1 buildsystem manager and 1 language plugin. Hence My suggestion to instead support this via a config page with a default and some wildcard-matching for individual files. This is not automatic but should suffice for most usescases without causing too much extra work.
Regarding the String I didn't see any arguments from you that would rule out having an enum or some other abstract type. Might even make sense to supply that from the language support, i.e. ask the language support which features it can support on the given file and how they match to the compiler and compiler flags, i.e. ANSI-C - enable when gcc is used with flag X, C99 enable with gcc and -c99 flag, C++ enable with g++, C++0x enable with g++ and flag Y. Heck, I'd put the decision into another API on ILanguageSupport which taks a list of compiler args, filename and compiler binary name and let the language support return an enum value...</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Alexandre, maybe you can try to figure out if it's C it in the language support side?
Also we'd have the same problem we have with the .h files, this should be decided by the language support (aka it's included from c++ or c). In case of the .c/.cpp files, you can try to discriminate by extension, for the moment?
It can look like a hack, but it's what we'd to in the buildsystem side anyway. Also it would mean depending on a newer kdevplatform as well.</pre>
<br />
<p>- Aleix</p>
<br />
<p>On June 20th, 2012, 1:43 a.m., Alexandre Courbot wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Alexandre Courbot.</div>
<p style="color: grey;"><i>Updated June 20, 2012, 1:43 a.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Add support for "language features"
Sometimes the same language can run using different variants - the most
obvious example is the C++ language support, which may also support C
and other variants and behave differently according to the type of file.
This patch adds a new method to IBuildSystemManager allowing it to get a
list of features to pass to ILanguageSupport::createParseJob as an
additional argument. ILanguageSupport can then adapt the behavior of its
parser according to the features the build manager says the parsed file
uses.
Corresponding support for kdevelop (since this patch breaks API compatibility) is there: https://git.reviewboard.kde.org/r/105300/</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Compiled kdevplatform & updated kdevelop, checked things were working.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>language/backgroundparser/backgroundparser.cpp <span style="color: grey">(417a8e4b7f38acfa959959895f186c11e3a76f93)</span></li>
<li>language/backgroundparser/tests/testlanguagesupport.h <span style="color: grey">(ed3864c9e8da8eed97d3d91500eec6c623fae41e)</span></li>
<li>language/backgroundparser/tests/testlanguagesupport.cpp <span style="color: grey">(3f88894d728610ebd433bff46936f38dcd2138be)</span></li>
<li>language/interfaces/ilanguagesupport.h <span style="color: grey">(22cedf09656aaf80275dd3a14d3752003fe9a912)</span></li>
<li>project/interfaces/ibuildsystemmanager.h <span style="color: grey">(c0813d8f781b0be29829b9278f191299af823b68)</span></li>
<li>project/interfaces/ibuildsystemmanager.cpp <span style="color: grey">(74af0e76f8c8bc9276d79ff54be4d3d41927c298)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/105299/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>