<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="https://git.reviewboard.kde.org/r/115696/">https://git.reviewboard.kde.org/r/115696/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 18th, 2014, 1:40 a.m. MSK, <b>Milian Wolff</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;">Woha, big patch! Thanks for the effort you put into this feature, much appreciated. There are a few things I'd like to see changed, but all in all this looks very good already.
If we can then in the future move the make-resolve stuff into a second plugin which implements the interface I want to have added, then it can easily be disabled if wanted.</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;">Thanks for the review, btw!!</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 18th, 2014, 1:40 a.m. MSK, <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/115696/diff/1/?file=243514#file243514line93" style="color: black; font-weight: bold; text-decoration: underline;">projectmanagers/custom-definesandincludes/customdefinesandincludesmanager.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">93</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QMutex</span> <span class="n">m_locker</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">why this one? should/is this code threadsafe? it should not have to.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Well, ParseJobs use this code. AFAIK many parse jobs can be running at the same time, but there is only one instance of CustomDefinesAndIncludesManager, so...</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 18th, 2014, 1:40 a.m. MSK, <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/115696/diff/1/?file=243530#file243530line176" style="color: black; font-weight: bold; text-decoration: underline;">projectmanagers/custom-definesandincludes/kcm_widget/projectpathsmodel.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">bool ProjectPathsModel::removeRows( int row, int count, const QModelIndex& parent )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">176</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="o">!</span><span class="n">project</span><span class="o">-></span><span class="n"><span class="hl">inProject</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">KDevelop</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">IndexedString</span></span><span class="p">(</span><span class="n">url</span><span class="p">)<span class="hl">)</span>)</span> <span class="p">{</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">173</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span><span class="p">(</span> <span class="o">!</span><span class="n">project</span><span class="o">-></span><span class="n"><span class="hl">folder</span></span><span class="p"><span class="hl">().</span></span><span class="n"><span class="hl">isParentOf</span></span><span class="p">(</span><span class="n">url</span><span class="p">)</span> <span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this should trigger new deprecation warnings for which I am to blame in some part.
anyhow - please don't use the deprecated API. Prefer the KDevelop::Path API by doing project->path().toUrl().isParentOf(url)</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Yes, I wanted to do it that way too, but that API return a slightly different result e.g. project->folder() returns /some/path/ while project->path().toUrl() /some/path (without /), and projectpathsmodel.cpp relies on trailing /. But you're right anyway, so I've workarounded it.
BTW is it desired behavior or just a project's API bug?</pre>
<br />
<p>- Sergey</p>
<br />
<p>On February 22nd, 2014, 9:02 p.m. MSK, Sergey Kalinichev wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Sergey Kalinichev.</div>
<p style="color: grey;"><i>Updated Feb. 22, 2014, 9:02 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=254662">254662</a>,
<a href="http://bugs.kde.org/show_bug.cgi?id=329788">329788</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdevelop
</div>
<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;">The features are:
1. It's possible to add includes/defines for files/directories from withing a project.
2. Data stored in Custom Build Manager's format.
3. Language plugins use IDefinesAndIncludes interface to retrieve custom includes/defines.
There are questions though:
1. What to do with .kdev_include_paths files? Should we convert it's data to the new format upon project loading or we could just ignore it for projects and use it only for "out of project" files?
2. For some reasong this plugin shows up above others in the list of plugins (see screenshots), is it ok, if not how to change that behaviour?
3. Does other languages have defines/includes notion, if not how to make it available only for C/C++ then? (X-KDevelop-Language=C++ doesn't seem to work...)
Also there are some features missing:
1. So far there is no notification mechanism about changed defines/includes. (that is you have to press F5 to see the changes).
2. There is no caching (I wonder if it needed at all?)
...</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;">Yes, modified/enhanced tests a little bit.</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>languages/plugins/custom-definesandincludes/kcm_widget/kcm_customdefinesandincludes.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/kcm_kdevcustomdefinesandincludes.desktop <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathsmodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/customdefinesandincludes.kcfgc <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/definesmodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/definesmodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/defineswidget.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/defineswidget.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/includesmodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/includesmodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/includeswidget.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/includeswidget.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/kcm_customdefinesandincludes.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/customdefinesandincludesmanager.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/debugarea.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/debugarea.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/customdefinesandincludes.kcfg <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/CMakeLists.txt <span style="color: grey">(847426b)</span></li>
<li>languages/cpp/includepathcomputer.cpp <span style="color: grey">(325ae55)</span></li>
<li>languages/plugins/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/customdefinesandincludesmanager.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathsmodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kdevcustomdefinesandincludesmanager.desktop.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/settingsconverter.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/settingsconverter.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/settingsmanager.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/settingsmanager.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/plugintest.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/plugintest.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/projects/multipathproject/.kdev4/multipathproject.kdev4 <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/projects/multipathproject/config.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/projects/multipathproject/multipathproject.kdev4 <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/projects/multipathproject/src/main.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/projects/simpleproject/.kdev4/simpleproject.kdev4 <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/projects/simpleproject/config.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/projects/simpleproject/simpleproject.kdev4 <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/projects/simpleproject/src/main.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/testconfig.h.in <span style="color: grey">(PRE-CREATION)</span></li>
<li>projectmanagers/custom-buildsystem/CMakeLists.txt <span style="color: grey">(e742fe4)</span></li>
<li>projectmanagers/custom-buildsystem/configconstants.h <span style="color: grey">(1f25752)</span></li>
<li>projectmanagers/custom-buildsystem/configwidget.h <span style="color: grey">(85dcb09)</span></li>
<li>projectmanagers/custom-buildsystem/configwidget.cpp <span style="color: grey">(6a665a6)</span></li>
<li>projectmanagers/custom-buildsystem/configwidget.ui <span style="color: grey">(d2157d4)</span></li>
<li>projectmanagers/custom-buildsystem/custombuildsystemconfig.h <span style="color: grey">(d9844b2)</span></li>
<li>projectmanagers/custom-buildsystem/custombuildsystemconfigwidget.h <span style="color: grey">(9f2efcf)</span></li>
<li>projectmanagers/custom-buildsystem/custombuildsystemconfigwidget.cpp <span style="color: grey">(1c3f770)</span></li>
<li>projectmanagers/custom-buildsystem/custombuildsystemplugin.cpp <span style="color: grey">(074a338)</span></li>
<li>projectmanagers/custom-buildsystem/tests/CMakeLists.txt <span style="color: grey">(6332faf)</span></li>
<li>projectmanagers/custom-buildsystem/tests/custombuildsystemplugintest.cpp <span style="color: grey">(ea9e2f5)</span></li>
<li>projectmanagers/custom-buildsystem/tests/projects/builddirproject/.kdev4/builddirproject.kdev4 <span style="color: grey">(d3d1e52)</span></li>
<li>projectmanagers/custom-buildsystem/tests/projects/multipathproject/.kdev4/multipathproject.kdev4 <span style="color: grey">(f988fe6)</span></li>
<li>projectmanagers/custom-buildsystem/tests/projects/simpleproject/.kdev4/simpleproject.kdev4 <span style="color: grey">(c5a856a)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/115696/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/02/12/c7683483-a74f-4f84-a91d-127af710dab4__custom_build_system_before.png">custom_build_system_before.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/02/12/f4473409-be37-435a-a600-4a0159e6dd96__custom_build_system_after.png">custom_build_system_after.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/02/12/e49fb372-dd4e-4715-bfa4-5dac59caf0be__custom_defines_and_includes.png">custom_defines_and_includes.png</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/02/22/eabc7944-30c4-44f4-a0e9-99435c5dc2f9__interface_in_kdevplatform">interface_in_kdevplatform</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/02/22/bf556239-493b-47c5-9610-11be03cb747d__diff_full">full patch</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>