<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/118718/">https://git.reviewboard.kde.org/r/118718/</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;">Woha, great work on the compiler support front - much appreciated. Sorry for my long delay in reviewing this.
There are some minor style issues, but also some fundamental design questions to be solved. In general, this looks solid though - good work!</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/118718/diff/2/?file=280793#file280793line31" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.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">31</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="s">"Clang"</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;">could you please add a comment here in the form:
// TODO kf5: use QStringLiteral
thanks</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/118718/diff/2/?file=280794#file280794line56" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h</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; ">public :</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">56</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">bool</span> <span class="n">setCompiler</span><span class="p">(</span> <span class="n">KDevelop</span><span class="o">::</span><span class="n">IProject</span><span class="o">*</span> <span class="n">project</span><span class="p">,</span> <span class="k"><span class="hl">const</span></span><span class="hl"> </span><span class="n"><span class="hl">QString</span></span><span class="o"><span class="hl">&</span></span><span class="hl"> </span><span class="n"><span class="hl">name</span></span><span class="p"><span class="hl">,</span></span> <span class="k">co<span class="hl">nst</span></span><span class="hl"> </span><span class="n"><span class="hl">QString</span></span><span class="o"><span class="hl">&</span></span><span class="hl"> </span><span class="n"><span class="hl">path</span></span> <span class="p">)</span> <span class="n">override</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">56</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">bool</span> <span class="n">setCompiler</span><span class="p">(</span> <span class="n">KDevelop</span><span class="o">::</span><span class="n">IProject</span><span class="o">*</span> <span class="n">project</span><span class="p">,</span> <span class="n"><span class="hl">CompilerPointer</span></span> <span class="n">co<span class="hl">mpiler</span></span> <span class="p">)</span> <span class="n">override</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;">const& the compilerpointer?</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/118718/diff/2/?file=280794#file280794line62" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h</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; ">public :</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">62</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="nl">private:</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">62</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">bool</span> <span class="n">registerCompiler</span><span class="p">(</span><span class="n">CompilerPointer</span> <span class="n">compiler</span><span class="p">)</span> <span class="n">override</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;">same here and below</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/118718/diff/2/?file=280795#file280795line49" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.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; ">using namespace KDevelop;</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">48</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="p">{</span> <span class="n">m_name</span> <span class="o">=</span> <span class="n">name</span><span class="p">,</span> <span class="n">m_path</span> <span class="o">=</span> <span class="n">path</span><span class="p">,</span> <span class="n">m_editable</span> <span class="o">=</span> <span class="n">editable</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;">use a member initializer list please
DummyCompiler(...)
: m_name(name)
, ...
{
}</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/118718/diff/2/?file=280795#file280795line64" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp</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">59</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"None"</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">63</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">virtual</span> <span class="n">CompilerPointer</span> <span class="n">createCompiler</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span><span class="o">&</span><span class="p">,</span> <span class="k">const</span> <span class="n">QString</span><span class="o">&</span><span class="p">,</span> <span class="kt">bool</span> <span class="o">=</span> <span class="nb">true</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;">brace on new line, also below</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/118718/diff/2/?file=280795#file280795line160" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp</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">154</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">auto</span> <span class="n">name</span> <span class="o">=</span> <span class="n">settings</span><span class="o">-></span><span class="n">currentCompiler</span><span class="p">(</span> <span class="n">projectConfig</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">156</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">auto</span> <span class="n">name</span> <span class="o">=</span> <span class="n">compiler</span> <span class="o">?</span> <span class="n">compiler</span><span class="o">-></span><span class="n">name</span><span class="p">()</span> <span class="o">:</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 />
<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/118718/diff/2/?file=280795#file280795line163" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp</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">157</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">compiler</span><span class="p"><span class="hl">.</span></span><span class="n"><span class="hl">compiler</span></span> <span class="o">&&</span> <span class="p">(</span> <span class="n"><span class="hl">compiler</span></span><span class="p"><span class="hl">.</span></span><span class="n">compiler</span><span class="o">-></span><span class="n">name</span><span class="p">()</span> <span class="o">!=</span> <span class="n">name</span> <span class="p">)</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">159</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">compiler</span> <span class="o">&&</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="o">!=</span> <span class="n">name</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;">the parens are not required 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/118718/diff/2/?file=280795#file280795line253" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.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">247</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setCompiler</span><span class="p">(</span><span class="n">it</span><span class="p">.</span><span class="n">key</span><span class="p">(),</span> <span class="n">m_factories</span><span class="p">.</span><span class="n">last</span><span class="p">()</span><span class="o">-></span><span class="n">createCompiler</span><span class="p">(</span><span class="s">""</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;">that looks odd - is this to generate some default compiler? then please add a new function for that with a proper name. This pattern is used in a few places in this patch and its always strange and I wonder what it should do :)
generally, some more comments would help so that one directly sees what a given loop will do. for this one e.g. something like "reset project compiler to default"
why is the last factory being used, btw? shouldn't the factory of the compiler that is unregistered be used?</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/118718/diff/2/?file=280798#file280798line45" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/icompiler.h</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">45</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">/// @return user visible name</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">45</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">void</span> <span class="nf">setPath</span><span class="p">(</span> <span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">path</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;">please don't inline stuff
also, don't make these functions virtual, you provide the proper implementation for these things after all.
</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/118718/diff/2/?file=280799#file280799line36" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/compilerprovider/icompilerfactory.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">36</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="n">CompilerPointer</span> <span class="n">createCompiler</span><span class="p">(</span> <span class="k">const</span> <span class="n">QString</span><span class="o">&</span> <span class="n">name</span><span class="p">,</span> <span class="k">const</span> <span class="n">QString</span><span class="o">&</span> <span class="n">path</span><span class="p">,</span> <span class="kt">bool</span> <span class="n">editable</span> <span class="o">=</span> <span class="nb">true</span> <span class="p">)</span> <span class="o">=</span> <span class="mi">0</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;">please add to the documentation, that editable should be set to false for default compilers detected in the path
</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/118718/diff/2/?file=280800#file280800line56" 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">56</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">/// @return true on success</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 document on why and when would this could fail?</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/118718/diff/2/?file=280800#file280800line57" 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">57</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">virtual</span> <span class="kt">bool</span> <span class="n">registerCompiler</span><span class="p">(</span><span class="n">CompilerPointer</span> <span class="n">compiler</span><span class="p">)</span> <span class="o">=</span> <span class="mi">0</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;">const&, also below</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/118718/diff/2/?file=280804#file280804line40" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/kcm_widget/compilersmodel.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">40</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">addCompiler</span><span class="p">(</span><span class="n">CompilerPointer</span> <span class="n">compiler</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;">const&</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/118718/diff/2/?file=280805#file280805line47" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/kcm_widget/compilersmodel.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">47</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">case</span> <span class="mi">0</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;">a good pattern to use in model implementations, is to define an
enum Columns {
NameColumn,
PathColumn,
NUM_COLUMNS
}
and use these enums inplace of the magic numbers
this greatly improves readability and makes it much simpler to extend the model</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/118718/diff/2/?file=280805#file280805line171" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/kcm_widget/compilersmodel.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">171</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">setCompilers</span><span class="p">(</span><span class="n">cmplrs</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 is pretty inefficient just to add a single compiler, no? just use beginInsertRow and insert the compiler at the end, should be just as easy and more efficient?</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/118718/diff/2/?file=280807#file280807line68" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.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">68</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">factory</span><span class="o">-></span><span class="n">name</span><span class="p">().</span><span class="n">isEmpty</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;">this should never happen, or?</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/118718/diff/2/?file=280812#file280812line278" style="color: black; font-weight: bold; text-decoration: underline;">languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.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 ProjectPathsWidget::setCurrentCompiler(const QString& name)</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">278</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">auto</span> <span class="n">c</span><span class="o">:</span> <span class="n">cw</span><span class="p">.</span><span class="n">compilers</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;">this and the loop below essentially does a semantic compilerProvider->setCompilers(cw.compilers())
and that's how I would expect it to be called from 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/118718/diff/2/?file=280814#file280814line46" 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 style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">44</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">/// @return name of the selected compiler, empty string if none is selected.</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">46</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">void</span> <span class="nf">writeCurrentCompiler</span><span class="p">(</span><span class="n">KConfig</span><span class="o">*</span> <span class="n">cfg</span><span class="p">,</span> <span class="n">CompilerPointer</span> <span class="n">compiler</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;">const& the ptr</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/118718/diff/2/?file=280815#file280815line181" 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 style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QList<ConfigEntry> SettingsManager::readPaths( KConfig* cfg ) const</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">181</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">auto</span> <span class="n">editable</span> <span class="o">=</span> <span class="n">grp</span><span class="p">.</span><span class="n">readEntry</span><span class="p">(</span> <span class="s">"compilerEditable"</span><span class="p">,</span> <span class="nb">true</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;">why do we serialize this? imo, we should not and instead re-detect at runtime whether system compilers still exist or not. maybe their path changes etc. and we would not detect that then.
so only save editable stuff, i.e. compilers the user added manually</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/118718/diff/2/?file=280815#file280815line226" 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 style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">QList<ConfigEntry> SettingsManager::readPaths( KConfig* cfg ) const</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">226</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">grp</span><span class="p">.</span><span class="n">writeEntry</span><span class="p">(</span><span class="s">"compilerName"</span><span class="p">,</span> <span class="n">compiler</span><span class="o">-></span><span class="n">name</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 compiler prefix here and below is not required, is it?
[Compilers]
[Compilers][1]
name = ...
path = ...
type = ...
[Compilers][2]
...
would suffice, imo</pre>
</div>
<br />
<p>- Milian Wolff</p>
<br />
<p>On June 13th, 2014, 7:37 a.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 June 13, 2014, 7:37 a.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;">Now there are compilers that auto-detected and those that added manually. The latter exist globally and can be used for any project.</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/compilerprovider/CMakeLists.txt <span style="color: grey">(45cccbc)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/compilerfactories.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.h <span style="color: grey">(8ab52ab)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/compilerprovider.cpp <span style="color: grey">(6483995)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.h <span style="color: grey">(b96df3b)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/gcclikecompiler.cpp <span style="color: grey">(97262ec)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/icompiler.h <span style="color: grey">(77ceb75)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/icompilerfactory.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/icompilerprovider.h <span style="color: grey">(21bfc05)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/msvccompiler.h <span style="color: grey">(e0ee668)</span></li>
<li>languages/plugins/custom-definesandincludes/compilerprovider/msvccompiler.cpp <span style="color: grey">(1c6b6c5)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/CMakeLists.txt <span style="color: grey">(ea0fe01)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/compilersmodel.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/compilersmodel.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/compilerswidget.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/customdefinesandincludes.kcfg <span style="color: grey">(9acb2e0)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/kcm_customdefinesandincludes.cpp <span style="color: grey">(c09b00e)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.h <span style="color: grey">(3618170)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.cpp <span style="color: grey">(c3a0823)</span></li>
<li>languages/plugins/custom-definesandincludes/kcm_widget/projectpathswidget.ui <span style="color: grey">(9831b94)</span></li>
<li>languages/plugins/custom-definesandincludes/settingsmanager.h <span style="color: grey">(f215268)</span></li>
<li>languages/plugins/custom-definesandincludes/settingsmanager.cpp <span style="color: grey">(fb7fd0f)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/118718/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/06/13/613c5d8c-6425-42ad-b0e4-1ccc2384a838__addCompiler.png">addCompiler.png</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>