search

eclipse / lsp4e

Language Server Protocol support in Eclipse IDE
Eclipse Public License 2.0
60 stars 53 forks source link

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

Closed ghentschke closed 5 months ago

ghentschke commented 5 months ago

The method ProjectionAnnotationModel.modifyAnnotations(Annotation[], Map<? extends Annotation, ? extends Position>, 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

rubenporras commented 5 months ago

Are we fixing a problem with ProjectionAnnotationModel.modifyAnnotations? Or how is it that "On large annotations the positions won't get updated properly which leads to removed folding annotations"?

ghentschke commented 5 months ago

Are we fixing a problem with ProjectionAnnotationModel.modifyAnnotations?

Not really. IMO we haven't used the proper method. And theProjectionAnnotationModel.modifyAnnotationPosition(foldingAnnotation, newPos) does exactly what we want: update the position of a Annotation.

rubenporras commented 5 months ago

Are we fixing a problem with ProjectionAnnotationModel.modifyAnnotations?

Not really. IMO we haven't used the proper method. And theProjectionAnnotationModel.modifyAnnotationPosition(foldingAnnotation, newPos) does exactly what we want: update the position of a Annotation.

I guess that you mean that call projectionAnnotationModel.modifyAnnotations(deletions.toArray(new Annotation[1]), additions, modifications.toArray(new Annotation[0])) is not the proper method. Why not?

ghentschke commented 5 months ago

I guess that you mean that call projectionAnnotationModel.modifyAnnotations(deletions.toArray(new Annotation[1]), additions, modifications.toArray(new Annotation[0])) is not the proper method. Why not?

That was my assumption I made after some debugging to determine why the folding annotations disappear as described in my issue #934. Unfortunately, I had not the time to determine the root cause why the previous method does not work on large files. The theProjectionAnnotationModel.modifyAnnotationPosition(foldingAnnotation, newPos) seems to be more appropriate for what we wanted and it works as expected also on large files.

ghentschke commented 5 months ago

@rubenporras thank you for quick review!