Adding code completion #87

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.

Sign up for GitHub

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

Jump to bottom

Open

tiagobstr wants to merge 6 commits into main from feature/code-completion

tiagobstr commented Sep 19, 2025 •

edited

Loading

Description

Adding code completion feature to be used on protostar;
Adding a generic method to inject a code completion "engine" so that in the future the completion logic can be handled for each different client/language;
Also fixing a small issue related with trying to use a location without permissions for the indexPath.

Testing

Manual tests

Checklist

The correct base branch is being used;
All active GitHub checks for tests and formatting are passing;


          initial version of the completion mechanism

466ef40

tiagobstr changed the title ~~initial version of the completion mechanism~~ Code Completion

tiagobstr added 2 commits

September 19, 2025 16:04


          adding missing file

4307ed2


          removing resolver

425f2ce

tiagobstr changed the title ~~Code Completion~~ Adding code completion


          small tweaks

ad3bdec

tiagobstr marked this pull request as ready for review

September 22, 2025 10:25

tiagobstr requested review from alessiostalla and ftomassetti

September 22, 2025 10:25

alessiostalla requested changes

View reviewed changes

Member

alessiostalla left a comment

Overall this is really nice in my opinion, I think the API could be improved by not hiding CompletableFuture as that gives more control to implementations (if we then see we tend to repeat boilerplate code around CF we can always include a default base completion engine as an abstract base class)

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt Outdated Show resolved Hide resolved

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt Outdated Show resolved Hide resolved

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt Outdated Show resolved Hide resolved

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt Outdated Show resolved Hide resolved

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt Show resolved Hide resolved

library/src/main/kotlin/com/strumenta/kolasu/languageserver/CompletionEngine.kt Outdated Show resolved Hide resolved

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt Outdated Show resolved Hide resolved

ftomassetti reviewed

View reviewed changes

Member

ftomassetti left a comment

I do not know enough about this library to provide a meaningful review, so I just provided some comments

library/src/main/kotlin/com/strumenta/kolasu/languageserver/CompletionEngine.kt Show resolved Hide resolved

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt Show resolved Hide resolved

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt Outdated Show resolved Hide resolved

tiagobstr added 2 commits

September 23, 2025 14:58


          removing reference to kolasu + adding trigger characters to the inter…

068436f

…face


          applying code review suggestions

d3a0f7b

tiagobstr requested a review from alessiostalla

September 26, 2025 15:12

alessiostalla approved these changes

View reviewed changes

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt

    
                  ): CompletableFuture<CompletionItem> =

                      CompletableFuture.completedFuture(

                          try { completionEngine?.resolve(item) ?: item }

                          catch (t: Throwable) {

Member

alessiostalla Sep 30, 2025

It's rarely appropriate to catch Throwable as that includes system errors such as OutOfMemoryError so unless there are compelling reasons to keep it like that I'd just catch Exception

library/src/main/kotlin/com/strumenta/kolasu/languageserver/KolasuServer.kt

    
                      item: CompletionItem

                  ): CompletableFuture<CompletionItem> =

                      CompletableFuture.completedFuture(

                          try { completionEngine?.resolve(item) ?: item }

Member

alessiostalla Sep 30, 2025

I would have moved this CompletableFuture to the engine interface as well for consistency, even though we've rarely used that method. However we can keep it like that and open an issue if this is blocking further work on code completion.

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

Labels

None yet