Skip to content

Conversation

@tartarughina
Copy link
Collaborator

Closes #103

Adopt custom version of jdtls respecting the lsp-types as defined by Zed when suggesting the active parameter.

Discovery and download of latest repo's release use GitHub methods as defined by zed_extension.

- Implement download of latest release of custom jdtls - Implement ~ expansin for user provided java home
@tartarughina tartarughina added the enhancement New feature or request label Oct 28, 2025
@cla-bot cla-bot bot added the cla-signed label Oct 28, 2025
Copy link
Collaborator

@playdohface playdohface left a comment

Choose a reason for hiding this comment

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

In terms of implementation looks mostly good to me, but there are a couple of open questions:

  • this might just be me not having paid attention, but can we document somewhere the tangible issue solved by having the correct active parameter, i.e. reproduction steps for the bug as it manifests in Zed?
  • is there an upstream PR to JDTLS? Even if it ends up being ignored or rejected, I'd like to at least try fixing the problem at the root if we can
  • what are the plans for maintaining the fork going forward? It's a bit of a security concern if we just download the latest release from what seems to the average person a random github repo and run it without any sandbox
@tartarughina
Copy link
Collaborator Author

  • this might just be me not having paid attention, but can we document somewhere the tangible issue solved by having the correct active parameter, i.e. reproduction steps for the bug as it manifests in Zed?

The issue currently exists in Zed when using the standard jdtls, which returns -1 when no active parameter is expected for suggestion. The main tracking issues are:
zed#20874 and jdtls#2434.

To reproduce:

  1. Install the current version of the extension.
  2. Recreate the code shown below.
  3. Place the cursor inside the function calls to trigger the parameter suggestion window.

The tangible issue:

Standard jdtls:

  • ArrayList constructor: All signatures require at least one parameter. When the cursor is placed inside the call, the signature helper is displayed:
    ArrayList constructor with signature help

  • println: Has both parameterized and parameterless overloads. Placing the cursor inside the call does not trigger parameter suggestions:
    println without parameter suggestion

Forked jdtls:

  • Signature help still triggers correctly for constructors:
    Forked jdtls - constructor signature help

  • Parameter suggestions now correctly appear for println when expected:
    Forked jdtls - println with parameter suggestion

Can humanity survive without it? Yes. Does this bother me? Immensely
  • is there an upstream PR to JDTLS? Even if it ends up being ignored or rejected, I'd like to at least try fixing the problem at the root if we can

I can create a PR, even though, at this point, Zed is treated as a third-class citizen compared to Eclipse or VS Code in the maintainers' eyes.

  • what are the plans for maintaining the fork going forward? It's a bit of a security concern if we just download the latest release from what seems to the average person a random github repo and run it without any sandbox

Maintaining the fork would require:

  • Keeping it in sync with upstream
  • Preparing and releasing a new version as soon as upstream publishes one

To further make our point and explain why a fork is being used, we could leave a comment in the code pointing to #103 and this PR.

@playdohface
Copy link
Collaborator

Let's open that upstream PR to JDTLS and link it here and/or in #103 and see what happens. I think if they ignore it or refuse, I am definitely sold on the idea of using our own fork, though we should maybe do the same thing that we did with java-debug and move it into zed-industries for trustworthiness. @MrSubidubi it would be great to have your input here

@tartarughina
Copy link
Collaborator Author

@tartarughina
Copy link
Collaborator Author

The PR on jdtls got merged. There's no more the need of our own fork

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

Labels

cla-signed enhancement New feature or request

2 participants