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

Remove the old fuzzy completion provider #909

Merged
merged 2 commits into from Oct 25, 2017

Conversation

Projects
None yet
3 participants
@maxbrunsfeld
Contributor

maxbrunsfeld commented Oct 25, 2017

I believe the FuzzyProvider was what we used before the SymbolProvider was created. Now we have an even newer default autocompletion provider, the SubsequenceProvider, so I think it's safe to remove the FuzzyProvider.

This came up because the FuzzyProvider is the only bundled package that uses the TextBuffer.onWillChange event, whose behavior I want to change for performance reasons. Specifically, I want to make it fire only once per text-buffer transaction, which will require removing the argument with the oldRange, newRange, oldText and newText properties.

/cc @leroix @nathansobo

@maxbrunsfeld maxbrunsfeld merged commit a2eab73 into master Oct 25, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@maxbrunsfeld maxbrunsfeld deleted the mb-remove-fuzzy-provider branch Oct 25, 2017

@park9140 park9140 removed the in progress label Oct 25, 2017

@leroix

This comment has been minimized.

Show comment
Hide comment
@leroix

leroix Oct 25, 2017

Contributor

@maxbrunsfeld I think this makes sense. We should remove Fuzzy from the config schema in package.json as well.

Will your TextBuffer changes make setText faster by chance? That could be a small win for the subsequence provider.

Contributor

leroix commented Oct 25, 2017

@maxbrunsfeld I think this makes sense. We should remove Fuzzy from the config schema in package.json as well.

Will your TextBuffer changes make setText faster by chance? That could be a small win for the subsequence provider.

@maxbrunsfeld

This comment has been minimized.

Show comment
Hide comment
@maxbrunsfeld

maxbrunsfeld Oct 25, 2017

Contributor

Will your TextBuffer changes make setText faster by chance? That could be a small win for the subsequence provider.

😞 I don't think they will; they are focused on optimizing cases where many small edits happen in one transaction (e.g. multi-cursor editing). I'm surprised that setText is slow though. Is this for the 'hidden' text-buffer that provides the config-based suggestions? Maybe it'd be worth actually implementing a FuzzyMatchSet class in superstring.

Contributor

maxbrunsfeld commented Oct 25, 2017

Will your TextBuffer changes make setText faster by chance? That could be a small win for the subsequence provider.

😞 I don't think they will; they are focused on optimizing cases where many small edits happen in one transaction (e.g. multi-cursor editing). I'm surprised that setText is slow though. Is this for the 'hidden' text-buffer that provides the config-based suggestions? Maybe it'd be worth actually implementing a FuzzyMatchSet class in superstring.

@leroix

This comment has been minimized.

Show comment
Hide comment
@leroix

leroix Oct 25, 2017

Contributor

It's actually pretty quick, but I'm in the territory where fractions of a millisecond matter.

screen shot 2017-10-25 at 3 29 02 pm

Contributor

leroix commented Oct 25, 2017

It's actually pretty quick, but I'm in the territory where fractions of a millisecond matter.

screen shot 2017-10-25 at 3 29 02 pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment