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

Introduce service API version 4.0 with simplified prefix handling #940

Merged
merged 10 commits into from Jan 3, 2018

Conversation

Projects
None yet
1 participant
@nathansobo
Contributor

nathansobo commented Jan 3, 2018

🍐 d with @maxbrunsfeld

Description of the Change

Autocomplete providers are passed a prefix string, which describes the characters preceding the cursor.

Previously, in API v3, this prefix was computed based on a regular expression in this package. It would recognize a fixed set of word characters or a sequence of non-word characters. Unfortunately, this clashed with the fact that different languages have different sets of valid word characters.

When @leroix added the subsequence provider in #886, he introduced the ability to complete words containing symbols, such as $hello. Unfortunately, since the prefix did not include the leading $ based on an assumption it was a non-word character, returning a completion of $hello would cause a duplication due to the prefix not including the $, yielding $$hello.

In #930, @leroix changed prefix calculation to use the Cursor.getCurrentWordPrefix method, but this was problematic for a couple of reasons.

  1. It was a breaking change, causing different prefixes to be passed to providers than they previously assumed
  2. The getCurrentWordPrefix method has a misleading name, and it actually returns contiguous runs of word or non-word characters in order to support cursor movement.

In this PR, we revert #930 and restore the original prefix behavior for API version 3. We then introduce API version 4, which changes the prefix computation as follows:

  • We only ever include characters that are part of a word. If autocomplete is triggered following a non-word character, the prefix is an empty string. In these cases, the buffer can be inspected directly based on the given position and the provider can do whatever it wants.
  • Unless they are explicitly black-listed in the editor.nonWordCharacters setting, the following characters are allowed within words: /\()"':,.;<>~!@#$%^&*|+=[]{}``?_-…. Most of these are excluded by default, but certain languages may relax the default non-word characters to allow characters like - or $ to be included in words.
  • The replacementPrefix is always defaulted to the prefix passed to the provider if no replacementPrefix is specified for a suggestion.

This PR also simplifies our handling of semver. Since we only need to deal with breaking changes, it replaces semver strings with simple integer comparisons.

Alternate Designs

We could have special-cased the SubsequenceProvider to compute its own prefix. In fact, we tried this in order to turn this change around more quickly, but then realized that fixing its tests would take more time and effort than just fundamentally improving the prefix calculation in a new API version.

Benefits

Fixes #855
Fixes breakage raised in #930 (comment)

Possible Drawbacks

Yet another API version for completion providers, and we still support version 1.0.

Applicable Issues

Fixes #855

nathansobo added some commits Jan 2, 2018

Simplify API versioning
Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>
Simplify prefix computation in 4.0 API
* Honor language settings for word characters
* Never include non-word characters

Signed-off-by: Nathan Sobo <nathan@github.com>
Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>
Always default replacement prefix to the word preceding cursor in API v4
Previously, we would only default the replacement prefix if the cursor
prefix contained all word characters, which would make it impossible to
complete prefixes containing symbols such as $.

Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>
Use 4.0 API in subsequence provider
This allows it to correctly autocomplete words containing symbolic
characters such as $.

@nathansobo nathansobo requested a review from joefitzgerald Jan 3, 2018

nathansobo added some commits Jan 3, 2018

Don't pass legacyPrefix to 4.0.0 providers
Signed-off-by: Max Brunsfeld <maxbrunsfeld@gmail.com>

@nathansobo nathansobo merged commit 5b7f353 into master Jan 3, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@nathansobo nathansobo deleted the api-4 branch Jan 3, 2018

@nathansobo nathansobo self-assigned this Jan 3, 2018

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