<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 />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 19th, 2014, 1:47 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/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="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>
</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;">>that looks odd - is this to generate some default compiler?
You mean createCompiler("", "")? It creates a compiler with an empty name and path. This is used in 2 situations: when user adds a new compiler (he'll set the name and the path afterward) and when we create the DummyCompiler that knows it's name.
> then please add a new function for that with a proper name
Why? The code imo is already very straightforward... Anyhow, what exactly this function should look like?
>why is the last factory being used, btw?
Because in that implementation the last factory was the the DummyFactory. But now I use a proper function to retrieve it.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 19th, 2014, 1:47 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/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="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>
</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;">This happens with the DummyFactory</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 19th, 2014, 1:47 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/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="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>
</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;">Sadly, but the ICompilerProvider interface doesn't have the setCompilers method, and I'm reluctand to add it, as I don't want to pollute the interface with rarely used methods that can be implemented with register/unregister calls instead</pre>
<br />
<p>- Sergey</p>
<br />
<p>On June 13th, 2014, 11:37 a.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 June 13, 2014, 11: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>