<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/118194/">https://git.reviewboard.kde.org/r/118194/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">there's still some duplicated code. And I do stay at what I said earlier: The interface must stay localizable (esp. the "none"), and the compiler names should be "pretty" ("GCC").
Also, I'd rename DAICompilerProvider to CompilerProvider and the ICompilerProvider etc. pp. stuff to just ICompiler. The IDAICompilerProvider should be removed, I think.</pre>
<br />
<div>
<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/118194/diff/2/?file=274314#file274314line114" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/includepathcomputer.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void IncludePathComputer::computeForeground()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">112</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span><span class="p">(</span><span class="k">const</span> <span class="k">auto</span><span class="o">&</span> <span class="n">dir</span> <span class="o">:</span> <span class="n">idm</span><span class="o">-></span><span class="n">includes</span><span class="p">(</span><span class="n">file</span><span class="p">,</span> <span class="n">IDefinesAndIncludesManager</span><span class="o">::</span><span class="n">UserDefined</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">114</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span><span class="p">(</span><span class="k">const</span> <span class="k">auto</span><span class="o">&</span> <span class="n">dir</span> <span class="o">:</span> <span class="n">idm</span><span class="o">-></span><span class="n">includes</span><span class="p">(</span><span class="n">file</span><span class="p">,</span> <span class="n">IDefinesAndIncludesManager</span><span class="o">::</span><span class="n"><span class="hl">Type</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">IDefinesAndIncludesManager</span></span><span class="o"><span class="hl">::</span></span><span class="n">UserDefined</span><span class="hl"> </span><span class="o"><span class="hl">|</span></span><span class="hl"> </span><span class="n"><span class="hl">IDefinesAndIncludesManager</span></span><span class="o"><span class="hl">::</span></span><span class="n"><span class="hl">CompilerSpecific</span></span><span class="p"><span class="hl">)</span>)){</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">nitpick: add a space after for</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274314#file274314line126" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/includepathcomputer.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void IncludePathComputer::computeForeground()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">124</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">m_gotPathsFromManager</span><span class="p">)</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">126</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="n">noProject</span> <span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">nitpick: if (noProject) {</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274314#file274314line127" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/includepathcomputer.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void IncludePathComputer::computeForeground()</pre></td>
</tr>
</tbody>
<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">127</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">for</span><span class="p">(</span> <span class="k">const</span> <span class="k">auto</span><span class="o">&</span> <span class="n">dir</span> <span class="o">:</span> <span class="n">IDefinesAndIncludesManager</span><span class="o">::</span><span class="n">manager</span><span class="p">()</span><span class="o">-></span><span class="n">includes</span><span class="p">(</span><span class="n">nullptr</span><span class="p">)</span> <span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">nitpick: for (const auto& dir : ...->includes(nullptr)) {</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274314#file274314line131" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/includepathcomputer.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">void IncludePathComputer::computeForeground()</pre></td>
</tr>
</tbody>
<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">131</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">}</span><span class="k">else</span> <span class="k">if</span> <span class="p">(</span><span class="o">!</span><span class="n">m_gotPathsFromManager</span><span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">nitpick: } else if ...</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274324#file274324line77" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/daicompilerprovider.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">77</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nl">private:</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">nitpick: add newline before</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274325#file274325line44" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/daicompilerprovider.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">44</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="o">!</span><span class="n">item</span> <span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I'd refactor this code (and the one below) to read:
{
auto project = item ? item->project() : nullptr;
Q_ASSERT(m_projects.contains(project));
auto info = m_projects[project];
Q_ASSERT(info.compiler);
return info.compiler->defines(info.path);
}
The first four lines can (and should) even be shared by both, ::defines and ::includes functions
</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274325#file274325line85" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/daicompilerprovider.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">85</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">m_projects</span><span class="p">[</span><span class="n">project</span><span class="p">].</span><span class="n">compiler</span> <span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">move this block before the assignment to m_projects[project] and operate directly on the compiler object</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274325#file274325line104" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/daicompilerprovider.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">104</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">KStandardDirs</span><span class="o">::</span><span class="n">findExe</span><span class="p">(</span> <span class="n">compiler</span><span class="o">-></span><span class="n">name</span><span class="p">()</span> <span class="p">).</span><span class="n">isEmpty</span><span class="p">()</span> <span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">as I said before - there must be two things here: a user-visible name of the compiler, potentially translatable. E.g. "GCC" or "MSVC". Then, there should be getter function for the path to the default-executable. This would use "gcc" or "msvc". and KStandardDirs::findExe.</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274325#file274325line125" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/daicompilerprovider.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">125</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="o">!</span><span class="n">compiler</span><span class="p">.</span><span class="n">compiler</span> <span class="o">&&</span> <span class="n">name</span> <span class="o">!=</span> <span class="s">"none"</span> <span class="p">)</span> <span class="p">{</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">the comparison to "none" is wrong, this must stay localizable.
I suggest you create a dummy provider for this case which just returns nothing. </pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274328#file274328line35" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/icompilerprovider.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">35</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">ICompilerProvider</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">please add documentation to this class</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274329#file274329line38" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/idaicompilerprovider.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">38</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="n">class</span> <span class="n">IDAICompilerProvider</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This class is only required for the bridge between settings and actual plugin, right?
Can't you just include daicompilerprovider.h and use that directly instead of adding this interface?
Just query for the plugin directly from the settings using its name (kdevcompilerprovider you named it, afaik).</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274335#file274335line84" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/definesandincludesmanager.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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; ">DefinesAndIncludesManager::DefinesAndIncludesManager( QObject* parent, const QVariantList& )</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">81</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">item</span><span class="p">)</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">84</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="n">QThread</span><span class="o">::</span><span class="n">currentThread</span><span class="p">()</span> <span class="o">==</span> <span class="n">qApp</span><span class="o">-></span><span class="kr">thread</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this and b::includes elow share the same code - extract it into a helper function and use that here
</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274342#file274342line138" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.ui</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">128</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <string>Reparse the project</string></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">138</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <string><span class="hl">gcc</span></string></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">must be localizable ("none" in other languages), and pretty-printed (i.e. GCC, MSVC)</pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274344#file274344line27" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/settingsmanager.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">27</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#include <kdemacros.h></span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this is wrong, you'll have to create your own export file similar to those we already have. </pre>
</div>
<br />
<div>
<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/118194/diff/2/?file=274345#file274345line156" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/settingsmanager.cpp</a>
<span style="font-weight: normal;">
(Diff revision 2)
</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">156</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">grp</span><span class="p">.</span><span class="n">readEntry</span><span class="p">(</span> <span class="s">"compiler"</span><span class="p">,</span> <span class="s">""</span> <span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"" -> QString()</pre>
</div>
<br />
<p>- Milian Wolff</p>
<br />
<p>On May 23rd, 2014, 12:27 p.m. UTC, 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 May 23, 2014, 12:27 p.m.</i></p>
<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;">To make it all work I had to turn IADM into global plugin, that way it can be used to retrieve includes/defines for files without project.
Also I made SettingsManager a shared library to ease access to it from different places.
</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;">All tests pass (except cppspecialcompletion, which doesn't work for me anyway).
Also I've tested it with 3 opened projects in one session, works fine for me.</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/cpp/CMakeLists.txt <span style="color: grey">(377f35e)</span></li>
<li>languages/cpp/cpplanguagesupport.cpp <span style="color: grey">(0f3bd4a)</span></li>
<li>languages/cpp/cpputils.h <span style="color: grey">(c951bd3)</span></li>
<li>languages/cpp/cpputils.cpp <span style="color: grey">(380dd4e)</span></li>
<li>languages/cpp/includepathcomputer.cpp <span style="color: grey">(822b0bc)</span></li>
<li>languages/cpp/msvcdefinehelper.cpp <span style="color: grey">(9b131ed)</span></li>
<li>languages/cpp/setuphelpers.h <span style="color: grey">(012dcf4)</span></li>
<li>languages/cpp/setuphelpers.cpp <span style="color: grey">(57505fc)</span></li>
<li>languages/cpp/setuphelpers_gcc_like.cpp <span style="color: grey">(b261589)</span></li>
<li>languages/cpp/setuphelpers_msvc.cpp <span style="color: grey">(5576c01)</span></li>
<li>languages/cpp/tests/CMakeLists.txt <span style="color: grey">(d6eb67a)</span></li>
<li>languages/cpp/tests/test_cppfiles.cpp <span style="color: grey">(76486d0)</span></li>
<li>languages/plugins/custom-definesandincludes/CMakeLists.txt <span style="color: grey">(be7d6e8)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/daicompilerprovider.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/daicompilerprovider.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/gcclikeprovider.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/gcclikeprovider.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/icompilerprovider.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/idaicompilerprovider.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/kdevcompilerprovider.desktop.cmake <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/msvcdefinehelper.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/msvcprovider.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/msvcprovider.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/definesandincludesmanager.h <span style="color: grey">(12a7763)</span></li>
<li>languages/plugins/custom-definesandincludes/definesandincludesmanager.cpp <span style="color: grey">(bdce956)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/CMakeLists.txt <span style="color: grey">(0d8c830)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/customdefinesandincludes.kcfg <span style="color: grey">(3b49940)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/customdefinesandincludes.kcfgc <span style="color: grey">(720af41)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/kcm_customdefinesandincludes.cpp <span style="color: grey">(75f5063)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/kcm_kdevcustomdefinesandincludes.desktop <span style="color: grey">(ef63c57)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp <span style="color: grey">(1456217)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.ui <span style="color: grey">(4e512a9)</span></li>
<li>languages/plugins/custom-definesandincludes/kdevdefinesandincludesmanager.desktop.cmake <span style="color: grey">(ad08867)</span></li>
<li>languages/plugins/custom-definesandincludes/settingsmanager.h <span style="color: grey">(9ae23d6)</span></li>
<li>languages/plugins/custom-definesandincludes/settingsmanager.cpp <span style="color: grey">(dc7aa3e)</span></li>
<li>languages/plugins/custom-definesandincludes/tests/plugintest.cpp <span style="color: grey">(1cfa2e4)</span></li>
<li>projectmanagers/custom-buildsystem/tests/projects/builddirproject/.kdev4/builddirproject.kdev4 <span style="color: grey">(33c8568)</span></li>
<li>projectmanagers/custom-buildsystem/tests/projects/multipathproject/.kdev4/multipathproject.kdev4 <span style="color: grey">(fd14aca)</span></li>
<li>projectmanagers/custom-buildsystem/tests/projects/simpleproject/.kdev4/simpleproject.kdev4 <span style="color: grey">(2c4bac5)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118194/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/05/23/3a7228b1-c22a-4f00-9ab7-3f4d12fc1107__kdevplatform.diff">kdevplatform.diff</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2014/05/23/4f199bb9-f4d0-4525-98f5-78e3d3e9829f__selectCompiler_image.png">selectCompiler_image.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>