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

Fix non-word character replacement #930

Merged
merged 1 commit into from Nov 23, 2017

Conversation

Projects
None yet
3 participants
@leroix
Contributor

leroix commented Nov 22, 2017

addendum to #928

fixes #920

For instance, replacing $foo with the suggestion, $foobar, results in
$$foobar.

This PR makes autocomplete-plus now use Cursor.getCurrentWordPrefix
instead of using its own prefix regex. This will help keep things consistent with the editor.nonWordCharacters setting.

Fix non-word character replacement
fixes #920

For instance, replacing $foo with the suggestion, $foobar, results in
$$foobar.

This PR makes autocomplete-plus now use `Cursor.getCurrentWordPrefix`
instead of using its own prefix regex.

@leroix leroix merged commit f85ad55 into master Nov 23, 2017

2 checks passed

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

@leroix leroix deleted the fix-nonwordchar-replacement branch Nov 23, 2017

@leroix leroix referenced this pull request Dec 13, 2017

Closed

Atom SASS/SCSS autocomplete doesn't work #16366

1 of 1 task complete
@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Dec 21, 2017

Contributor

@joefitzgerald You mentioned this PR is a breaking change? Can you explain a bit?

Contributor

nathansobo commented Dec 21, 2017

@joefitzgerald You mentioned this PR is a breaking change? Can you explain a bit?

@joefitzgerald

This comment has been minimized.

Show comment
Hide comment
@joefitzgerald

joefitzgerald Dec 22, 2017

Member

@nathansobo the prior prefix code was put in place due to inadequacies in many of the grammars in generating the word prefix for scenarios important to autocomplete. For example, in .source.go files, when you type . after Expect(w.String()), the prefix is now ))., which is not correct. This is perhaps the most important use case for autocomplete in a .go file (autocompleting after a ., and it broke because we changed the semantics for how the prefix is calculated.

I have no issue with moving the responsibility for this back to the grammars; that is actually the correct and most maintainable solution. I have an issue with the fact that we made a change to the semantics for prefix calculation without preserving the old behavior for backwards compatibility. Ideally the new prefix semantics would be included in a new field in the suggestion request. I've worked around it for now in go-plus, but I suspect other provider authors have been surprised by changes in semantics associated with this PR.

Member

joefitzgerald commented Dec 22, 2017

@nathansobo the prior prefix code was put in place due to inadequacies in many of the grammars in generating the word prefix for scenarios important to autocomplete. For example, in .source.go files, when you type . after Expect(w.String()), the prefix is now ))., which is not correct. This is perhaps the most important use case for autocomplete in a .go file (autocompleting after a ., and it broke because we changed the semantics for how the prefix is calculated.

I have no issue with moving the responsibility for this back to the grammars; that is actually the correct and most maintainable solution. I have an issue with the fact that we made a change to the semantics for prefix calculation without preserving the old behavior for backwards compatibility. Ideally the new prefix semantics would be included in a new field in the suggestion request. I've worked around it for now in go-plus, but I suspect other provider authors have been surprised by changes in semantics associated with this PR.

@nathansobo

This comment has been minimized.

Show comment
Hide comment
@nathansobo

nathansobo Jan 2, 2018

Contributor

@joefitzgerald What did you expect the prefix to be?

Contributor

nathansobo commented Jan 2, 2018

@joefitzgerald What did you expect the prefix to be?

@joefitzgerald

This comment has been minimized.

Show comment
Hide comment
@joefitzgerald

joefitzgerald Jan 2, 2018

Member

In the scenario I described when you type ., the prefix should be (i.e. nothing) or .. I have to go back and look to see what the prior behavior was.

Member

joefitzgerald commented Jan 2, 2018

In the scenario I described when you type ., the prefix should be (i.e. nothing) or .. I have to go back and look to see what the prior behavior was.

nathansobo added a commit that referenced this pull request Jan 3, 2018

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