<table><tr><td style="">mwolff requested changes to this revision.<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/D17932">View Revision</a></tr></table><br /><div><div><p>Hey there, sorry for the long delay. In general, I think your suggestions are very sane - most notably preferring exact case matches over fuzzy matches is a good thing to have!</p>
<p>But I wonder: do we really need to make this configurable? Can't we just always do this? I.e. can you explain which models would have an insensitive exact match, and which ones have sensitive exact matches?</p>
<p>Can you please submit the next patch iteration with context (I suggest you use <tt style="background: #ebebeb; font-size: 13px;">arc diff</tt> for that)</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/D17932#inline-99995">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katecompletionmodel.cpp:1554</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(251, 175, 175, .7);"> <span style="color: #aa4000">if</span><span class="p">(</span><span class="n">m_unimportant</span> <span style="color: #aa2211">&&</span> <span style="color: #aa2211">!</span><span class="n">rhs</span><span class="p">.</span><span class="n">m_unimportant</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 class="bright"></span></span><span class="bright"> </span><span class="p">(</span><span class="n">m_unimportant</span> <span style="color: #aa2211">&&</span> <span style="color: #aa2211">!</span><span class="n">rhs</span><span class="p">.</span><span class="n">m_unimportant</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; "> <span style="color: #aa4000">return</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;">unrelated whitespace changes should be fixed in separate commits</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/D17932#inline-99999">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katecompletionmodel.cpp:1575</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">if</span><span class="p">(</span> <span class="n">thisStartWithFilter</span> <span style="color: #aa2211">&&</span> <span style="color: #aa2211">!</span> <span class="n">rhsStartsWithFilter</span> <span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</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;">here and below:remove space after <tt style="background: #ebebeb; font-size: 13px;">!</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/D17932#inline-99996">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katecompletionmodel.cpp:1888</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 class="n">QChar</span> <span class="n">toLowerIfInsensitive</span><span class="p">(</span><span class="n">QChar</span> <span class="n">c</span><span class="p">,</span> <span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseSensitivity</span> <span class="n">caseSensitive</span><span class="p">)</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"><span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">static or anon namespace</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/D17932#inline-99998">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katecompletionmodel.cpp:2026</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">if</span> <span class="p">(</span><span class="n">matchCompletion</span> <span style="color: #aa2211">&&</span> <span class="n">match</span><span class="p">.</span><span class="n">length</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">m_nameColumn</span><span class="p">.</span><span class="n">length</span><span class="p">())</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">model</span><span style="color: #aa2211">-></span><span class="n">matchCaseSensitivity</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseInsensitive</span> <span style="color: #aa2211">&&</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">maybe simplify this to:</p>
<p style="padding: 0; margin: 8px;">if (matchCompletion && m_nameColumn.startsWith(match, model->exactMatchCaseSensitivity())) {</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);">matchCompletion = PerfectMatch;
m_haveExactMatch = true;</pre></div>
<p style="padding: 0; margin: 8px;">}</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/D17932#inline-99997">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katecompletionmodel.cpp:2029</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 class="n">model</span><span style="color: #aa2211">-></span><span class="n">exactMatchCaseSensitivity</span><span class="p">()</span> <span style="color: #aa2211">==</span> <span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseSensitive</span> <span style="color: #aa2211">&&</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa2211">!</span> <span class="n">m_nameColumn</span><span class="p">.</span><span class="n">startsWith</span><span class="p">(</span><span class="n">match</span><span class="p">,</span> <span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseSensitive</span><span class="p">))</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span style="color: #aa4000">return</span> <span class="n">matchCompletion</span><span class="p">;</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">remove whitespace after !</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/D17932#inline-100000">View Inline</a><span style="color: #4b4d51; font-weight: bold;">katecompletionmodel.h:394</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 class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseSensitivity</span> <span class="n">m_matchCaseSensitivity</span> <span style="color: #aa2211">=</span> <span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseInsensitive</span><span class="p">;</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseSensitivity</span> <span class="n">m_exactMatchCaseSensitivity</span> <span style="color: #aa2211">=</span> <span class="n">Qt</span><span style="color: #aa2211">::</span><span class="n">CaseInsensitive</span><span class="p">;</span> <span style="color: #74777d">// Must be equal to or stricter than m_matchCaseSensitivity.</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">should this comment be asserted in the setters or is it handled gracefully in the logic below?</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R39 KTextEditor</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D17932">https://phabricator.kde.org/D17932</a></div></div><br /><div><strong>To: </strong>thomassc, KTextEditor, KDevelop, mwolff<br /><strong>Cc: </strong>mwolff, cullmann, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, head7, kfunk, sars, dhaumann<br /></div>