Skip to content
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

Autocomplete Implementation #1949

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Autocomplete Implementation #1949

wants to merge 10 commits into from

Conversation

FastestMolasses
Copy link
Member

Description

Adds first iteration of the autocomplete feature.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

handleNotification(notification)
case let .error(error):
print("Error from EventStream for \(key.languageId.rawValue): \(error)")
}
}

private func handleRequest(_ request: ServerRequest) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets leave this commented for now.


/// Will query the language server for autocomplete suggestions and then display the window.
@MainActor
func showAutocompleteWindow() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mentioned it in the review of the CETV pr, but this function should be moved to CESE. That should simplify this type, as you could instead add a parameter to CESE for an optional ItemBoxDelegate, and ignore the unnecessary TextViewCoordinator conformance. Then make the ItemBoxDelegate pass the text controller when it inserts items so you can still make that range conversion on line 111.

That does make it a little harder to get this object down to be visible to the CodeFileView, but check how it works with the content coordinator. I tried to make it so it's extendable for new types like this.

  • Add a published property to CodeFileDocument
  • Extend the type in LanguageServerFileMap to contain this object
  • When opening a document, set this object in the CodeFileDocument's new published property

@FastestMolasses FastestMolasses marked this pull request as ready for review December 30, 2024 04:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants