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

[Spring Boot Tools] Flag to configure the "JAVA_HOME or PATH..." message display #478

Closed
offer8 opened this issue Jun 11, 2020 · 11 comments
Closed
Assignees
Milestone

Comments

@offer8
Copy link

offer8 commented Jun 11, 2020

I understand that the extension requires JDK in order for the live hover mechanism to work.
Assuming it is not a must can the following message:

image

be configured by a flag to prevent displaying it on extensions activation.

@martinlippert martinlippert added this to the 4.7.0.RELEASE milestone Jun 11, 2020
@martinlippert
Copy link
Member

@BoykoAlex Can you take a look at this (for the VSCode extension)?

@BoykoAlex BoykoAlex self-assigned this Jun 11, 2020
@BoykoAlex
Copy link
Contributor

I'll add a preference setting "spring-boot.ls.checkJVM". It can be set either by modifying settings.json directly "spring-boot.ls.checkJVM": false or via VSCode.workspace.getConfiguration("spring-boot.ls")
How does this sound @offer8 ?
@kdvolder are you ok with it as well?

@kdvolder
Copy link
Member

@kdvolder are you ok with it as well?

Yes. But...

Unless it is hard/impossible to implement, I would prefer the error popup itself has a 'Do not show again' checkbox or something built in. If this popup is something that our language server launch code creates (rather than coming from vscode itself), then it shouldn't be too hard to implement that I think.

This would be better than having the user need to realize all by themselves that they can disable the annoying popup by adding a setting somewhere manually.

@offer8
Copy link
Author

offer8 commented Jun 11, 2020

@BoykoAlex sounds good.
@kdvolder we would like to prevent those kind of pop-ups if possible in advance.

@kdvolder
Copy link
Member

@kdvolder we would like to prevent those kind of pop-ups if possible in advance.

Presumably there will still be a 'setting' to record that the popup is disabled. So you could set it 'in advance' if you really wanted to. I was just thinking that most users probably won't care to disable a popup unless they saw it at least once. And at that point if they were annoyed by the popup they might not even realize there exists an obscure setting that can disable it.

@kdvolder
Copy link
Member

kdvolder commented Jun 11, 2020

That being said... I'm okay to go with the simpler solution of just having a setting with no extra widgetry in the popup. Avoid the extra work/complications of implementing this 'checkbox' in he UI if @offer8 who's requesting this feature doesn't really care about it anyway.

@BoykoAlex
Copy link
Contributor

Fixed with 08b1b4d

@kdvolder I like the idea of having "Do not show again" button on that warning message. Don't think it complicated anything - a bit of investigation into VSCode API only :-)

The setting is spring-boot.ls.checkJVM defaulted to true - can be changed programatically to false - I suspect that is your use case @offer8. The button Do not show again on the warning message is added and is simply another way to let the user tweak the setting.

@kdvolder
Copy link
Member

Just one little nitpick. While I'm sure its technically almost the same whether a setting is 'true' or 'false' by default. However, my experience is that it tends to be a bit of a source of potential bugs when the default value is 'true' and you have to explicitly set it to 'false' to disable. The reason for this is some code may accidentally treat absence of a setting as being false instead of true.

So, I've sort of come to the opinion/conclusion, that if possible, it is best to make newly added preferences so that their default value is actually false.

So maybe we can switch this setting to be something like spring-boot.ls.checkJVMdisabled which is 'false' by default.

I don't feel too strongly about this, you can probably come up with good arguments to do the opposite (like some folks might thing it illogical to set something to 'true' to disable something :-)

Anyhow @BoykoAlex, think about it and decide whether you agree.

@BoykoAlex
Copy link
Contributor

BoykoAlex commented Jun 11, 2020

@kdvolder I'd keep it as is... other settings follow the same convention - off == false, on == true.

@offer8
Copy link
Author

offer8 commented Jun 11, 2020

Thanks for quick fix!
The vscode release version for this one will be V_1.19.0?

@kdvolder
Copy link
Member

kdvolder commented Jun 11, 2020

The vscode release version for this one will be V_1.19.0?

Right, and if you want to try it early, you can download a snapshot .vsix file from the bottom of this page:
https://dist.springsource.com/snapshot/STS4/nightly-distributions.html

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

No branches or pull requests

4 participants