<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 March 1st, 2014, 9:32 p.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/4/?file=246083#file246083line32" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/settingsconverter.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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">32</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">const</span> <span class="n">QString</span> <span class="n">definesKey</span><span class="p">(</span><span class="s">"Defines"</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;">here and below:
const QString definesKey = QLatin1String("Defines");</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;">Fixed. But why is it better than just a QString? I've seen this http://qt-project.org/doc/qt-4.8/QLatin1String.html#details, but still it's not clear to me why do we need it (In the current implementation I guess speed isn't an issue, is it because of QT_NO_CAST_FROM/TO_ASCII then?)</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 1st, 2014, 9:32 p.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/4/?file=246085#file246085line53" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/settingsmanager.cpp</a>
<span style="font-weight: normal;">
(Diff revision 4)
</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">53</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">if</span> <span class="p">(</span><span class="n">path</span><span class="p">.</span><span class="n">path</span> <span class="o">==</span> <span class="n">cPath</span><span class="p">.</span><span class="n">path</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;">you modify the path here but never do anything with it then - whats this about? do you wanted to modify cPath?
generally, can you explain this loop with a comment so I can understand what you are trying to do? the repeated loop looks pretty wrong I'd say, at the least its probably more code than required</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;">I was trying to merge converted paths avoiding duplicates. But now looking at the code I see that it was really broken, moreover that branch was never executed, as "convertedPaths" had some values only at the first run when "paths" were empty. So I've just removed it.
Thanks for pointing that out!!</pre>
<br />
<p>- Sergey</p>
<br />
<p>On February 25th, 2014, 10:35 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. 25, 2014, 10:35 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/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/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/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/includeswidget.ui <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/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/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/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/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/kcm_custombuildsystem.cpp <span style="color: grey">(e05874b)</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/kcmuitestmain.cpp <span style="color: grey">(b90d84f)</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>