Skip to content

fix: split CompletionParams and CompletionContext computation#1

Open
rubenporras wants to merge 2 commits into
FlorianKroiss:abort-on-badlocationfrom
rubenporras:pr-1486
Open

fix: split CompletionParams and CompletionContext computation#1
rubenporras wants to merge 2 commits into
FlorianKroiss:abort-on-badlocationfrom
rubenporras:pr-1486

Conversation

@rubenporras
Copy link
Copy Markdown

to reduce the time until we extract the character at the document offset

FlorianKroiss and others added 2 commits January 26, 2026 17:50
@rubenporras
Copy link
Copy Markdown
Author

@FlorianKroiss , if you agree, you can accept this on your fork and I think it will then be applied to your PR, then we can merge both together.

}

private @Nullable String getPositionCharacter(IDocument document, int offset) {
int positionCharacterOffset = offset > 0 ? offset - 1 : offset;
Copy link
Copy Markdown
Owner

@FlorianKroiss FlorianKroiss Jan 27, 2026

Choose a reason for hiding this comment

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

This is missing a check if the document is empty. That's probably why a lot of tests are failing.

@rubenporras I now have a better idea of what you want to change. If it is ok for you, I would apply these change manually to my fork and incorporate the missing check?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sure, go for it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I thought the check was not necessay, but I cannot argue with the failing tests ;)

@FlorianKroiss FlorianKroiss force-pushed the abort-on-badlocation branch 2 times, most recently from c30715b to 9d391c6 Compare January 28, 2026 09:34
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