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.

Sign up for GitHub

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

[#934] fix removed folding annotations on document change #935

Merged
merged 6 commits into from
Mar 4, 2024
Merged

[#934] fix removed folding annotations on document change #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da88ce5
[#934] fix removed folding annotations on document change
ghentschke Feb 28, 2024
a22ad82
Mark old method signature as deprecated
ghentschke Mar 4, 2024
c900e65
keep API stable
ghentschke Mar 4, 2024
9a307fe
Prevent old API
ghentschke Mar 4, 2024
084e662
Call old method to keep API stable
ghentschke Mar 4, 2024
4573ef1
Update comment
ghentschke Mar 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e/META-INF/MANIFEST.MF
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lsp4e/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
</parent>
<artifactId>org.eclipse.lsp4e</artifactId>
<packaging>eclipse-plugin</packaging>
<version>0.18.5-SNAPSHOT</version>
<version>0.18.6-SNAPSHOT</version>

<build>
<plugins>
Expand Down
32 changes: 24 additions & 8 deletions ...eclipse.lsp4e/src/org/eclipse/lsp4e/operations/folding/LSPFoldingReconcilingStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public void reconcile(IRegion subRegion) {
private void applyFolding(List<FoldingRange> ranges) {
// these are what are passed off to the annotation model to
// actually create and maintain the annotations
final var modifications = new ArrayList<Annotation>();
final var modifications = new ArrayList<Annotation>(); // not used anymore, can be removed later with the deprecated updateAnnotations method
final var deletions = new ArrayList<FoldingAnnotation>();
final var existing = new ArrayList<FoldingAnnotation>();
final var additions = new HashMap<Annotation, Position>();
Expand Down Expand Up @@ -253,13 +253,10 @@ private void updateAnnotation(List<Annotation> modifications, List<FoldingAnnota
* @param newPos
* the new position that caused the annotations need for updating and
* null otherwise.
* @param modifications
* the list of annotations to be modified
* @param deletions
* the list of annotations to be deleted
*/
protected void updateAnnotations(Annotation existingAnnotation, Position newPos, List<Annotation> modifications,
List<FoldingAnnotation> deletions) {
protected void updateAnnotations(Annotation existingAnnotation, Position newPos, List<FoldingAnnotation> deletions) {
if (existingAnnotation instanceof FoldingAnnotation foldingAnnotation) {
// if a new position can be calculated then update the position of
// the annotation,
Expand All @@ -269,16 +266,35 @@ 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);
Copy link
Contributor Author

@ghentschke ghentschke Feb 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

modifications can be removed from the arguments list of this method. But this would be an API break.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think there will be many subclasses of LSPFoldingReconcilingStrategy, so I would prefer that we modify the signature accordingly (or probably just remove the method, as it does not update anything any longer.)

@mickaelistria , what do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI Mickael is not available this week.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since Mickael is not available this week, could you mark the method as deprecated for removal and document on the javadoc that the parameter is unused? Then we can cleanup on a later iteration.

theProjectionAnnotationModel.modifyAnnotationPosition(foldingAnnotation, newPos);
}
} else {
deletions.add(foldingAnnotation);
}
}
}

/**
* 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<Annotation> modifications,
List<FoldingAnnotation> deletions) {
updateAnnotations(existingAnnotation, newPos, deletions);
}

/**
* <p>
* Searches the given {@link DirtyRegion} for annotations that now have a length
Expand Down