-
-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Rewrite word completion to play nice with other completion providers. #1125
Rewrite word completion to play nice with other completion providers. #1125
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. I'll merge it together with the snippets branch into my "merge-dev-fixes" branch and dogfood it for a day or two.
There are a couple of other things that could be improved in the original but they are not related to this issue so I'll submit separate PRs for those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One issue I noticed that this PR introduces is that in some cases a prefix matches both a word and the same word followed by a space.
On a blank line type a (new) word
On the next line type the same word followed by a space
On the next line start typing the same word again - the completion shows two entries that are visually the same but one contains an extra space
…e into word-completion-improvements
Just pushed changes that should address this problem, @jeremypw . Thank you for the instructions to reproduce it. |
I can confirm that the above issue with spaces is fixed. I'll dogfood for a day or two to see if anything else arises. |
plugins/word-completion/engine.vala
Outdated
public bool is_delimiter (unichar c) { | ||
private Scratch.Plugins.PrefixTree prefix_tree; | ||
|
||
public const string DELIMITERS = " .,;:?{}[]()0123456789+-=&|-<>*\\/\r\n\t\'\"`"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not consider hyphen and underscore as delimiters (see my separate PR on this issue). If you agree, could you change it here?
@igordsm I'd like to merge this as soon as possible - are you able to address the underscore/hyphen issue? |
Fixes #1147
This makes word completion work with Snippets from #1113 . It's an extensive rewrite of the plugin, so there are probably lots of possible improvements. I've been using it without much problem in the last hours, so I'm sending for review.