-
Notifications
You must be signed in to change notification settings - Fork 58
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Ensure that active highlights are removed when a source view is closed (
#855) Ensure that active highlights are removed when a source view is closed This commit resolves a bug related to erroneously persisted highlights in source files. It includes a unit test that verifies the correctness of the fix, and a few minor improvements to the testing framework. --- Bug Description: Before this change, source highlights would sometimes persist after a source view was closed. This would happen, for example, after the following sequence of steps: 1. Open a source. Ensure that some keyword is highlighted. 2. Activate the option "Toggle Split Editor (Horizontal)". 3. In the newly opened source view, highlight a different keyword. 4. At this point, both sets of highlights (the one from step 1 and the one from step 3) should be visible in both source views. This is correct behavior. 5. Close one of the source views, while keeping the other open. Before this commit, both sets of highlights would persist. This is incorrect: only the set of highlights related to the source view that remains open should be displayed. Cause: Each instance of HighlightReconcilingStrategy adds/removes highlights independently, and keeps track internally of all the highlights it has created. Before this commit, uninstalling a HighlightReconcilingStrategy did not remove the active highlights. Solution: When a source view is closed, all active highlights associated with its HighlightReconcilingStrategy should be removed. We accomplish this by calling `removeOccurrenceAnnotations()` during the `uninstall()` method of `HighlightReconcilingStrategy.java`. Testing: A unit test was created which approximately replicates the sequence of steps outlined above. To implement this unit test, the following changes were made in org.eclipse.lsp4e.test: 1. In HighlightTest.java, the function `assertAnnotationDoesNotExist` was created (for consistency, the existing function `assertHasAnnotation` was renamed to `assertAnnotationExists`). 2. The `mockDocumentHighlights` returned by the `MockTextDocumentService` are now `Position`-dependent. The unit tests in `HighlightTest.java` were adapted accordingly.
- Loading branch information
1 parent
a22947f
commit a339f14
Showing
8 changed files
with
156 additions
and
28 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ | |
| * Lucas Bullen (Red Hat Inc.) - Bug 508458 - Add support for codelens | ||
| * Pierre-Yves B. <[email protected]> - Bug 525411 - [rename] input field should be filled with symbol to rename | ||
| * Rubén Porras Campo (Avaloq Evolution AG) - Add support for willSaveWaitUntil. | ||
| * Joao Dinis Ferreira (Avaloq Group AG) - Add support for position-dependent mock document highlights | ||
| *******************************************************************************/ | ||
| package org.eclipse.lsp4e.tests.mock; | ||
|
|
||
|
|
@@ -21,6 +22,7 @@ | |
| import java.util.Arrays; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.ConcurrentLinkedQueue; | ||
| import java.util.function.Function; | ||
|
|
@@ -95,7 +97,7 @@ public class MockTextDocumentService implements TextDocumentService { | |
| private SignatureHelp mockSignatureHelp; | ||
| private List<CodeLens> mockCodeLenses; | ||
| private List<DocumentLink> mockDocumentLinks; | ||
| private List<? extends DocumentHighlight> mockDocumentHighlights; | ||
| private Map<Position, List<? extends DocumentHighlight>> mockDocumentHighlights; | ||
| private LinkedEditingRanges mockLinkedEditingRanges; | ||
|
|
||
| private CompletableFuture<DidOpenTextDocumentParams> didOpenCallback; | ||
|
|
@@ -165,8 +167,10 @@ public CompletableFuture<List<? extends Location>> references(ReferenceParams pa | |
| } | ||
|
|
||
| @Override | ||
| public CompletableFuture<List<? extends DocumentHighlight>> documentHighlight(DocumentHighlightParams position) { | ||
| return CompletableFuture.completedFuture(mockDocumentHighlights); | ||
| public CompletableFuture<List<? extends DocumentHighlight>> documentHighlight(DocumentHighlightParams params) { | ||
| return CompletableFuture.completedFuture((mockDocumentHighlights != null) // | ||
| ? mockDocumentHighlights.getOrDefault(params.getPosition(), Collections.emptyList()) | ||
| : null); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -390,7 +394,7 @@ public void setSignatureHelp(SignatureHelp signatureHelp) { | |
| this.mockSignatureHelp = signatureHelp; | ||
| } | ||
|
|
||
| public void setDocumentHighlights(List<? extends DocumentHighlight> documentHighlights) { | ||
| public void setDocumentHighlights(Map<Position, List<? extends DocumentHighlight>> documentHighlights) { | ||
| this.mockDocumentHighlights = documentHighlights; | ||
| } | ||
|
|
||
|
|
@@ -457,5 +461,5 @@ public void setFoldingRanges(List<FoldingRange> foldingRanges) { | |
| @Override | ||
| public CompletableFuture<List<FoldingRange>> foldingRange(FoldingRangeRequestParams params) { | ||
| return CompletableFuture.completedFuture(this.foldingRanges); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.