<table><tr><td style="">SGOrava 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/D27791">View Revision</a></tr></table><br /><div><div><p>Thank you for your contribution.</p>

<p>Our Gitlab instance was moved just recently (few days ago).</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/D27791#inline-157119">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pluginsmanager.cpp:55</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">connect</span><span class="p">(</span><span class="n">mApp</span><span style="color: #aa2211">-></span><span class="n">plugins</span><span class="p">(),</span> <span style="color: #aa2211">&</span><span class="n">Plugins</span><span style="color: #aa2211">::</span><span class="n">availablePluginsChanged</span><span class="p">,</span> <span style="color: #aa4000">this</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">PluginsManager</span><span style="color: #aa2211">::</span><span class="n">refresh</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><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">ui</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">search</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">QLineEdit</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">textChanged</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">PluginsManager</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">addFilter</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; ">    <span class="n">ui</span><span style="color: #aa2211">-></span><span class="n">list</span><span style="color: #aa2211">-></span><span class="n">setItemDelegate</span><span class="p">(</span><span style="color: #aa4000">new</span> <span class="n">PluginListDelegate</span><span class="p">(</span><span class="n">ui</span><span style="color: #aa2211">-></span><span class="n">list</span><span class="p">));</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">same here, I would keep a blank line after connect commands.</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/D27791#inline-157117">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pluginsmanager.cpp:173</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: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; "><span style="color: #aa4000">void</span> <span class="n">PluginsManager</span><span style="color: #aa2211">::</span><span class="n">itemChanged</span><span class="p">(</span><span class="n">QListWidgetItem</span><span style="color: #aa2211">*</span> <span class="n">item</span><span class="p">)</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Please do not delete the blank line, or lines which you do not WANT to delete.</p>

<p style="padding: 0; margin: 8px;">It is good to keep unrelated code intact.</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/D27791#inline-157116">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pluginsmanager.cpp:246</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: rgba(151, 234, 151, .6);">       <span style="color: #aa4000">const</span> <span class="n">QString</span><span style="color: #aa2211">&</span> <span class="n">pluginName</span> <span style="color: #aa2211">=</span> <span class="n">ui</span><span style="color: #aa2211">-></span><span class="n">list</span><span style="color: #aa2211">-></span><span class="n">item</span><span class="p">(</span><span class="n">i</span><span class="p">)</span><span style="color: #aa2211">-></span><span class="n">text</span><span class="p">();</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">        <span style="color: #aa4000">if</span> <span class="p">(</span><span class="n">pluginName</span><span class="p">.</span><span class="n">startsWith</span><span class="p">(</span><span class="n">filter</span><span class="p">,</span><span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseInsensitive</span><span class="p">)</span> <span style="color: #aa2211">||</span> <span style="color: #aa2211">!</span><span class="n">filter</span><span class="p">.</span><span class="n">size</span><span class="p">())</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);">            <span class="n">ui</span><span style="color: #aa2211">-></span><span class="n">list</span><span style="color: #aa2211">-></span><span class="n">item</span><span class="p">(</span><span class="n">i</span><span class="p">)</span><span style="color: #aa2211">-></span><span class="n">setHidden</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;">This checks only if the plugin name starts with given string which is insufficient for proper search.</p>

<p style="padding: 0; margin: 8px;">Using QString::contains() fits the criteria of "search" much more.</p>

<p style="padding: 0; margin: 8px;">Plus Always use {} for commands after the conditions.</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/D27791#inline-157118">View Inline</a><span style="color: #4b4d51; font-weight: bold;">pluginsmanager.h:47</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 style="color: #aa4000">void</span> <span style="color: #004012">itemChanged</span><span class="p">(</span><span class="n">QListWidgetItem</span><span style="color: #aa2211">*</span> <span class="n">item</span><span class="p">);</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(251, 175, 175, .7);">
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="bright">    </span><span style="color: #aa4000"><span class="bright">void</span></span><span class="bright"> </span><span style="color: #004012"><span class="bright">addFilter</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">const</span></span><span class="bright"> </span><span class="n"><span class="bright">QString</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">newText</span></span><span class="bright"></span><span class="p"><span class="bright">);</span></span>
</div><div style="padding: 0 8px; margin: 0 4px; ">    <span style="color: #aa4000">void</span> <span style="color: #004012">refresh</span><span class="p">();</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">In headers methods are sometimes divided by blank lines to indicate that they do different kind of job than the previous "group" of methods, it is good to keep those blanks and find a better place for the new entry (this is sometimes a challenge but it is worth it because it makes code more readable (at least for me))</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R875 Falkon</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D27791">https://phabricator.kde.org/D27791</a></div></div><br /><div><strong>To: </strong>malibushko, SGOrava<br /><strong>Cc: </strong>falkon, grune, akilgus, siraj_qazi, saishm, anmolgautam, SGOrava, iodelay, spoorun, ptabis, navarromorales, clivej, mparillo<br /></div>