Skip to content
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.

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

feat(java)!: move to nvim-java for jdtls setup #1095

Closed
wants to merge 1 commit into from
Closed

Conversation

mehalter
Copy link
Member

@mehalter mehalter commented Jul 8, 2024

Closes #1062
Closes #1064

📑 Description

This moves the Java pack to use nvim-java which greatly decreases the complexity of the pack as well as adds stuff like spring-boot support out of the box.

I did some testing and it spins up totally fine, the only downside I have found so far is that the debugger in my testing doesn't seem to get configured correctly. A debugging adapter is created but no DAP configurations are inserted. There seems to be an upstream issue regarding this: nvim-java/nvim-java#169

Someone might want to run this down or verify if it's a me problem before we merge this in.

TODO

  • Confirm debugger and make sure that works to not introduce a big regression

Copy link

github-actions bot commented Jul 8, 2024

Review Checklist

Does this PR follow the [Contribution Guidelines](development guidelines)? Following is a partial checklist:

Proper conventional commit scoping:

  • If you are adding a new plugin, the scope would be the name of the category it is being added into. ex. feat(utility): added noice.nvim plugin

  • If you are modifying a pre-existing plugin or pack, the scope would be the name of the plugin folder. ex. fix(noice-nvim): fix LSP handler error

  • Pull request title has the appropriate conventional commit type and scope where the scope is the name of the pre-existing directory in the project as described above

  • README is properly formatted and uses fenced in links with <url> unless they are inside a [title](url)

  • Proper usage of opts table rather than setting things up with the config function.

@mehalter mehalter changed the title feat(java): move to nvim-java for jdtls setup feat(java)!: move to nvim-java for jdtls setup Jul 8, 2024
@Juniar-Rakhman
Copy link
Contributor

I work with the java pack on a daily basis. Can I test this out first?

@mehalter
Copy link
Member Author

Please do

@Juniar-Rakhman
Copy link
Contributor

Tried your changes, lombok isn't working (using java 21):
image

Give me sometimes to test nvim-java out to find the optimal configs for both Spring and non-Spring project (Quarkus)

@Juniar-Rakhman
Copy link
Contributor

I was able to solve the lombok issue by doing full reset on nvim and also deleting my ~/.m2 folder. I couldn't get debugging to work because of this upstream issue: nvim-java/nvim-java#125. I tried disabling spring-boot-tools via config, but it does not work:

return {
  { import = "astrocommunity.pack.java" },
  {
    "AstroNvim/astrolsp",
    opts = {
      config = {
        jdtls = {
          settings = {
            java = {
              configuration = {
                java_debug_adapter = {
                  enable = true,
                },
                spring_boot_tools = {
                  enable = false,
                },
                jdk = {
                  auto_install = false,
                },
                runtimes = {
                  {
                    name = "JavaSE-21",
                    path = os.getenv "HOME" .. "/.sdkman/candidates/java/21.0.1-tem",
                  },
                },
              },
            },
          },
        },
      },
    },
  },
}

@Juniar-Rakhman
Copy link
Contributor

I don't think nvim-java is mature enough yet to replace nvim-jdtls.
@mehalter How about putting this into a separate pack, i.e. pack/spring-boot? That way, we can keep the already working java pack and experiment with nvim-java using spring-boot pack?

@mehalter
Copy link
Member Author

@Juniar-Rakhman the options for disabling spring boot are not part of jdtls. They should be passed to the opts for nvim-java

@mehalter
Copy link
Member Author

I think if we do that, we might as well just make a plugin for spring-boot.nvim and just use it with the current Java pack. No need to use nvim-java at all. I also don't care at all. I don't use Java nor care what is in this pack. Feel free to open a PR with whatever you want.

I do think configuring spring boot directly rather than using nvim-java makes the most sense

@mehalter
Copy link
Member Author

https://github.com/JavaHello/spring-boot.nvim

I don't even know if it needs to be a pack, it can just be a plugin in the lsp section or whatever makes the most sense. Provides a pack of things

@Juniar-Rakhman
Copy link
Contributor

@Juniar-Rakhman the options for disabling spring boot are not part of jdtls. They should be passed to the opts for nvim-java

thanks! the following config works for me:

return {
  { import = "astrocommunity.pack.java" },
  {
    "nvim-java/nvim-java",
    opts = {
      jdk = {
        auto_install = false,
      },
      spring_boot_tools = {
        enable = false,
      },
    },
  },
  {
    "AstroNvim/astrolsp",
    opts = {
      config = {
        jdtls = {
          settings = {
            java = {
              configuration = {
                runtimes = {
                  {
                    name = "JavaSE-21",
                    path = os.getenv "HOME" .. "/.sdkman/candidates/java/21.0.1-tem",
                  },
                },
              },
            },
          },
        },
      },
    },
  },
}

Debugging works on test files at least, further testing is needed. You need to turn off the spring-boot-tools to get around the bug.

@Juniar-Rakhman
Copy link
Contributor

Juniar-Rakhman commented Jul 14, 2024

@mehalter I did some further testing. Debugging and testing works, but not as intuitive as nvim-jdtls. Nvim-jdtls functionalities that I often use, such as generating tests, go to tests, attaching debugger to a running process are still missing. Debugging experience is not very nice, when running test in debug mode, the dap-ui does not open automatically.

So in conclusion I would rather stick with nvim-jdtls, maybe we could upgrade the current java pack and integrate nvim-jdtls with spring-boot.nvim.

I know that nvim-java is much easier to config, but I think the current java pack is good enough in hiding the complexity of nvim-jdtls. I don't think moving the pack to nvim-java is worth the missing functionalities and the regression in user experience.

@mehalter mehalter closed this Jul 14, 2024
@Uzaaft
Copy link
Member

Uzaaft commented Jul 14, 2024

Thanks for taking the time to test this PR @Juniar-Rakhman. This is exactly what the community near more off! 🫶🏽

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.

Add a spring-boot pack Replace nvim-jdtls with nvim-java
3 participants