Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Handle replaceStart/replaceEnd in completion proposal #526

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

Conversation

mfussenegger
Copy link
Contributor

@mfussenegger mfussenegger commented Dec 8, 2023

First commit is from #525


Second commit fixes #524
Ensures completion text can be appended by clients

E.g.

For com.| the completion proposal has:

completion: com.sun.jndi.ldap.pool
completionLocation: 3
replaceEnd: 4
replaceStart: 0

insertText will be sun.jndi.ldap.pool

For List.| the completion proposal has:

completion: of()
completionLocation: 4
replaceStart: 5
replaceEnd: 5

insertText will be of()

I didn't find any existing completion tests, so not sure how to write a test case for this

Fixes build errors. The old location is no longer available:

https://download.eclipse.org/releases/2023-12/202311171000/ returns 404
@mfussenegger mfussenegger changed the title completion inserttext Handle replaceStart/replaceEnd in completion proposal Dec 8, 2023
Fixes microsoft#524
Ensures completion text can be appended by clients

E.g.

For `com.|` the completion proposal has:

    completion: com.sun.jndi.ldap.pool
    completionLocation: 3
    replaceEnd: 4
    replaceStart: 0

`insertText` will be `sun.jndi.ldap.pool`

For `List.|` the completion proposal has:

    completion: of()
    completionLocation: 4
    replaceStart: 5
    replaceEnd: 5

`insertText` will be `of()`
@mfussenegger mfussenegger force-pushed the completion-inserttext branch from c343437 to d5eb2db Compare December 8, 2023 12:19
@mfussenegger
Copy link
Contributor Author

Any chance to get some feedback on this?

$.setInsertText(insertText.substring(prefix));
} else {
$.setInsertText(insertText);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't work for VS Code. Typing Sys to trigger completion, select System and it becomes tem.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CompletionItem of completions response always have start = 0 and text prefix inclusion is mixed
2 participants