jdt-ls: init at 0.67.0 by jlesquembre · Pull Request #107424 · NixOS/nixpkgs

jlesquembre · 2020-12-22T21:47:38Z

Motivation for this change

Add java language server

Things done

jlesquembre · 2021-01-16T10:58:40Z

/marvin opt-in
/status needs_reviewer

marvin-mk2 · 2021-01-16T10:58:42Z

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

matt-snider · 2021-01-16T11:15:25Z

pkgs/development/tools/jdt-ls/jdt-ls-wrapper.sh

I think -D and -X options need to come before -jar to take effect.

You are right, fixing it

matt-snider · 2021-01-16T11:16:58Z

pkgs/development/tools/jdt-ls/default.nix

It's strange that you have $out/share/plugins and I have $out/share/java/plugins. Do they both work? What is the difference between these two directories?

I think it doesn't matter, they just need to be in the same directory. Did you pick share/java/plugins for a specific reason?
In the config.ini file, we have something like this: osgi.bundles=reference\:file\:....jar As I understood it, that jars are relative to the launcher jar, the one in the java ... -jar file.jar command

share/java/plugins would be a better idea because it is java specific and it is clear in a combined environment where the path is coming from.

matt-snider · 2021-01-16T11:17:49Z

pkgs/development/tools/jdt-ls/default.nix

Oh that's a nice thing to note. I wasn't aware. I think it would be OK to only support config_linux and config_mac to start with, and provide an option to the attrset later on for the lightweight version if someone is looking for it.

jlesquembre · 2021-01-17T22:17:41Z

