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

Add support for subfolder gradle projects #1336

Merged
merged 3 commits into from
May 25, 2023

Conversation

gayanper
Copy link
Contributor

In the multi module setup where the parent folder is open by vscode, when wrapper is enabled, the LS fails the resolved the wrapper since it looks in workspace folder which is the root folder of the multi module project structure. The wrapper folders are actually in leaf level folders where the gradle files are found. So this fix try to resolve wrapper consider the current gradle file path to resolve it first, if that fails it falls back to workspace folder as before.

When resolving wrapper consider the current gradle file path to resolve it.
@gayanper
Copy link
Contributor Author

@jdneo @testforstephen WDYT ?

@jdneo
Copy link
Member

jdneo commented May 18, 2023

@gayanper I'll take a look recently. To help me understand the case that this PR fixes, could you share a screenshot showing the folder structure of the workspace we are talking about?

@testforstephen
Copy link
Contributor

In the multi module setup where the parent folder is open by vscode, when wrapper is enabled, the LS fails the resolved the wrapper since it looks in workspace folder which is the root folder of the multi module project structure.

If the language server failed to locate the correct wrapper, the fix should be applied in eclipse.jdt.ls as well. jdt.ls has its own logic for finding the gradle distribution. This repo vscode-repo is an independent project and does not interact with jdt.ls.

@gayanper
Copy link
Contributor Author

If the language server failed to locate the correct wrapper, the fix should be applied in eclipse.jdt.ls as well. jdt.ls has its own logic for finding the gradle distribution. This repo vscode-repo is an independent project and does not interact with jdt.ls.

jdt.ls works without issues. The reason might be the project import er traverse the folder structure to find gradle files recursively. And those found paths are used by gradle eclipse tooling to import projects into workspace.

@gayanper
Copy link
Contributor Author

@gayanper I'll take a look recently. To help me understand the case that this PR fixes, could you share a screenshot showing the folder structure of the workspace we are talking about?

image

If i open the gradle-spring folder, the language server only provides the basic closures. But if i open the demo folder inside the gradle-spring then all plugin closures are provided.

@jdneo
Copy link
Member

jdneo commented May 19, 2023

I tried to open workspace contains gradle project in a subfolder, but it does not give me completion for a jacoco closure. Do I miss anything?

gradle.mp4

@gayanper
Copy link
Contributor Author

@jdneo when this issues happens you won't get inbuilt gradle plugin completions even. For example java, test and also dependency and different dependency configuration completions.

Could you try on those with and without fix on same setup?

@jdneo
Copy link
Member

jdneo commented May 19, 2023

Not sure if it is due to something wrong with my local setup. I'll ask @CsCherrYY later.

@gayanper
Copy link
Contributor Author

@gayanper
Copy link
Contributor Author

@gayanper
Copy link
Contributor Author

@jdneo hope the above screenshots makes it more clear

@gayanper
Copy link
Contributor Author

@jdneo @testforstephen I would say if we support multi-root workspace folders, then we should try to fix this. I found few more such extension which doesn't work in the way i explained above. I think vscode expect to use multi-root workspace folders in such situations like this https://medium.com/rewrite-tech/visual-studio-code-tips-for-monorepo-development-with-multi-root-workspaces-and-extension-6b69420ecd12

@gayanper
Copy link
Contributor Author

@jdneo @testforstephen I would say if we support multi-root workspace folders, then we should try to fix this. I found few more such extension which doesn't work in the way i explained above. I think vscode expect to use multi-root workspace folders in such situations like this https://medium.com/rewrite-tech/visual-studio-code-tips-for-monorepo-development-with-multi-root-workspaces-and-extension-6b69420ecd12

We can close this if you also agree, The extension works as it is with multi-root workspaces.

@jdneo
Copy link
Member

jdneo commented May 22, 2023

I would say if we support multi-root workspace folders, then we should try to fix this.

Yes, I agree.

Today I tried to setup the project on another machine and now everything works as expected. Not sure what's wrong happened days ago...

Copy link
Member

@jdneo jdneo left a comment

Choose a reason for hiding this comment

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

Overall, the change looks good to me. I found another problem when trying the change, but that's another problem which we can address in another PR in the future.


Update: There is already an issue tracking this problem. #1237

@jdneo jdneo added the bug Something isn't working label May 22, 2023
@jdneo
Copy link
Member

jdneo commented May 23, 2023

@gayanper I think this PR can be merged after the variable name changes. :)

Copy link
Member

@jdneo jdneo left a comment

Choose a reason for hiding this comment

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

Thank you @gayanper

@jdneo jdneo merged commit 653c096 into microsoft:develop May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants