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

support multi-module java projects #546

Closed
wants to merge 2 commits into from
Closed

support multi-module java projects #546

wants to merge 2 commits into from

Conversation

ukaszg
Copy link

@ukaszg ukaszg commented Sep 30, 2020

Fixes #541
This will support recursive maven,etc. projects of any depth and with or without git subrepositories.

@ukaszg
Copy link
Author

ukaszg commented Sep 30, 2020

will work on test errors after I figure out how to run travis locally

@joaotavora
Copy link
Owner

will work on test errors after I figure out how to run travis locally

I haven't had time to look at your pull request, but I can comment on this matter. Travis CI is broken for Eglot for a number of weeks now, but make check from the terminal should work fine if you have the programs installed. Eglot should skip any tests for servers that it doesn't find.

@ukaszg
Copy link
Author

ukaszg commented Sep 30, 2020

It seems that everything is working properly with eclipse.jdt.ls. I had problems with pyls, but once I disabled it, tests pass:

Ran 40 tests, 16 results as expected, 0 unexpected, 24 skipped (2020-09-30 11:04:37+0200, 8.276912 sec)

24 skipped results:
  SKIPPED  auto-detect-running-server
  SKIPPED  auto-reconnect
  SKIPPED  auto-shutdown
  SKIPPED  basic-completions
  SKIPPED  basic-diagnostics
  SKIPPED  basic-xref
  SKIPPED  eglot-eldoc-after-completions
  SKIPPED  eglot-ensure
  SKIPPED  eglot-lsp-abiding-column
  SKIPPED  eglot-multiline-eldoc
  SKIPPED  eglot-single-line-eldoc
  SKIPPED  javascript-basic
  SKIPPED  json-basic
  SKIPPED  python-autopep-formatting
  SKIPPED  python-yapf-formatting
  SKIPPED  rename-a-symbol
  SKIPPED  rls-hover-after-edit
  SKIPPED  rls-watches-files
  SKIPPED  slow-async-connection
  SKIPPED  slow-sync-connection-intime
  SKIPPED  slow-sync-connection-wait
  SKIPPED  slow-sync-timeout
  SKIPPED  snippet-completions
  SKIPPED  snippet-completions-with-company

@skangas
Copy link
Collaborator

skangas commented Jan 14, 2022

Could you link an example of a project where I could test your patch? It could be a real project you are working on or whatever, basically just some repository I could clone.

Thanks.

@ukaszg
Copy link
Author

ukaszg commented Jan 14, 2022

I was using my work code to test this, so no I can't. I've already switched to the other implementation. Just make any 3-level deep maven project or just delete this PR.

@skangas
Copy link
Collaborator

skangas commented Jan 14, 2022

I was using my work code to test this, so no I can't. I've already switched to the other implementation. Just make any 3-level deep maven project or just delete this PR.

I'd rather not delete the PR if it is useful. :-)

Please help me here, as I have forgotten all Java I once know (or so it feels). What is a "3-level deep maven project"? Does that refer to the directory structure? Does simple-java-maven-app fit the bill? Thanks.

@ukaszg
Copy link
Author

ukaszg commented Jan 14, 2022

ok, I'll prepare something. please give me some time.

@skangas
Copy link
Collaborator

skangas commented Jan 15, 2022

BTW, it would be even better if we could have a test case for this, of course.

@ukaszg
Copy link
Author

ukaszg commented Jan 15, 2022

Sorry I've never written unit tests for emacs. Here is a sample project https://github.com/ukaszg/java-test

The issue was, that the current implementation would find nested project "two" but wouldn't dig deep enough to find "three".

@skangas
Copy link
Collaborator

skangas commented Jan 15, 2022

Here is a sample project https://github.com/ukaszg/java-test

The issue was, that the current implementation would find nested project "two" but wouldn't dig deep enough to find "three".

Thanks, that helps! I'll look into it.

@joaotavora
Copy link
Owner

joaotavora commented Jan 18, 2022

I say this methaphoricaly, but this commit is a monster. @ukaszg how about coming up with a non-ELisp shell script that does these things and starts whatever java process with the correct options, consults environment variables, etc. Then you'd be doing not only Eglot but the whole LSP world a favor, as that script would work for a multitude of LSP clients. Also, the facilities you have available in the shell for dealing with such things are probably much better and less verbose than Elisp.

@nemethf
Copy link
Collaborator

nemethf commented Apr 4, 2022

I close this PR because it can't be merged anymore: the defmethod in question has been removed. See #868 and afe2e11.
Probably this is not even necessary anymore. @ukaszg, can you check it?

@nemethf nemethf closed this Apr 4, 2022
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.

[Feature request] support multi-module java projects
4 participants