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

WIP monitor gradle projects with kotlin build scripts #129

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lmmarsano
Copy link

@lmmarsano lmmarsano commented May 7, 2019

lsp-java wasn't importing gradle projects initialized to use kotlin build scripts.
As a result, lsp was indicating valid code as wrong, and it wasn't locating the project root/workspace.
This pull request fixes those issues for me.

I also exposed the list of possible project configuration files as a custom variable, since it might save users inconvenience when trying out new/experimental build tools.

Despite adding tests to cover these cases, however, the project's tests don't seem work at all, even on the continuous integration platform.
I'd recommend someone who's ever gotten ecukes to work (never has for me), to fix the tests.

Does this need further work?

add tests for gradle DSLs
- groovy
- kotlin
import fails for gradle kotlin DSL
convert `project-types` local variable to a custom variable for
extendability
include build.gradle.kts to detect all gradle projects
update glob patterns to watch these files
@yyoncho
Copy link
Member

yyoncho commented May 7, 2019

I guess this change is fixing your issue because you are not adding the proper project roots. Normally this method should not be used and it is leftover from the time jdt-ls was supporting only one root. You may verify that by doing lsp-describe-session - one of the roots should be the folder containing your build file.

@lmmarsano
Copy link
Author

Thanks.
Removing old workspace folders also seems to work.
Not sure what the issue was: switching back and forth between script languages deterministically broke and restored language validation, even though lsp-describe-session was listing the correct project root.
Since I can't reproduce the issue anymore, and this code shouldn't be relevant, I think we should close this request without merging.

@lmmarsano lmmarsano closed this May 8, 2019
@yyoncho
Copy link
Member

yyoncho commented May 8, 2019

We might still want to monitor the build file for changes, e. g. when you edit pom.xml file it notifies the server and the server refreshes the workspace.

@lmmarsano
Copy link
Author

Sure. Anything I should change or add?

@@ -1122,7 +1126,7 @@ PROJECT-URI uri of the item."
("registerOptions" (ht ("watchers"
(vector (ht ("globPattern" "**/*.java"))
(ht ("globPattern" "**/pom.xml"))
(ht ("globPattern" "**/*.gradle"))
(ht ("globPattern" "**/*.gradle{,.kts}"))
Copy link
Member

Choose a reason for hiding this comment

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

It is more about the testing - you may keep this line(although I am not sure whether this pattern works) and then modify the kts file(e. g. add new dependency) and then:

  1. Verify that lsp-java is sending update to the server
  2. Verify that jdt-ls is updating the workspace(you will start to receive notifications).

If 2) is not happening we have to file jdtls bug.

Copy link
Author

@lmmarsano lmmarsano May 8, 2019

Choose a reason for hiding this comment

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

Nothing happens with the line as written.
Should we consider this a bug?

As separate lines, however, client works, and server fails to update:

  • *lsp-log* shows workspace/didChangeWatchedFiles notifications
  • ~/.emacs.d/workspace/.metadata/.log shows !MESSAGE >> workspace/didChangeWatchedFiles but no message like !MESSAGE Updated java-test in 382 ms that shows with modification to groovy build files.
    When commenting out a dependency, imports continue to show as resolved in kotlin build file projects, though groovy build file projects get 'import cannot be resolved' updates.

Copy link
Member

Choose a reason for hiding this comment

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

IMO it is a bug. Can you check whether the change is applied if you restart the jdt ls server? If it works, that means the problem is in jdt-ls which does not process the file notification.

Copy link
Author

Choose a reason for hiding this comment

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

No change: commented out dependency still resolves after lsp-restart-workspace.
eclipse-jdtls/eclipse.jdt.ls#449 seems to indicate Kotlin build file support is missing.

@lmmarsano lmmarsano reopened this May 8, 2019
@lmmarsano lmmarsano changed the title detect & import gradle projects with kotlin build scripts WIP monitor gradle projects with kotlin build scripts May 8, 2019
@lmmarsano
Copy link
Author

eclipse-jdtls/eclipse.jdt.ls#449 seems relevant

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.

2 participants