More checks for ignored markers #2540
Conversation
| @@ -170,6 +170,13 @@ private void collectNonJavaProblems(List<Diagnostic> diagnostics, boolean isDiag | |||
| } | |||
| list.add(marker); | |||
| } | |||
| list.removeIf(marker -> JavaLanguageServerPlugin.getPreferencesManager().getClientPreferences().excludedMarkerTypes().stream().anyMatch(t -> { | |||
There was a problem hiding this comment.
Why is this change needed ? At the very bottom of collectNonJavaProblems(..) there is a call to : https://github.com/eclipse/eclipse.jdt.ls/blob/c254d4226145152734100fe8120f71fd58876890/org.eclipse.jdt.ls.core/src/org/eclipse/jdt/ls/core/internal/handlers/BaseDiagnosticsHandler.java#L181 . The method handles list filtering via. excludedMarkerTypes, so why do it here when it's going to happen anyways ?
|
I re-evaluated the issue and I agree fixing it on the LS side on the moment doesn't seem the best place. So let's close in favor of improvements on client side. I may reopen one day with a more consolidated use-case explaining why this may be worth including. |
|
Fair. I did mention this location in #2524 (review) , so I was at least open to doing it in I think down the road using the diagnostic tags is going to be the best solution and filtering by those. I just haven't checked if other clients are really respecting that. |
|
I re-reevaluated this issue and I think it is necessary for eclipseide-jdtls, and can also improve the performance of JDT-LS.
|
mickaelistria
force-pushed
the
markers-2
branch
2 times, most recently
from
2b1be46
to
1901810
Compare
mickaelistria
changed the title
mickaelistria
force-pushed
the
markers-2
branch
from
1901810
to
637b127
Compare
Prevents extra work and extra notifications for excluded markers by quitting the visit ASAP.
| @@ -119,6 +121,9 @@ public void resourceChanged(IResourceChangeEvent event) { | |||
| */ | |||
| @Override | |||
| public boolean visit(IResourceDelta delta) throws CoreException { | |||
| if (delta.getFlags() == IResourceDelta.MARKERS && Arrays.stream(delta.getMarkerDeltas()).map(IMarkerDelta::getMarker).noneMatch(WorkspaceDiagnosticsHandler::isInteresting)) { | |||
There was a problem hiding this comment.
Should the check be ((delta.getFlags() & IResourceDelta.MARKERS) == 1) in case there are additional flags embedded ? (it's a bitmask).
There was a problem hiding this comment.
I don't think so; the goal here is to skip the visit only if it's about an ignored marker change. If the change is combined with another type of change (eg content), it's probably still worth visiting it.
No description provided.