With auto completion don't show completions that don't match from beginning of typed word
Needs RevisionPublic

Authored by ahmadsamir on Jul 15 2019, 4:46 PM.

Details

Summary

When invoking auto completion for a word, don't show the completions
menu if the available matches don't match the beginning of the word
typed by the user.

Previously if no matches were found, the completions menu was invoked
if there were completion items that "contained" the matched word (not
at the beginning of the matches), if that word appeared at a word
beginning, which was marked by a capital letter or an underscore, e.g.
"Foo" would match "BarFoo" and "Bar_Foo". The user can still maunally
invoke the menu.

Use range for instead of foreach.

BUG: 381024
FIXED-IN: 5.61.0

Test Plan
  • Open a document in kate and type the following:

Foo
BarFoo
Bar_Foo

  • Type: foo
  • Check that the auto completions menu isn't shown
  • Check that manually invoking the menu, shows the completions that "contain" the typed word.

Diff Detail

Repository
R39 KTextEditor
Branch
less-trigger-happy-completions (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14027
Build 14045: arc lint + arc unit
ahmadsamir created this revision.Jul 15 2019, 4:46 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptJul 15 2019, 4:46 PM
ahmadsamir requested review of this revision.Jul 15 2019, 4:46 PM

Given KDevelop uses the completion a lot more extensive than Kate, I would like to have some input from there, too ;=)

kossebau added a comment.EditedJul 15 2019, 7:29 PM

Default is false, so does not change behaviour unless someone toggles the switch, right?

I would rename the option to WordCompletionMatchFromWordStartOnly though, as from what I can tell completion often only does from start, and matching also in the word is an extra feature. Edit: as in, reading WordCompletionMatchFromWordStartin the code triggered with me "yes, what else? MatchFromEnd?"

Actually, perhaps that whole option could/should be even an enum for 3 cases:
a) from full word start
b) from subword start (CamelCase, _)
c) match anywhere in word.
I vaguely remember some cases where I got annoyed by cross-subword hits accidentally creating a match.

Default is false, so does not change behaviour unless someone toggles the switch, right?

I would rename the option to WordCompletionMatchFromWordStartOnly though, as from what I can tell completion often only does from start, and matching also in the word is an extra feature. Edit: as in, reading WordCompletionMatchFromWordStartin the code triggered with me "yes, what else? MatchFromEnd?"

Noted. Thought I most likely didn't add "only" because the config name was becoming rather too long, but you have a point.

Actually, perhaps that whole option could/should be even an enum for 3 cases:
a) from full word start
b) from subword start (CamelCase, _)
c) match anywhere in word.
I vaguely remember some cases where I got annoyed by cross-subword hits accidentally creating a match.

a and b are already in the MatchType enum, StartsWithMatch and ContainsMatch respectively, the new config is overriding the current behaviour where if there's no StartsWithMatch, it goes to the ContainsMatch route. As for c, I think that would be too many completions, the accuracy would be blown to bits...

brauch added a subscriber: brauch.Jul 16 2019, 9:32 AM

I'm not sure if this solves the right problem.

Where I notice this issue a lot is when typing "return". The keyword completion suggests "return", and when I want a newline, I complete the "return" first. This doesn't affect C++ (because of the trailing ";"), but it does affect other languages.

I think this is pretty much the same issue. Maybe it should be solved by hiding the completion window if it was not explicitly invoked if there is any entry matching what you currently typed?

Then typing Foo would not have the completion window open at all in the described case (thus solving your problem, as well as mine), and if you still wanted to complete BarFoo, you would press Ctrl+Space.

Change the diff altogether to not show the menu with "ContainsMatch"

ahmadsamir updated this revision to Diff 61881.EditedJul 16 2019, 10:24 PM
ahmadsamir retitled this revision from Add a config to only show completions matching word beginning to With auto completion don't show completions that don't match from beginning of typed word.
ahmadsamir edited the summary of this revision. (Show Details)
ahmadsamir edited the test plan for this revision. (Show Details)

Verbatim

ahmadsamir edited the summary of this revision. (Show Details)

Take 2

I'm not sure if this solves the right problem.

Where I notice this issue a lot is when typing "return". The keyword completion suggests "return", and when I want a newline, I complete the "return" first. This doesn't affect C++ (because of the trailing ";"), but it does affect other languages.

I think this is pretty much the same issue. Maybe it should be solved by hiding the completion window if it was not explicitly invoked if there is any entry matching what you currently typed?

Then typing Foo would not have the completion window open at all in the described case (thus solving your problem, as well as mine), and if you still wanted to complete BarFoo, you would press Ctrl+Space.

I'm not sure if this solves the right problem.

Where I notice this issue a lot is when typing "return". The keyword completion suggests "return", and when I want a newline, I complete the "return" first. This doesn't affect C++ (because of the trailing ";"), but it does affect other languages.

I think this is pretty much the same issue. Maybe it should be solved by hiding the completion window if it was not explicitly invoked if there is any entry matching what you currently typed?

Great idea. Done.

Then typing Foo would not have the completion window open at all in the described case (thus solving your problem, as well as mine), and if you still wanted to complete BarFoo, you would press Ctrl+Space.

https://phabricator.kde.org/D22500

brauch added inline comments.Jul 16 2019, 10:31 PM
src/completion/katecompletionmodel.cpp
2029

Maybe you want to set the flag here too?

brauch added inline comments.Jul 16 2019, 10:33 PM
src/completion/katecompletionmodel.cpp
2029

Actually no, probably not. ;)

Also here, looks good to me, but I would wait for feedback from somebody else in addition. Thank you for working on this!

ahmadsamir marked 2 inline comments as done.Aug 3 2019, 7:16 PM

Ping.

Any feedback on that from others?

cullmann requested changes to this revision.Aug 25 2019, 7:44 PM

Actually, I did read the bug once more and tried it out myself.
I think we should not unconditionally remove that feature.
If you read the bug, one user complaints about it, another one likes this.
If at all, we should make it an option, default like ATM.

This revision now requires changes to proceed.Aug 25 2019, 7:44 PM

Unconditionally removing this is not a good idea, since the feature significantly helps to discover API via code completion.

In any case, feedback from @kfunk, @molff is needed here imho