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

Match diagnostics with empty ranges #576

Merged
merged 1 commit into from
Mar 28, 2023

Conversation

mickaelistria
Copy link
Contributor

In Eclipse, we set a minimal range of 1 character, even for diagnostics that have an empty range (just a position); so this fixes ensure such markers and diagnostics are still matched together upon update, to avoid re-creating diagnostics.

@@ -207,7 +207,7 @@ private IMarker getExistingMarkerFor(IDocument document, Diagnostic diagnostic,
for (IMarker marker : remainingMarkers) {
try {
if (LSPEclipseUtils.toOffset(diagnostic.getRange().getStart(), document) == MarkerUtilities.getCharStart(marker)
&& LSPEclipseUtils.toOffset(diagnostic.getRange().getEnd(), document) == MarkerUtilities.getCharEnd(marker)
&& (LSPEclipseUtils.toOffset(diagnostic.getRange().getEnd(), document) == MarkerUtilities.getCharEnd(marker) || Objects.equals(diagnostic.getRange().getStart(), diagnostic.getRange().getEnd()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since when we create the marker, we already set the end position to be one char forward, would the existing condition not match already? Why do we need a new one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since when we create the marker, we already set the end position to be one char forward

Only in case of empty range

would the existing condition not match already?

No, it does not match this case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@mickaelistria
Copy link
Contributor Author

The test failure is actually caused by the MockLanguageServer being incorrect in some aspects and sending diagnostics from the wrong server. I'm working on improving this in a dedicated commit.

In Eclipse, we set a minimal range of 1 character, even for diagnostics
that have an empty range (just a position); so this fixes ensure such
markers and diagnostics are still matched together upon update, to avoid
re-creating diagnostics.
@mickaelistria mickaelistria merged commit 82dd320 into eclipse:master Mar 28, 2023
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.

None yet

2 participants