@matt-snider thanks for taking a look! Based on your comments I did some updates, I also added more comments in the script to make it easier to understand how it works, hopefully is more clear now.
I hope to get the java LS added, I don't care which PR is merged, if you find something useful in my PR, feel free to take it. My only concern is that currently in your version, using makeWrapper, the final script doesn't seem too flexible (at least I didn't get how to override the default options)

marvin-mk2 · 2021-01-21T01:26:24Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-01-24T06:09:20Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-01-27T09:43:06Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-01-30T11:18:38Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-02-02T11:39:02Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-02-05T13:16:31Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

rycee · 2021-02-06T18:51:02Z

pkgs/development/tools/jdt-ls/default.nix

+      # There are config_ss_* directories too, use them to run the lightweight
+      # version, see
+      # https://github.com/eclipse/eclipse.jdt.ls/wiki/Running-the-lightweight-syntax-language-server
+      config-path = if stdenv.isDarwin then "config_mac" else "config_linux";


I think it looks very good overall and should be merged, but should use camel case here:

Suggested change

config-path = if stdenv.isDarwin then "config_mac" else "config_linux";

configPath = if stdenv.isDarwin then "config_mac" else "config_linux";

matt-snider · 2021-02-07T09:48:23Z

@rycee Just take note that there is also #99330 which this PR is based on. That PR predates this one by over two months.

The main difference is the wrapper script used here instead of makeWrapper in mine. As I commented there, I don't think a separate script is necessary, since it mostly just handles arguments like -Xmx which can be passed more simply via JAVA_OPTS. My main criticism was that if this approach is to be supported, it means adding a new script parameter each time someone wants another Java option supported. Support for --java-opts has since been added to this PR, but that eliminates the need for the script entirely in my opinion. Otherwise there will be two ways to specify these parameters (--xmx 2g or --java-opts "-Xmx2g"), which while not a problem, is just not necessary.

That being said, from a user perspective, both PRs will deliver the same functionality. Also, I admittedly wasn't familiar with this way of building a wrapper script with substituteInPlace, so perhaps that is actually the more idiomatic approach in Nix.

@jlesquembre I think we should come to some sort of a decision here on which PR we will go with. Given how little they differ, this is a bit of a waste of time to have the code reviewed twice. You opened this PR over two months after mine, but if it is the better approach, I'd be OK to close mine. It's just that the difference is so minor, that I'm having trouble deciding whether that is the case.

In retrospect, I think it would have been better for you to have commented your suggestions on my PR instead of opening a separate one, or at least linked to my PR in your description so that the fact that yours is a duplicate is more obvious.

marvin-mk2 · 2021-02-12T16:05:45Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-02-15T21:09:07Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-02-18T23:15:55Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-02-22T03:37:30Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-02-25T07:20:19Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-02-28T07:21:25Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-03T09:29:03Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-06T12:16:08Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-09T13:47:46Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-12T17:36:06Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-15T22:15:21Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-19T00:57:20Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-22T05:37:37Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-25T20:36:32Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

marvin-mk2 · 2021-03-28T20:37:39Z

Reminder: Please review!

This Pull Request is awaiting review. If you are the assigned reviewer, please have a look. Try to find another reviewer if necessary. If you can't, please say so. If the status is not accurate, please change it. If nothing happens, this PR ~~will be~~ should be put back in the needs_reviewer queue in one day.

Note: This feature is currently broken. The bot will not actually change the status. If you see this message multiple times, please request a status change manually.

stale · 2021-10-12T14:43:32Z

I marked this as stale due to inactivity. → More info

Artturin · 2022-05-01T23:33:07Z

Merged the other pr

jlesquembre marked this pull request as draft December 22, 2020 21:47

jlesquembre mentioned this pull request Dec 22, 2020

jdt-language-server: init at 1.8.0 #99330

Merged

10 tasks

ofborg bot added the 8.has: package (new) This PR adds a new package label Dec 22, 2020

ofborg bot requested a review from matt-snider December 22, 2020 21:56

jlesquembre force-pushed the jdt-ls branch from 7a14547 to 6f65c13 Compare December 24, 2020 08:55

jlesquembre force-pushed the jdt-ls branch from 6f65c13 to 63c6fce Compare January 2, 2021 22:55

jlesquembre marked this pull request as ready for review January 2, 2021 22:58

jlesquembre force-pushed the jdt-ls branch from 63c6fce to ce89871 Compare January 11, 2021 21:05

marvin-mk2 bot added the marvin label Jan 16, 2021

marvin-mk2 bot added the needs_reviewer label Jan 16, 2021

matt-snider reviewed Jan 16, 2021

View reviewed changes

marvin-mk2 bot added awaiting_reviewer and removed needs_reviewer labels Jan 16, 2021

jdt-ls: init at 0.67.0

3825eb5

jlesquembre force-pushed the jdt-ls branch from ce89871 to 3825eb5 Compare January 17, 2021 21:38

ofborg bot requested a review from matt-snider January 17, 2021 21:47

rycee reviewed Feb 6, 2021

View reviewed changes

jlesquembre removed awaiting_reviewer labels Mar 29, 2021

hqurve mentioned this pull request Aug 2, 2021

java-language-server: init at 0.2.38 #131262

Merged

11 tasks

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 12, 2021

Artturin closed this May 1, 2022

	config-path = if stdenv.isDarwin then "config_mac" else "config_linux";
	configPath = if stdenv.isDarwin then "config_mac" else "config_linux";

Uh oh!

Comments

Conversation

jlesquembre commented Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation for this change

Things done

Uh oh!

jlesquembre commented Jan 16, 2021

Uh oh!

marvin-mk2 bot commented Jan 16, 2021

Uh oh!

matt-snider Jan 16, 2021

Choose a reason for hiding this comment

Uh oh!

jlesquembre Jan 17, 2021

Choose a reason for hiding this comment

Uh oh!

matt-snider Jan 16, 2021

Choose a reason for hiding this comment

Uh oh!

jlesquembre Jan 17, 2021

Choose a reason for hiding this comment

Uh oh!

SuperSandro2000 Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

matt-snider Jan 16, 2021

Choose a reason for hiding this comment

Uh oh!

jlesquembre commented Jan 17, 2021

Uh oh!

marvin-mk2 bot commented Jan 21, 2021

Uh oh!

marvin-mk2 bot commented Jan 24, 2021

Uh oh!

marvin-mk2 bot commented Jan 27, 2021

Uh oh!

marvin-mk2 bot commented Jan 30, 2021

Uh oh!

marvin-mk2 bot commented Feb 2, 2021

Uh oh!

marvin-mk2 bot commented Feb 5, 2021

Uh oh!

rycee Feb 6, 2021

Choose a reason for hiding this comment

Uh oh!

matt-snider commented Feb 7, 2021

Uh oh!

marvin-mk2 bot commented Feb 12, 2021

Uh oh!

marvin-mk2 bot commented Feb 15, 2021

Uh oh!

marvin-mk2 bot commented Feb 18, 2021

Uh oh!

marvin-mk2 bot commented Feb 22, 2021

Uh oh!

marvin-mk2 bot commented Feb 25, 2021

Uh oh!

marvin-mk2 bot commented Feb 28, 2021

Uh oh!

marvin-mk2 bot commented Mar 3, 2021

Uh oh!

marvin-mk2 bot commented Mar 6, 2021

Uh oh!

marvin-mk2 bot commented Mar 9, 2021

Uh oh!

marvin-mk2 bot commented Mar 12, 2021

Uh oh!

marvin-mk2 bot commented Mar 15, 2021

Uh oh!

marvin-mk2 bot commented Mar 19, 2021

Uh oh!

marvin-mk2 bot commented Mar 22, 2021

Uh oh!

marvin-mk2 bot commented Mar 25, 2021

Uh oh!

marvin-mk2 bot commented Mar 28, 2021

Uh oh!

stale bot commented Oct 12, 2021

jlesquembre commented Dec 22, 2020 •

edited

Loading