<table><tr><td style="">ematirov added a comment.
</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/D3580" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>Thanks! Just some more nitpicks.</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/D3580#inline-14104" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">cvsproxy.cpp:188</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: #ffd0d0;">    <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 class="p"><span class="bright">)</span></span><span class="bright"> </span><span style="color: #aa4000">delete</span> <span class="n">job</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span style="color: #aa4000">delete</span> <span class="n">job</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">    <span style="color: #aa4000">return</span> <span style="color: #aa4000">nullptr</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">There are more such cases in cvsproxy.cpp. (Them are listed in file with warnings). Could you fix them too 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/D3580#inline-14103" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">expandingdelegate.cpp:134</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="n">s</span><span class="p">.</span><span class="n">setHeight</span><span class="p">(</span> <span class="n">widgetSize</span><span class="p">.</span><span class="n">height</span><span class="p">()</span> <span style="color: #aa2211">+</span> <span class="n">s</span><span class="p">.</span><span class="n">height</span><span class="p">()</span> <span style="color: #aa2211">+</span> <span style="color: #601200">10</span> <span class="p">);</span> <span style="color: #74777d">//10 is the sum that must match exactly the offsets used in ExpandingWidgetModel::placeExpandingWidgets</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">  <span class="p">}</span> <span style="color: #aa4000">else</span> <span style="color: #aa4000">if</span><span class="p">(</span> <span class="n">model</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">isPartiallyExpanded</span><span class="p">(</span> <span class="n">index</span> <span class="p">)</span> <span class="p">)</span> <span class="p">{</span>
</div><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">model</span><span class="p">()</span><span style="color: #aa2211">-></span><span class="n">isPartiallyExpanded</span><span class="p">(</span> <span class="n">index</span> <span class="p">)</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="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; ">    <span class="n">s</span><span class="p">.</span><span class="n">setHeight</span><span class="p">(</span> <span class="n">s</span><span class="p">.</span><span class="n">height</span><span class="p">()</span> <span style="color: #aa2211">+</span> <span style="color: #601200">30</span> <span style="color: #aa2211">+</span> <span style="color: #601200">10</span> <span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please use "NotExpandable" there (and in other cases) instead of constant there since it's enum. The main idea of problem there is that order of values in enum can be changed, so NotExpandable will become no-zero and all these ifs will not work as expected.</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/D3580#inline-14107" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">testview.cpp:243</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">foreach</span></span><span class="bright"> </span><span class="p"><span class="bright">(</span></span><span class="n">QStandardItem</span><span style="color: #aa2211">*<span class="bright"></span></span><span class="bright"> </span><span class="n">item<span class="bright"></span></span><span class="bright"></span><span class="p"><span class="bright">,</span></span> <span class="n">m_model</span><span style="color: #aa2211">-></span><span class="n">findItems</span><span class="p">(</span><span class="n">project</span><span style="color: #aa2211">-></span><span class="n">name</span><span class="p">())<span class="bright">)</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">    <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #ffd0d0;">        <span style="color: #aa4000">return</span> <span class="n">item</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">    <span class="bright"></span><span class="n"><span class="bright">QList</span></span><span class="bright"></span><span style="color: #aa2211"><span class="bright"><</span></span><span class="n">QStandardItem</span><span style="color: #aa2211">*<span class="bright">></span></span><span class="bright"> </span><span class="n"><span class="bright">std</span>item<span class="bright">list</span></span><span class="bright"> </span><span style="color: #aa2211"><span class="bright">=</span></span> <span class="n">m_model</span><span style="color: #aa2211">-></span><span class="n">findItems</span><span class="p">(</span><span class="n">project</span><span style="color: #aa2211">-></span><span class="n">name</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 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">stditemlist</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">size</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>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">        <span style="color: #aa4000">return</span> <span class="bright"></span><span class="n"><span class="bright">std</span>item<span class="bright">list</span></span><span class="bright"></span><span class="p"><span class="bright">.</span></span><span class="bright"></span><span class="n"><span class="bright">at</span></span><span class="bright"></span><span class="p"><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>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Let's use just !stditemlist.IsEmpty() and stditemlist.first().<br />
And probably "itemsForProject" or something like so will be more meaningful name for that. ;)</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R33 KDevPlatform</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D3580" rel="noreferrer">https://phabricator.kde.org/D3580</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>spencerb, KDevelop, kfunk, ematirov<br /><strong>Cc: </strong>kdevelop-devel<br /></div>