Skip to content

Fixing edit application and code completion filtering in the LSP client. - #9633

Open
lahodaj wants to merge 1 commit into
apache:masterfrom
lahodaj:typescript-fixes
Open

lahodaj wants to merge 1 commit into
apache:masterfrom
lahodaj:typescript-fixes

Conversation

@lahodaj

@lahodaj lahodaj commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

I was doing some things in TypeScript recently, and there were a few problems that bothered me in the current TypeScript support. This PR is trying to solve two of them:

  • when applying text edits, when two or more (insert) edits have the same start position, the LSP specification says the outcome should be as if there was one edit, and the inserted text was concatenated in the encounter order (ide/lsp.client/src/org/netbeans/modules/lsp/client/Utils.java). The current implementation will put the texts in the opposite order, as it sorts in the reverse order using a stable sort, so the edits with the same position remain in the same order as in the original. As a consequence, the first text edit is inserted first, and then the second one is inserted in front of it. The proposal here is to use a "natural" order to sort the edits, and then reverse, which results in the correct inserted texts.
  • the TS server does not filter completion results. E.g. having obj.test| (| marks the code completion spot), the server will return all methods of obj, not only those starting with test. The client should do the filtering based on filterText (or label if there's no filterText).

^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

  • are all your own work, and you have the right to contribute them.
  • are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

LLMs, Commit messages and PR description:

  • Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.
  • LLM assisted commits should be attributed with an Assisted-by: MODEL_NAME MODEL_VERSION line appended to the commit message.
    • Please mention coding assistance in the PR description too (eg. by adding the same Assisted-by line from above)
    • Please describe the changes in your own words - we'd like to know you understand the changes being made!

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

  1. Was this PR correctly labeled, did the right tests run? When did they run?
  2. Is this PR squashed?
  3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
  4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

@lahodaj lahodaj added this to the NB32 milestone Sep 22, 2026
@lahodaj lahodaj added LSP [ci] enable Language Server Protocol tests ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Sep 22, 2026
@mbien mbien added the TypeScript [ci] enable web job label Sep 22, 2026

@matthiasblaesing matthiasblaesing left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes make sense to me. Having multiple edits for the same position indeed asks for trouble, but the implementation suggested here seems to be in line with the LSP documentation. The filtering also makes sense. This might need refinement later for typecript private (#var) variables, but these are broken right now anyway.

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

Labels

ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) LSP [ci] enable Language Server Protocol tests TypeScript [ci] enable web job

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants