microsoft / vscode-java-dependency

Manage Java projects in Visual Studio Code.
https://marketplace.visualstudio.com/items?itemName=vscjava.vscode-java-dependency
MIT License
152 stars 74 forks source link

Exclude `**/node_modules/**` from build files search #794

Closed gluxon closed 4 months ago

gluxon commented 11 months ago

This is a strange PR and I would have no qualms with it being rejected.

Similar to https://github.com/microsoft/vscode-java-debug/pull/1234, I'm hoping to ignore node_modules to prevent large CPU usage from this extension when my teammates open a Node.js project. There's details in the linked pull request, but happy to elaborate if there's questions. Thanks!

jdneo commented 11 months ago

Hi @gluxon,

Thank you for contribution.

I understand your demand. What about reusing the value set by the setting java.import.exclusions?

The **/node_modules/** is set for that setting by default. And you can use this sample code to combine all of them into one glob pattern: https://github.com/redhat-developer/vscode-java/blob/f598b7938c771987b0da0f85225540ed9af6ffca/src/utils.ts#L100-L112

gluxon commented 11 months ago

Thanks for the suggestion! Would you also recommend copying over the java.import.exclusions settings schema from the vscode-java extension?

https://github.com/redhat-developer/vscode-java/blob/f598b7938c771987b0da0f85225540ed9af6ffca/package.json#L451

I would be worried about that schema getting out of sync between this repository and https://github.com/microsoft/vscode-java if we were to copy it.

jdneo commented 11 months ago

Would you also recommend copying over the java.import.exclusions settings schema from the vscode-java extension?

Sorry, do you mean duplicating that setting into this extension as well?

gluxon commented 11 months ago

Right! Would you recommend copying the java.import.exclusion schema from vscode-java to the configuration object here? https://github.com/microsoft/vscode-java-dependency/blob/b3d964bc24f23bcaf8fd42e8640ad4b83081bec0/package.json#L228

I also had a small question on whether it makes sense to reuse the java.import.exclusion configuration. We could also use a new config, but I think you would know better than I do what makes most sense.

https://github.com/redhat-developer/vscode-java/pull/3348#issuecomment-1776591738

jdneo commented 11 months ago

Thank you for the clarification.

I think we do not need to duplicate it, since that will confuse the users if they see two similar settings there.

There is very little chance that the schema will change for that setting. If you want to make sure no regression will happen in the future, maybe add a very simple test case - to verify that setting is an array. This can be a guard.

gluxon commented 9 months ago

Testing the proposed changes here: https://github.com/microsoft/vscode-java-dependency/commit/423b835dc14fb5b238c36a62b856b16cf3a6d931

This seems to work well when the vscode-java extension is installed.

Screenshot 2024-01-01 at 7 12 01 PM

But since if only vscode-java-dependency is installed (without vscode-java), an empty '' string gets read. That makes sense since VS Code isn't able to read the default values provided by vscode-java.

Screenshot 2024-01-01 at 7 14 16 PM

Is that okay? I think we would have to copy the java.import.exclusions settings schema into this extension otherwise, which we discussed not wanting to do earlier.

gluxon commented 4 months ago

@jdneo Is there a recommendation for the problem mentioned in the last comment? If this is too complicated to do, I can close this PR.