Generate code action to ignore certain compiler issues. #1765
Conversation
org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/ChangeUtil.java
Show resolved
Hide resolved
org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/CodeActionHandler.java
Outdated
Show resolved
Hide resolved
rgrunber
force-pushed
the
ignore-squiggles
branch
2 times, most recently
from
fd67dc4
to
2a344d7
Compare
rgrunber
force-pushed
the
ignore-squiggles
branch
from
2a344d7
to
1bc54d4
Compare
|
|
rgrunber
force-pushed
the
ignore-squiggles
branch
from
1bc54d4
to
1b70c2b
Compare
| String compilerOption = CompilerOptions.optionKeyFromIrritant(irritant); | ||
| if (compilerOption != null) { | ||
| IJavaProject proj = unit.getJavaProject(); | ||
| IFile settingsFile = proj.getProject().getFolder(ProjectUtils.WORKSPACE_LINK).getFolder(ProjectUtils.SETTINGS).getFile("org.eclipse.jdt.core.prefs"); |
There was a problem hiding this comment.
It just ignores this compiler problem at the project level. I prefer to have an extra menu to ignore it at workspace level.
There was a problem hiding this comment.
Also this will create a _/.settings file in my workspace when i try it in a maven project.
There was a problem hiding this comment.
It just ignores this compiler problem at the project level. I prefer to have an extra menu to ignore it at workspace level.
Are you referring to a multi-root workspace, or do you want to have the setting get persisted to java.settings.url, if it exists ?
The "_/.settings" in a Maven project is a bug and will be fixed.
There was a problem hiding this comment.
Are you referring to a multi-root workspace, or do you want to have the setting get persisted to
java.settings.url, if it exists ?
If it's multi-module maven/gradle projects, the user has to suppress the same compiler problem at each project one by one. That's not convenient.
Persistence can be a challenge. If a global setting exists, it would be nice to add new setting there. The question is, how do you handle a global setting if it was not previously set? Is it possible to create a new setting file in current workspace (e.g. .vscode/java.prefs) and update client configuration java.settings.url to that file?
There was a problem hiding this comment.
I agree, it would be really nice to have two different actions: one for ignoring in the current workspace, and one for ignoring in all workspaces.
The question is, how do you handle a global setting if it was not previously set?
I've thought about this, and I think the main problem is introduced by the fact that java.settings.url is allowed to be unset. That requires us to somehow prompt the user about creating one.
Couldn't we instead introduce some logic in the VS Code extension to send a default settings url to the server if not explicitely specified in java.settings.url? That is to say, if java.settings.url is unset (or maybe introduce a special variable, like ${extensionStorage}), the client will send something like globalStorageUri/org.eclipse.jdt.ls.prefs as the value to the server (creating the file if it doesn't exist). And we could also add a command for the command palette to open said file (for easy access). Something like Java: Open Language Server Preferences or Java: Open Global Preferences File.
Of course, that's specific to the VS Code client, but as a fallback we could just send an error message saying that the user needs to specify a valid java.settings.url.
What do you think?
There was a problem hiding this comment.
Oh, and I just came to think of another problem: it seems like java.settings.url is currently allowed to be a web URL, so we can never guarantee it's writable. Honestly I think it would be better to require it to be a file path instead of a URL, but changing that would be a breaking change I guess. The only benefit of allowing web URLs as I see it is that you can easily "import" settings, but I would rather see that implemented as a command that downloads said file to the configured file path. It being a URL can also be confusing for users, a file path is simpler to configure.
Introducing such a change might be out of scope for this PR though, so I think an acceptable alternative would be to provide an error message saying that the URL is not a writable file.
There was a problem hiding this comment.
Currently, when java.settings.url is set, this will append the necessary value there. What I don't currently support is creating a local settings file when none is set.
Could we improve the language server to accept a non-existent file-path of java.settings.url ? This allows us to optionally create it should we need to (eg. this particular case here). Clients would have to set a default java.settings.url value or not get the code action.
The only question is do we want to do it in this PR or have it be a separate one.
- Fixes redhat-developer/vscode-java#1791 - Some JDT core compiler problems can be ignored using the setting preference file, so offer a code action to do this for a given error/warning - Code action appears only when a `java.settings.url` is detected that is on the local filesystem - Add testcase Signed-off-by: Roland Grunberg <rgrunber@redhat.com>
rgrunber
force-pushed
the
ignore-squiggles
branch
from
1b70c2b
to
0cdf490
Compare
preference file, so offer a code action to do this for a given
error/warning
Signed-off-by: Roland Grunberg rgrunber@redhat.com