Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Implement the Provider API 4.1.0 with the new RFC #1102

Closed
wants to merge 1 commit into from

Conversation

illright
Copy link
Contributor

@illright illright commented Jan 8, 2021

Description of the Change

Implement the additional features for the Provider API in order to support LSP
Relevant issue describing the new API in detail: #1101

Benefits

LSP compliance will allow much smarter autocompletion for supporting languages

Possible Drawbacks

There are a couple of points where the behaviour was slightly altered:

  • Atom's auto-indentation system has misbehaved so now we pass parameters to insertText that disable any whitespace modification by the package itself to comply with the LSP spec and allow providers to handle indentation

I tried various parameters in Selection.insertText but none of them worked properly. Originally I had an idea to create another setting where providers could ask the package not to do any whitespace modification, which would have been perfect and backwards-compatible, but it seems like the whitespace modification didn't work in the first place, so technically we are still backwards-compatible, albeit due to broken Atom behaviour.
OR maybe I messed up while experimenting, that's also a possibility. I'm willing to elaborate on my findings if needed.

  • Hiding the Autocomplete window on cursor movement without text modification now happens 100ms after the cursor movement

This is a hack to support commit characters in light of Bracket Matcher's behaviour. Here's the situation in more detail:
A commit character is a character that you can type while you have your Autocomplete suggestions dropdown visible and a preferred item selected. When you type that character, it causes the selected item to be confirmed and inserted, and the commit character to be inserted after it.
For example, it could be used in completing functions, where the commit character is (. Typing an opening parenthesis allows you to confirm the suggestion and keep coding right away.

The problem is the Bracket Matcher. When you type an opening parenthesis, the Bracket Matcher also types the closing parenthesis and then moves the cursor left to allow you to keep writing without disturbance. This cursor movement, however, is caught by the cursor observer and it causes the autocomplete suggestion list to be hidden before the "text edited" callback gets a chance to run and a commit character gets to be detected. This slight hack will give the "text edited" callback some time.

I agree this is not a pretty solution, but my aim was to make as few changes to the codebase as possible, and that's what I came up with. I'm open to changing this behaviour.

  • The replacementPrefix is no longer checked before replacing the suggestion.

Now that the new TextEdit objects can span arbitrary ranges, checking for replacement prefix matching is no longer an option, and frankly I'm not sure if that's even needed.

Applicable Issues

Closes #1101

P.S.

I must confess I'm not entirely proud of the code I have written. My goal was to remain as backwards-compatible as possible and change as little code as possible. However, the codebase is in dire need of cleanup, the deprecated APIs should be removed, blah blah. But again, I need your permission to do it. I'm gladly willing to convert the codebase to TypeScript, refactor and properly deprecate stuff if you say this is acceptable.

Tests are failing, by the way, will fix them in upcoming commits, but figured I'd publish the changes to spark discussion.

@illright
Copy link
Contributor Author

Alright, there's something fundamentally wrong with the test suite. In progress of fixing the tests I added a line with a console.log and that has immediately caused 108 tests to start timing out and failing. I'll get back to this PR once I fix up the codebase

@illright illright closed this Jan 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: Change the Provider API to allow LSP compliance
1 participant