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

Promote end-user usage instruction more in the README #1887

Merged

Conversation

mfussenegger
Copy link
Contributor

The README was rather developer focused (and only targeting eclipse
users at that)

This puts the focus more on end-user usage.

The main motivation for this is to enable other editors or plugins to
point users to the installation instructions within this repository to
focus their own efforts on explaining how to configure the
editor/plugin.

@eclipse-ls-bot
Copy link

Can one of the admins verify this patch?

README.md Outdated
Comment on lines 87 to 103
----------------------------

6. Choosing a value for `-configuration`: this is the path to your platform's configuration directory. For linux, use `./config_linux`. For windows, use `./config_win`. For mac/OS X, use `./config_mac`.
**Pre-requisite: Java 11 must be installed on your machine and configured in Eclipse.**

7. Choosing a value for `-data`: the value for your data directory, should be the directory where your active workspace is, and you wish for the java langserver to add in its default files. Should also be the absolute path to this directory, ie., /home/username/workspace
0. Fork and clone the repository
1. Install [Eclipse IDE for Eclipse Committers](https://www.eclipse.org/downloads/packages/release/2018-09/r/eclipse-ide-eclipse-committers) that will have the most needed plugins already installed. Alternatively,
you can get the [Eclipse IDE for Java developers](https://www.eclipse.org/downloads/packages/release/2018-09/r/eclipse-ide-java-developers)
and just install Eclipse PDE from the Eclipse Marketplace.

8. Notes about debugging: the `-agentlib:` is for connecting a java debugger agent to the process, and if you wish to debug the server from the start of execution, set `suspend=y` so that the JVM will wait for your debugger prior to starting the server
2. Once installed use `File > Open Projects from File System...` and
point it at `eclipse.jdt.ls` and Eclipse should automatically
detect the projects and import it properly.

3. If, after importing the projects, you see an error on `pom.xml` about Tycho, you can use Quick Fix
(Ctrl+1) to install the Tycho maven integration.

4. At that point, some plug-ins should still be missing in order to build the project. You can either open `org.eclipse.jdt.ls.target/org.eclipse.jdt.ls.tp.target` in the Target Editor (which is the default editor) and click on `Set Target Platform`, or alternatively, open `Preferences > Plug-in Development > Target Platform` and select `Java Language Server Target Definition`). Eclipse will take some time to download all the required dependencies. It should then be able to compile all the projects in the workspace.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd also like to add instructions for how to develop eclipse.jdt.ls using eclipse.jdt.ls - I found #305 but didn't really manage to extract how it is supposed to work.

Is https://github.com/gorkem/jdt.ls.pde/ still up2date and if so - should adding it to the bundles configuration enable eclipse.jdt.ls to load eclipse.jdt.ls ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Using eclipse.jdt.ls to dogfood eclispe.jdt.ls is already supported, please see the contribution guide https://github.com/eclipse/eclipse.jdt.ls/blob/master/CONTRIBUTING.md#a-setting-up-the-jdt-language-server-in-vs-code.

Also i think we can remove the part "Development First Time Setup" from README, because CONTRIBUTING.md has already covered how to setup the develop environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a fixup that changes the Development First Time Setup section to just have a link to the contributing file.

@mfussenegger mfussenegger marked this pull request as ready for review September 23, 2021 14:13
README.md Outdated
1. Choose a connection type from "Managing connection types" section below, and then set those environment variables in your terminal or specify them as system properties with `-D` prior to continuing
1. Chose a value for `-configuration`: this is the path to your platform's configuration directory. For linux, use `./config_linux`. For windows, use `./config_win`. For mac/OS X, use `./config_mac`.
2. Change the filename of the jar in `-jar ./plugins/...` to match the version you built or downloaded.
3. Choose a value for `-data`: the value for your data directory, should be the directory where your active workspace is, and you wish for the java langserver to add in its default files. Should also be the absolute path to this directory, ie., /home/username/workspace
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I've seen this should be unique per project(?).
eclipse.jdt.ls always ends up creating a jdt.ls-java-project folder inside (instead of project specific ones).

Or is this due to the lsp client not setting some initialization options that it should?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's per workspace. -data parameter is used to specify a local cache directory to store the metadata files (e.g. index files, logs, etc.) generated by Java language server.

Please notice that this parameter is not the user's project root folder. The project root folder you want to import is passed by LSP Initialization options.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the clarification. I rephrased the third point here a bit in the fixup I pushed

Copy link
Contributor

@testforstephen testforstephen left a comment

Choose a reason for hiding this comment

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

Looks good to me. Except for a minor typo.

README.md Outdated
```

6. Choosing a value for `-configuration`: this is the path to your platform's configuration directory. For linux, use `./config_linux`. For windows, use `./config_win`. For mac/OS X, use `./config_mac`.
1. Chose a value for `-configuration`: this is the path to your platform's configuration directory. For linux, use `./config_linux`. For windows, use `./config_win`. For mac/OS X, use `./config_mac`.
Copy link
Contributor

Choose a reason for hiding this comment

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

typos. Chose -> Choose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. I rebased and squashed the previous fixup. I hope that's okay.

The README was rather developer focused (and only targeting eclipse
users at that)

This puts the focus more on end-user usage.

The main motivation for this is to enable other editors or plugins to
point users to the installation instructions within this repository to
focus their own efforts on explaining how to configure the
editor/plugin.

Signed-off-by: Mathias Fussenegger <f.mathias@zignar.net>
@testforstephen testforstephen merged commit 9974132 into eclipse-jdtls:master Sep 26, 2021
@testforstephen testforstephen added this to the End September 2021 milestone Sep 26, 2021
@testforstephen
Copy link
Contributor

@mfussenegger Looks better, thanks for your contribution.

@mfussenegger mfussenegger deleted the readme-usage-instruction branch September 26, 2021 13:36
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.

None yet

3 participants