<table><tr><td style="">mwolff requested changes to this revision.<br />mwolff added a reviewer: mwolff.<br />mwolff added a comment.<br />This revision now requires changes to proceed.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D4095" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>awesome work Aleix! I got a few nitpicks, but in general this looks really good already. I'd say clean it up a bit and then merge it?</p>
<p>The only things that may require some more discussion are the startup procedure for the server instead of relying on the 100ms delay, and whether the "serverSupported" flag is global or per-project (i.e. per-cmake binary)</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16837" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakemanager.cpp:142</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="n">Q_GLOBAL_STATIC_WITH_ARGS</span><span class="p">(</span><span style="color: #aa4000">bool</span><span class="p">,</span> <span class="n">s_serverSupported</span><span class="p">,</span> <span class="p">(</span><span style="color: #304a96">false</span><span class="p">))</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">for PODs, you don't need Q_GLOBAL_STATIC. simply make this a</p>
<p style="padding: 0; margin: 8px;">namespace {<br />
static bool s_serverSupported = false;<br />
}</p>
<p style="padding: 0; margin: 8px;">on a conceptual level - this depends on the cmake binary that can be selected for every project, no? So this should not be a static at all, but rather a per-project property?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16838" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakemanager.cpp:154</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">error</span><span class="p">()</span> <span style="color: #aa2211">!=</span> <span style="color: #601200">0</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">qCDebug</span><span class="p">(</span><span class="n">CMAKE</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"couldn't load successfully"</span> <span style="color: #aa2211"><<</span> <span class="n">job</span><span style="color: #aa2211">-></span><span class="n">project</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">name</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">m_projects</span><span class="p">.</span><span class="n">remove</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">project</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">make this a warning? also, do we want to show the user that? this can be serious and render the whole cmake support useless after all, right?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16839" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakemanager.cpp:174</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="bright"></span><span class="n"><span class="bright">CMakeImportJob</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">*</span></span><span class="bright"> </span><span class="n"><span class="bright">job</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">new</span></span><span class="bright"> </span><span class="n"><span class="bright">CMakeImportJob</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">project</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">this</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;"> <span class="bright"></span><span class="n"><span class="bright">connect</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">job</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">&</span></span><span class="bright"></span><span class="n"><span class="bright">CMakeImportJob</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">result</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #aa4000"><span class="bright">this</span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">&</span></span><span class="bright"></span><span class="n"><span class="bright">CMakeManager</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">::</span></span><span class="bright"></span><span class="n"><span class="bright">importFinished</span></span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="bright"> </span><span style="color: #aa4000"><span class="bright">if</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">job</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">error</span></span><span class="bright"></span><span class="p"><span class="bright">()</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">!=</span></span><span class="bright"> </span><span style="color: #601200"><span class="bright">0</span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span><span class="bright"> </span><span class="p"><span class="bright">{</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="bright"> </span><span class="n"><span class="bright">qCDebug</span></span><span class="bright"></span><span class="p"><span class="bright">(</span></span><span class="bright"></span><span class="n"><span class="bright">CMAKE</span></span><span class="bright"></span><span class="p"><span class="bright">)</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright"><<</span></span><span class="bright"> </span><span style="color: #766510"><span class="bright">"couldn't load successfully"</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright"><<</span></span><span class="bright"> </span><span class="n"><span class="bright">job</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">project</span></span><span class="bright"></span><span class="p"><span class="bright">()</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright">-></span></span><span class="bright"></span><span class="n"><span class="bright">name</span></span><span class="bright"></span><span class="p"><span class="bright">(</span>);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">m_projects</span><span class="p">.</span><span class="n">remove</span><span class="p">(</span><span class="n">job</span><span style="color: #aa2211">-></span><span class="n">project</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">warning, and see above? actually the whole code is duplicated - move this into a proper slot? or at least share the lambda body?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16842" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeprojectdata.cpp:3</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> * Copyright 2013 Aleix Pol <aleixpol@kde.org></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">the future is now! it's 2017 :P</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16844" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeprojectdata.cpp:31</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">,</span> <span class="n">targets</span><span class="p">(</span><span class="n">targets</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">,</span> <span class="n">watcher</span><span class="p">(</span><span style="color: #aa4000">new</span> <span class="n">QFileSystemWatcher</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">,</span> <span class="n">m_testSuites</span><span class="p">(</span><span class="n">tests</span><span class="p">)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">missing parent?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16843" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeprojectdata.cpp:33</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">,</span> <span class="n">m_testSuites</span><span class="p">(</span><span class="n">tests</span><span class="p">)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">empty line at end please</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16840" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeprojectdata.h:61</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; "><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">CMakeProjectData</span><span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">QHash</span><span style="color: #aa2211"><</span><span class="n">KDevelop</span><span style="color: #aa2211">::</span><span class="n">Path</span><span class="p">,</span> <span class="n">QStringList</span><span style="color: #aa2211">>&</span> <span class="n">targets</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">CMakeFilesCompilationData</span> <span style="color: #aa2211">&</span><span class="n">data</span><span class="p">,</span> <span style="color: #aa4000">const</span> <span class="n">QVector</span><span style="color: #aa2211"><</span><span class="n">Test</span><span style="color: #aa2211">></span> <span style="color: #aa2211">&</span><span class="n">tests</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">& next to type names</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16841" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeprojectdata.h:71</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QVector</span><span style="color: #aa2211"><</span><span class="n">Test</span><span style="color: #aa2211">></span> <span class="n">testSuites</span><span class="p">()</span> <span style="color: #aa4000">const</span> <span class="p">{</span> <span style="color: #aa4000">return</span> <span class="n">m_testSuites</span><span class="p">;</span> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">simply make it public like the stuff above and remove the getter?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16847" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:3</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> * Copyright 2016 Aleix Pol <aleixpol@kde.org></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">2017</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16849" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:24</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #304a96">#include</span> <span class="cpf">"cmakeutils.h"</span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #304a96">#include</span> <span class="cpf"><KRandom></span><span style="color: #304a96"></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #304a96">#include</span> <span class="cpf"><QJsonDocument></span><span style="color: #304a96"></span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">unused?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16848" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:38</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QTemporaryFile</span> <span class="n">file</span><span class="p">(</span><span style="color: #766510">"kdevelopcmake-"</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">file</span><span class="p">.</span><span class="n">open</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">keep it around, otherwise the file gets removed again, no? or is that OK?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16850" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:45</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">m_process</span><span class="p">.</span><span class="n">setProcessChannelMode</span><span class="p">(</span><span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">ForwardedChannels</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">m_process</span><span class="p">.</span><span class="n">start</span><span class="p">(</span><span class="n">CMake</span><span style="color: #aa2211">::</span><span class="n">findExecutable</span><span class="p">(),</span> <span class="p">{</span><span style="color: #766510">"-E"</span><span class="p">,</span> <span style="color: #766510">"server"</span><span class="p">,</span> <span style="color: #766510">"--experimental"</span><span class="p">,</span> <span style="color: #766510">"--pipe="</span> <span style="color: #aa2211">+</span> <span class="n">path</span><span class="p">});</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QStringLiteral, and I hope cmake supports "--pipe", "path" as two args? i.e. instead of <tt style="background: #ebebeb; font-size: 13px;">--pipe=path</tt>, can you pass <tt style="background: #ebebeb; font-size: 13px;">--pipe path</tt>?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16851" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:62</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QTimer</span><span style="color: #aa2211">*</span> <span class="n">connectTimer</span> <span style="color: #aa2211">=</span> <span style="color: #aa4000">new</span> <span class="n">QTimer</span><span class="p">(</span><span style="color: #aa4000">this</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">connectTimer</span><span style="color: #aa2211">-></span><span class="n">setInterval</span><span class="p">(</span><span style="color: #601200">100</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">connectTimer</span><span style="color: #aa2211">-></span><span class="n">setSingleShot</span><span class="p">(</span><span style="color: #304a96">true</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">can you add a comment saying why this is required? is there really no "good" way to wait for the server to start up? what is creator doing here? have you talked to thunger about this? looks really brittle</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16852" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:67</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">});</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">connect</span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">m_process</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">QProcess</span><span style="color: #aa2211">::</span><span class="n">started</span><span class="p">,</span> <span class="n">connectTimer</span><span class="p">,</span> <span style="color: #aa4000">static_cast</span><span style="color: #aa2211"><</span><span style="color: #aa4000">void</span><span class="p">(</span><span class="n">QTimer</span><span style="color: #aa2211">::*</span><span class="p">)()</span><span style="color: #aa2211">></span><span class="p">(</span><span style="color: #aa2211">&</span><span class="n">QTimer</span><span style="color: #aa2211">::</span><span class="n">start</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'd simplify the above a bit (imo) by doing something like:</p>
<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">connect(...started, this, [this, path] () {
// please document me
QTimer::singleShot(100, this, [this, path] () {
m_localSocket->connectToServer(path, QIODevice::ReadWrite);
});
});</pre></div></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16853" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:81</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #aa4000">static</span> <span style="color: #aa4000">const</span> <span class="n">QByteArray</span> <span class="n">openTag</span> <span style="color: #aa2211">=</span> <span style="color: #766510">"</span><span style="color: #bb6622">\n</span><span style="color: #766510">[== </span><span style="color: #bb6622">\"</span><span style="color: #766510">CMake Server</span><span style="color: #bb6622">\"</span><span style="color: #766510"> ==[</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #aa4000">static</span> <span style="color: #aa4000">const</span> <span class="n">QByteArray</span> <span class="n">closeTag</span> <span style="color: #aa2211">=</span> <span style="color: #766510">"</span><span style="color: #bb6622">\n</span><span style="color: #766510">]== </span><span style="color: #bb6622">\"</span><span style="color: #766510">CMake Server</span><span style="color: #bb6622">\"</span><span style="color: #766510"> ==]</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QByteArrayLiteral, wrap in functions returning QByteArray to get rid of global static initialization</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16855" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:90</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">auto</span> <span class="n">len</span> <span style="color: #aa2211">=</span> <span class="n">m_localSocket</span><span style="color: #aa2211">-></span><span class="n">write</span><span class="p">(</span><span class="n">data</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QTextStream</span><span class="p">(</span><span class="n">stdout</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"writing...</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span> <span style="color: #aa2211"><<</span> <span class="n">QJsonDocument</span><span class="p">(</span><span class="n">object</span><span class="p">).</span><span class="n">toJson</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="n">len</span><span style="color: #aa2211">></span><span style="color: #601200">0</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">qCDebug(...) << "writing..." << object;</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16854" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:91</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QTextStream</span><span class="p">(</span><span class="n">stdout</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"writing...</span><span style="color: #bb6622">\n</span><span style="color: #766510">"</span> <span style="color: #aa2211"><<</span> <span class="n">QJsonDocument</span><span class="p">(</span><span class="n">object</span><span class="p">).</span><span class="n">toJson</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="n">len</span><span style="color: #aa2211">></span><span style="color: #601200">0</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">spaces around operators</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16857" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:98</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">m_buffer</span> <span style="color: #aa2211">+=</span> <span class="n">m_localSocket</span><span style="color: #aa2211">-></span><span class="n">readAll</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">for</span><span class="p">(;</span> <span class="n">m_buffer</span><span class="p">.</span><span class="n">size</span><span class="p">()</span> <span style="color: #aa2211">></span> <span class="n">openTag</span><span class="p">.</span><span class="n">size</span><span class="p">();</span> <span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; padding: 12px; margin: 0; background: rgba(71, 87, 120, 0.08);">const auto openTag = ::openTag(); // once you added the getter
const auto oepnTagSize = openTag.size();
// use both below</pre></div></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16859" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:99</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">m_buffer</span> <span style="color: #aa2211">+=</span> <span class="n">m_localSocket</span><span style="color: #aa2211">-></span><span class="n">readAll</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">for</span><span class="p">(;</span> <span class="n">m_buffer</span><span class="p">.</span><span class="n">size</span><span class="p">()</span> <span style="color: #aa2211">></span> <span class="n">openTag</span><span class="p">.</span><span class="n">size</span><span class="p">();</span> <span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this can go into an infinite-loop, no?</p>
<p style="padding: 0; margin: 8px;">if m_buffer.size() == openTag.size() + 1, then we go into the loop, but we won't find the close tag and thus cannot ever get out of the loop</p>
<p style="padding: 0; margin: 8px;">see below</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16856" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:103</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">int</span> <span class="n">idx</span> <span style="color: #aa2211">=</span> <span class="n">m_buffer</span><span class="p">.</span><span class="n">indexOf</span><span class="p">(</span><span class="n">closeTag</span><span class="p">,</span> <span class="n">openTag</span><span class="p">.</span><span class="n">size</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">idx</span><span style="color: #aa2211">>=</span><span style="color: #601200">0</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">emitResponse</span><span class="p">(</span><span class="n">m_buffer</span><span class="p">.</span><span class="n">mid</span><span class="p">(</span><span class="n">openTag</span><span class="p">.</span><span class="n">size</span><span class="p">(),</span> <span class="n">idx</span> <span style="color: #aa2211">-</span> <span class="n">openTag</span><span class="p">.</span><span class="n">size</span><span class="p">()));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">spaces around operators</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16860" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:106</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">m_buffer</span> <span style="color: #aa2211">=</span> <span class="n">m_buffer</span><span class="p">.</span><span class="n">mid</span><span class="p">(</span><span class="n">idx</span> <span style="color: #aa2211">+</span> <span class="n">closeTag</span><span class="p">.</span><span class="n">size</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">}</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">else break?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16861" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:117</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">}</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">Q_ASSERT</span><span class="p">(</span><span style="color: #aa2211">!</span><span class="n">error</span><span class="p">.</span><span class="n">error</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">Q_ASSERT</span><span class="p">(</span><span class="n">doc</span><span class="p">.</span><span class="n">isObject</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">since we don't generate the response data, I wouldn't assert on it. if there's a cmake bug it would kill kdevelop...</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16862" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.cpp:128</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">sendCommand</span><span class="p">({</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">{</span><span style="color: #766510">"cookie"</span><span class="p">,</span> <span style="color: #766510">""</span><span class="p">},</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">{</span><span style="color: #766510">"type"</span><span class="p">,</span> <span style="color: #766510">"handshake"</span><span class="p">},</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">here and below: QStringLiteral</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16845" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.h:3</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> * Copyright 2016 Aleix Pol <aleixpol@kde.org></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">dito, 2017</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16846" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserver.h:29</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="n">class</span> <span class="n">KDEVCMAKECOMMON_EXPORT</span> <span style="color: #a0a000">CMakeServer</span> <span class="p">:</span> <span class="n">public</span> <span class="n">QObject</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this is actually a CMakeServerClient, right?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16867" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:3</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> * Copyright 2014 Kevin Funk <kfunk@kde.org></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">really?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16868" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:30</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #aa4000">template</span> <span style="color: #aa2211"><</span><span style="color: #aa4000">typename</span> <span class="n">T</span><span class="p">,</span> <span style="color: #aa4000">typename</span> <span class="n">Q</span><span class="p">,</span> <span style="color: #aa4000">typename</span> <span class="n">W</span><span style="color: #aa2211">></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #aa4000">static</span> <span class="n">T</span> <span class="n">kTransform</span><span class="p">(</span><span style="color: #aa4000">const</span> <span class="n">Q</span><span style="color: #aa2211">&</span> <span class="n">list</span><span class="p">,</span> <span class="n">W</span> <span class="n">func</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">you only use it with T == Path::List, so why not hardcode that?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16870" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:39</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d">///copied from the makefile resolver</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #aa4000">static</span> <span class="n">QRegularExpression</span> <span class="n">defineRegularExpression</span><span class="p">()</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">please add it as a getter to the MakefileResolver API, which is used elsewhere in CMake already. That way we don't duplicate this magic</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16869" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:43</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">static</span> <span style="color: #aa4000">const</span> <span class="n">QRegularExpression</span> <span class="n">pattern</span><span class="p">(</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #766510">"-D([^</span><span style="color: #bb6622">\\</span><span style="color: #766510">s=]+)(?:=(?:</span><span style="color: #bb6622">\"</span><span style="color: #766510">(.*?)(?<!</span><span style="color: #bb6622">\\\\</span><span style="color: #766510">)</span><span style="color: #bb6622">\"</span><span style="color: #766510">|([^</span><span style="color: #bb6622">\\</span><span style="color: #766510">s]*)))?"</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QStringLiteral</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16871" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:69</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QHash</span><span style="color: #aa2211"><</span><span class="n">QString</span><span class="p">,</span> <span class="n">QString</span><span style="color: #aa2211">></span> <span class="n">ret</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span><span style="color: #aa2211">&</span> <span class="n">defineRx</span> <span style="color: #aa2211">=</span> <span class="n">defineRegularExpression</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">auto</span> <span class="n">it</span> <span style="color: #aa2211">=</span> <span class="n">defineRx</span><span class="p">.</span><span class="n">globalMatch</span><span class="p">(</span><span class="n">compileFlags</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">this could probably be part of a helper function in the MakefileResolver (see above), since that does something like that already elsewhere, I guess?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16872" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:83</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">int</span> <span class="n">eqIdx</span> <span style="color: #aa2211">=</span> <span class="n">define</span><span class="p">.</span><span class="n">indexOf</span><span class="p">(</span><span class="n">QLatin1Char</span><span class="p">(</span><span style="color: #766510">'='</span><span class="p">));</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">eqIdx</span><span style="color: #aa2211"><</span><span style="color: #601200">0</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">ret</span><span class="p">[</span><span class="n">define</span><span class="p">]</span> <span style="color: #aa2211">=</span> <span class="n">QString</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">spaces around operators</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16875" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:146</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">connect</span><span class="p">(</span><span class="n">m_server</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">CMakeServer</span><span style="color: #aa2211">::</span><span class="n">response</span><span class="p">,</span> <span style="color: #aa4000">this</span><span class="p">,</span> <span class="p">[</span><span style="color: #aa4000">this</span><span class="p">](</span><span style="color: #aa4000">const</span> <span class="n">QJsonObject</span> <span style="color: #aa2211">&</span><span class="n">response</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">response</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span style="color: #766510">"type"</span><span class="p">)</span> <span style="color: #aa2211">==</span> <span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"reply"</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span> <span class="n">inReplyTo</span> <span style="color: #aa2211">=</span> <span class="n">response</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span style="color: #766510">"inReplyTo"</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QStringLiteral for the "type"</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16876" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:146</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">connect</span><span class="p">(</span><span class="n">m_server</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">CMakeServer</span><span style="color: #aa2211">::</span><span class="n">response</span><span class="p">,</span> <span style="color: #aa4000">this</span><span class="p">,</span> <span class="p">[</span><span style="color: #aa4000">this</span><span class="p">](</span><span style="color: #aa4000">const</span> <span class="n">QJsonObject</span> <span style="color: #aa2211">&</span><span class="n">response</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">response</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span style="color: #766510">"type"</span><span class="p">)</span> <span style="color: #aa2211">==</span> <span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"reply"</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span> <span class="n">inReplyTo</span> <span style="color: #aa2211">=</span> <span class="n">response</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span style="color: #766510">"inReplyTo"</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">introduce var for type, as it's used again below</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16873" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:147</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">response</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span style="color: #766510">"type"</span><span class="p">)</span> <span style="color: #aa2211">==</span> <span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"reply"</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">const</span> <span style="color: #aa4000">auto</span> <span class="n">inReplyTo</span> <span style="color: #aa2211">=</span> <span class="n">response</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span style="color: #766510">"inReplyTo"</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">qCDebug</span><span class="p">(</span><span class="n">CMAKE</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"replying..."</span> <span style="color: #aa2211"><<</span> <span class="n">inReplyTo</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QStringLiteral</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16874" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:149</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">qCDebug</span><span class="p">(</span><span class="n">CMAKE</span><span class="p">)</span> <span style="color: #aa2211"><<</span> <span style="color: #766510">"replying..."</span> <span style="color: #aa2211"><<</span> <span class="n">inReplyTo</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">inReplyTo</span> <span style="color: #aa2211">==</span> <span style="color: #766510">"handshake"</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">m_server</span><span style="color: #aa2211">-></span><span class="n">configure</span><span class="p">({});</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">QLatin1String</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16877" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.cpp:161</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="p">}</span> <span style="color: #aa4000">else</span> <span style="color: #aa4000">if</span><span class="p">(</span><span class="n">response</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span style="color: #766510">"type"</span><span class="p">)</span> <span style="color: #aa2211">==</span> <span class="n">QLatin1String</span><span class="p">(</span><span style="color: #766510">"error"</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">setError</span><span class="p">(</span><span style="color: #601200">2</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">setErrorText</span><span class="p">(</span><span class="n">response</span><span class="p">.</span><span class="n">value</span><span class="p">(</span><span style="color: #766510">"errorMessage"</span><span class="p">).</span><span class="n">toString</span><span class="p">());</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">what's this magic number? is there no enum for this?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16864" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.h:2-3</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d">/* KDevelop CMake Support</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> * Copyright 2014 Kevin Funk <kfunk@kde.org></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">really?</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16866" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cmakeserverimportjob.h:34</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span class="n">Q_OBJECT</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #a0a000">public</span><span class="p">:</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">indent</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16878" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">test_cmakemanager.cpp:283</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"> <span class="n">QEXPECT_FAIL</span><span class="p">(</span><span style="color: #766510">""</span><span class="p">,</span> <span style="color: #766510">"Will fix soon, hopefully"</span><span class="p">,</span> <span class="n">Abort</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; "> <span class="n">ProjectTargetItem</span> <span style="color: #aa2211">*</span><span class="n">target</span> <span style="color: #aa2211">=</span> <span class="n">targets</span><span class="p">.</span><span class="n">first</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">famous last words :D</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D4095#inline-16879" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">test_cmakeserver.cpp:3</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> * Copyright 2016 Aleix Pol Gonzalez <aleixpol@kde.org></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;"><span style="color: #74777d"> *</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">2017</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R32 KDevelop</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D4095" rel="noreferrer">https://phabricator.kde.org/D4095</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>apol, kfunk, KDevelop, mwolff<br /><strong>Cc: </strong>mwolff, antonanikin, arrowdodger, kfunk, kdevelop-devel<br /></div>