From da88ce5bc6ac6a01a485a99284c210d53a7b1b19 Mon Sep 17 00:00:00 2001 From: Gesa HENTSCHKE Date: Wed, 28 Feb 2024 12:04:18 +0100 Subject: [PATCH 1/6] [#934] fix removed folding annotations on document change The method ProjectionAnnotationModel.modifyAnnotations(Annotation[], Map, Annotation[]) does not consider Position changes in any cases. On large annotations the positions won't get updated properly which leads to removed folding annotations. fixes #934 --- .../operations/folding/LSPFoldingReconcilingStrategy.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java index b37201b03..a2fa08b3a 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java @@ -269,9 +269,7 @@ protected void updateAnnotations(Annotation existingAnnotation, Position newPos, Position oldPos = theProjectionAnnotationModel.getPosition(foldingAnnotation); // only update the position if we have to if (!newPos.equals(oldPos)) { - oldPos.setOffset(newPos.offset); - oldPos.setLength(newPos.length); - modifications.add(foldingAnnotation); + theProjectionAnnotationModel.modifyAnnotationPosition(foldingAnnotation, newPos); } } else { deletions.add(foldingAnnotation); From a22ad8267438e7768b914aadea289cd69c95af57 Mon Sep 17 00:00:00 2001 From: Gesa HENTSCHKE Date: Mon, 4 Mar 2024 09:58:56 +0100 Subject: [PATCH 2/6] Mark old method signature as deprecated - Users should use the new method with updated signature. --- org.eclipse.lsp4e/META-INF/MANIFEST.MF | 2 +- org.eclipse.lsp4e/pom.xml | 2 +- .../LSPFoldingReconcilingStrategy.java | 37 +++++++++++++------ 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/org.eclipse.lsp4e/META-INF/MANIFEST.MF b/org.eclipse.lsp4e/META-INF/MANIFEST.MF index 5826694f7..9462bedbd 100644 --- a/org.eclipse.lsp4e/META-INF/MANIFEST.MF +++ b/org.eclipse.lsp4e/META-INF/MANIFEST.MF @@ -2,7 +2,7 @@ Manifest-Version: 1.0 Bundle-ManifestVersion: 2 Bundle-Name: Language Server Protocol client for Eclipse IDE (Incubation) Bundle-SymbolicName: org.eclipse.lsp4e;singleton:=true -Bundle-Version: 0.18.5.qualifier +Bundle-Version: 0.18.6.qualifier Bundle-RequiredExecutionEnvironment: JavaSE-17 Require-Bundle: org.eclipse.core.runtime;bundle-version="3.12.0", org.eclipse.equinox.common;bundle-version="3.8.0", diff --git a/org.eclipse.lsp4e/pom.xml b/org.eclipse.lsp4e/pom.xml index 7a622805f..cd56c3ad6 100644 --- a/org.eclipse.lsp4e/pom.xml +++ b/org.eclipse.lsp4e/pom.xml @@ -7,7 +7,7 @@ org.eclipse.lsp4e eclipse-plugin - 0.18.5-SNAPSHOT + 0.18.6-SNAPSHOT diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java index a2fa08b3a..3565d4c2a 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java @@ -139,7 +139,6 @@ public void reconcile(IRegion subRegion) { private void applyFolding(List ranges) { // these are what are passed off to the annotation model to // actually create and maintain the annotations - final var modifications = new ArrayList(); final var deletions = new ArrayList(); final var existing = new ArrayList(); final var additions = new HashMap(); @@ -151,7 +150,7 @@ private void applyFolding(List ranges) { if (ranges != null) { Collections.sort(ranges, Comparator.comparing(FoldingRange::getEndLine)); for (FoldingRange foldingRange : ranges) { - updateAnnotation(modifications, deletions, existing, additions, foldingRange.getStartLine(), + updateAnnotation(deletions, existing, additions, foldingRange.getStartLine(), foldingRange.getEndLine(), FoldingRangeKind.Imports.equals(foldingRange.getKind())); } } @@ -167,8 +166,7 @@ private void applyFolding(List ranges) { } // send the calculated updates to the annotations to the // annotation model - theProjectionAnnotationModel.modifyAnnotations(deletions.toArray(new Annotation[1]), additions, - modifications.toArray(new Annotation[0])); + theProjectionAnnotationModel.modifyAnnotations(deletions.toArray(new Annotation[1]), additions, new Annotation[0]); } } @@ -230,15 +228,14 @@ public void projectionEnabled() { * the end line number * @throws BadLocationException */ - private void updateAnnotation(List modifications, List deletions, - List existing, Map additions, int line, Integer endLineNumber, boolean collapsedByDefault) + private void updateAnnotation(List deletions, List existing, Map additions, int line, Integer endLineNumber, boolean collapsedByDefault) throws BadLocationException { int startOffset = document.getLineOffset(line); int endOffset = document.getLineOffset(endLineNumber) + document.getLineLength(endLineNumber); final var newPos = new Position(startOffset, endOffset - startOffset); if (!existing.isEmpty()) { FoldingAnnotation existingAnnotation = existing.remove(existing.size() - 1); - updateAnnotations(existingAnnotation, newPos, modifications, deletions); + updateAnnotations(existingAnnotation, newPos, deletions); } else { additions.put(new FoldingAnnotation(collapsedByDefault), newPos); } @@ -253,13 +250,10 @@ private void updateAnnotation(List modifications, List modifications, - List deletions) { + protected void updateAnnotations(Annotation existingAnnotation, Position newPos, List deletions) { if (existingAnnotation instanceof FoldingAnnotation foldingAnnotation) { // if a new position can be calculated then update the position of // the annotation, @@ -277,6 +271,27 @@ protected void updateAnnotations(Annotation existingAnnotation, Position newPos, } } + /** + * Update annotations. + * + * @param existingAnnotation + * the existing annotations that need to be updated based on the + * given dirtied IndexRegion + * @param newPos + * the new position that caused the annotations need for updating and + * null otherwise. + * @param modifications + * the list of annotations to be modified - not used anymore, that's why this method is deprecated. + * @param deletions + * the list of annotations to be deleted + * @deprecated use {@link LSPFoldingReconcilingStrategy#updateAnnotations(Annotation, Position, List)} + */ + @Deprecated(since = "0.18.6", forRemoval = true) + protected void updateAnnotations(Annotation existingAnnotation, Position newPos, List modifications, + List deletions) { + updateAnnotations(existingAnnotation, newPos, deletions); + } + /** *

* Searches the given {@link DirtyRegion} for annotations that now have a length From c900e65b9d10196ad16d5e27d6779734db8db4c7 Mon Sep 17 00:00:00 2001 From: Gesa HENTSCHKE Date: Mon, 4 Mar 2024 10:58:45 +0100 Subject: [PATCH 3/6] keep API stable --- .../operations/folding/LSPFoldingReconcilingStrategy.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java index 3565d4c2a..6194eef9d 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java @@ -139,6 +139,7 @@ public void reconcile(IRegion subRegion) { private void applyFolding(List ranges) { // these are what are passed off to the annotation model to // actually create and maintain the annotations + final var modifications = new ArrayList(); // not used anymore final var deletions = new ArrayList(); final var existing = new ArrayList(); final var additions = new HashMap(); @@ -150,7 +151,7 @@ private void applyFolding(List ranges) { if (ranges != null) { Collections.sort(ranges, Comparator.comparing(FoldingRange::getEndLine)); for (FoldingRange foldingRange : ranges) { - updateAnnotation(deletions, existing, additions, foldingRange.getStartLine(), + updateAnnotation(modifications, deletions, existing, additions, foldingRange.getStartLine(), foldingRange.getEndLine(), FoldingRangeKind.Imports.equals(foldingRange.getKind())); } } @@ -228,7 +229,8 @@ public void projectionEnabled() { * the end line number * @throws BadLocationException */ - private void updateAnnotation(List deletions, List existing, Map additions, int line, Integer endLineNumber, boolean collapsedByDefault) + private void updateAnnotation(List modifications, List deletions, + List existing, Map additions, int line, Integer endLineNumber, boolean collapsedByDefault) throws BadLocationException { int startOffset = document.getLineOffset(line); int endOffset = document.getLineOffset(endLineNumber) + document.getLineLength(endLineNumber); From 9a307fe70144c4687523c18572a69c54d8f998de Mon Sep 17 00:00:00 2001 From: Gesa HENTSCHKE Date: Mon, 4 Mar 2024 11:33:15 +0100 Subject: [PATCH 4/6] Prevent old API --- .../operations/folding/LSPFoldingReconcilingStrategy.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java index 6194eef9d..80e3aac9b 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java @@ -167,7 +167,8 @@ private void applyFolding(List ranges) { } // send the calculated updates to the annotations to the // annotation model - theProjectionAnnotationModel.modifyAnnotations(deletions.toArray(new Annotation[1]), additions, new Annotation[0]); + theProjectionAnnotationModel.modifyAnnotations(deletions.toArray(new Annotation[1]), additions, + modifications.toArray(new Annotation[0])); } } From 084e662857d4404981f3d0c9685b185dd568ce45 Mon Sep 17 00:00:00 2001 From: Gesa HENTSCHKE Date: Mon, 4 Mar 2024 11:38:12 +0100 Subject: [PATCH 5/6] Call old method to keep API stable --- .../lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java index 80e3aac9b..4105781c2 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java @@ -238,7 +238,7 @@ private void updateAnnotation(List modifications, List Date: Mon, 4 Mar 2024 11:40:29 +0100 Subject: [PATCH 6/6] Update comment --- .../lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java index 4105781c2..9e25df966 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java @@ -139,7 +139,7 @@ public void reconcile(IRegion subRegion) { private void applyFolding(List ranges) { // these are what are passed off to the annotation model to // actually create and maintain the annotations - final var modifications = new ArrayList(); // not used anymore + final var modifications = new ArrayList(); // not used anymore, can be removed later with the deprecated updateAnnotations method final var deletions = new ArrayList(); final var existing = new ArrayList(); final var additions = new HashMap();