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

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

Closed

Conversation

gluxon
Copy link

@gluxon gluxon commented Oct 20, 2023

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

Similar to microsoft/vscode-java-debug#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
Copy link
Member

jdneo commented Oct 22, 2023

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
Copy link
Author

gluxon commented Oct 24, 2023

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
Copy link
Member

jdneo commented Oct 24, 2023

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
Copy link
Author

gluxon commented Oct 24, 2023

Right! Would you recommend copying the java.import.exclusion schema from vscode-java to the configuration object here?

"configuration": {

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.

redhat-developer/vscode-java#3348 (comment)

@jdneo
Copy link
Member

jdneo commented Oct 24, 2023

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
Copy link
Author

gluxon commented Jan 2, 2024

Testing the proposed changes here: 423b835

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
Copy link
Author

gluxon commented May 23, 2024

@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.

@gluxon gluxon closed this May 23, 2024
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