D17932: Improvements to completion

[Milian Wolff]

mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.


  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!
  
  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?
  
  Can you please submit the next patch iteration with context (I suggest you use `arc diff` for that)

INLINE COMMENTS

> katecompletionmodel.cpp:1554
>  
> -    if(m_unimportant && !rhs.m_unimportant){
> +    if (m_unimportant && !rhs.m_unimportant) {
>          return false;

unrelated whitespace changes should be fixed in separate commits

> katecompletionmodel.cpp:1575
> +        
> +        if( thisStartWithFilter && ! rhsStartsWithFilter ) {
> +            return true;

here and below:remove space after `!`

> katecompletionmodel.cpp:1888
>  
> +QChar toLowerIfInsensitive(QChar c, Qt::CaseSensitivity caseSensitive) 
> +{

static or anon namespace

> katecompletionmodel.cpp:2026
>  
>      if (matchCompletion && match.length() == m_nameColumn.length()) {
> +        if (model->matchCaseSensitivity() == Qt::CaseInsensitive &&

maybe simplify this to:

if (matchCompletion && m_nameColumn.startsWith(match, model->exactMatchCaseSensitivity())) {

  matchCompletion = PerfectMatch;
  m_haveExactMatch = true;

}

> katecompletionmodel.cpp:2029
> +            model->exactMatchCaseSensitivity() == Qt::CaseSensitive &&
> +            ! m_nameColumn.startsWith(match, Qt::CaseSensitive)) {
> +            return matchCompletion;

remove whitespace after !

> katecompletionmodel.h:394
> +    Qt::CaseSensitivity m_matchCaseSensitivity = Qt::CaseInsensitive;
> +    Qt::CaseSensitivity m_exactMatchCaseSensitivity = Qt::CaseInsensitive;  // Must be equal to or stricter than m_matchCaseSensitivity.
> +    

should this comment be asserted in the setters or is it handled gracefully in the logic below?

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D17932

To: thomassc, #ktexteditor, #kdevelop, mwolff
Cc: mwolff, cullmann, kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, head7, kfunk, sars, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20190110/04e0ec7d/attachment.html>