Skip to content

Prevent IllegalStateException setting visible region with collapsed projection region #3456

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+145 −1
Open

Prevent IllegalStateException setting visible region with collapsed projection region #3456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 17 additions & 1 deletion ...les/org.eclipse.text/projection/org/eclipse/jface/text/projection/ProjectionDocument.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package org.eclipse.jface.text.projection;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import org.eclipse.jface.text.AbstractDocument;
Expand Down Expand Up @@ -345,7 +346,9 @@ private void internalAddMasterDocumentRange(int offsetInMaster, int lengthInMast
int endOffset= offsetInMaster +lengthInMaster;
left.setLength(endOffset - left.getOffset());
left.segment.markForStretch();

if (index < fragments.length && fragments[index].getOffset() + fragments[index].getLength() < left.getOffset() + left.getLength()) {
((Fragment)fragments[index]).segment.delete();
}
} else if (right != null) {
right.setOffset(right.getOffset() - lengthInMaster);
right.setLength(right.getLength() + lengthInMaster);
Expand Down Expand Up @@ -438,6 +441,11 @@ private void internalRemoveMasterDocumentRange(int offsetInMaster, int lengthInM
segment= new Segment(offset, fragment.segment.getOffset() + fragment.segment.getLength() - offset);
newFragment.segment= segment;
segment.fragment= newFragment;
if (newFragment.getLength() != 0 && fMasterDocument.containsPosition(fFragmentsCategory, newFragment.getOffset(), 0)) {
// prevent inserting position with non-zero length after position with 0-length by removing zero-length position
removePositionAt(fMasterDocument, fFragmentsCategory, new Position(newFragment.getOffset(), 0));
removePositionAt(this, fSegmentsCategory, new Position(fMapping.toImageOffset(newFragment.getOffset()), 0));
}
fMasterDocument.addPosition(fFragmentsCategory, newFragment);
addPosition(fSegmentsCategory, segment);

Expand All @@ -454,6 +462,14 @@ private void internalRemoveMasterDocumentRange(int offsetInMaster, int lengthInM
}
}

private void removePositionAt(IDocument document, String category, Position toRemove) throws BadPositionCategoryException {
Position[] positions= document.getPositions(category);
int index= super.computeIndexInPositionList(Arrays.asList(positions), toRemove.getOffset(), true);
if (index != -1 && positions[index].getLength() == toRemove.getLength()) {
document.removePosition(category, positions[index]);
}
}

/**
* Returns the sequence of all master document regions which are contained
* in the given master document range and which are not yet part of this
Expand Down
128 changes: 128 additions & 0 deletions ...s/org.eclipse.jface.text.tests/src/org/eclipse/jface/text/tests/ProjectionViewerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
import org.eclipse.jface.text.BadLocationException;
import org.eclipse.jface.text.Document;
import org.eclipse.jface.text.IDocument;
import org.eclipse.jface.text.IDocumentInformationMapping;
import org.eclipse.jface.text.IRegion;
import org.eclipse.jface.text.ITextOperationTarget;
import org.eclipse.jface.text.ITextSelection;
import org.eclipse.jface.text.Position;
import org.eclipse.jface.text.Region;
import org.eclipse.jface.text.projection.ProjectionDocument;
import org.eclipse.jface.text.source.AnnotationModel;
import org.eclipse.jface.text.source.IOverviewRuler;
import org.eclipse.jface.text.source.IVerticalRuler;
Expand Down Expand Up @@ -433,6 +435,132 @@ public void testSetVisibleRegionExpandsBorderingProjectionRegions() {
}
}

@Test
public void testImageLineStateAfterSettingVisibleRegionsWithProjectionsSetMethodAndClass() throws BadLocationException {
// https://github.com/eclipse-platform/eclipse.platform.ui/pull/3456
Shell shell= new Shell();
shell.setLayout(new FillLayout());
TestProjectionViewer viewer= new TestProjectionViewer(shell, null, null, false, SWT.NONE);
String documentContent= """
public class TM {
void a() {
// ...
}

void b() {
// ...
}

void c() {
// ...
}
}
""";
Document document= new Document(documentContent);
viewer.setDocument(document, new AnnotationModel());
viewer.enableProjection();
addAnnotationBetween(viewer, new ProjectionAnnotation(false), "\tvoid a()", "\t}");
ProjectionAnnotation annotationToCollapse= new ProjectionAnnotation(false);
addAnnotationBetween(viewer, annotationToCollapse, "\tvoid b()", "\t}");
addAnnotationBetween(viewer, new ProjectionAnnotation(false), "\tvoid c()", "\t}");

shell.setVisible(true);
try {
viewer.getProjectionAnnotationModel().collapse(annotationToCollapse);

Position firstMethod= findPositionFromStartAndEndText(viewer, "\tvoid a()", "}");
viewer.setVisibleRegion(firstMethod.getOffset(), firstMethod.getLength() + 1);
viewer.setVisibleRegion(documentContent.indexOf("class"), documentContent.length() - documentContent.indexOf("class"));

IDocumentInformationMapping mapping= ((ProjectionDocument) viewer.getVisibleDocument()).getDocumentInformationMapping();
// toImageLine should not throw exceptions and yield the correct values
for (int i= 0; i < 5; i++) {
int imageLine= mapping.toImageLine(i);// should not throw exception
assertEquals(i, imageLine);
}
assertEquals(-1, mapping.toImageLine(6), "should still be collapsed");
for (int i= 7; i < documentContent.split("\n").length; i++) {
int imageLine= mapping.toImageLine(i);// should not throw exception
assertEquals(i - 1, imageLine);
}
} finally {
shell.dispose();
}
}

@Test
public void testImageLineStateAfterSettingVisibleRegionsWithProjectionsSetDifferentMethods() throws BadLocationException {
// https://github.com/eclipse-platform/eclipse.platform.ui/pull/3456
Shell shell= new Shell();
shell.setLayout(new FillLayout());
TestProjectionViewer viewer= new TestProjectionViewer(shell, null, null, false, SWT.NONE);
String documentContent= """
public class TM {
void a() {
// ...
}

void b() {
// ...
}

void c() {
// ...
}
}
""";
Document document= new Document(documentContent);
viewer.setDocument(document, new AnnotationModel());
viewer.enableProjection();
addAnnotationBetween(viewer, new ProjectionAnnotation(false), "\tvoid a()", "\t}");
ProjectionAnnotation annotationToCollapse= new ProjectionAnnotation(false);
addAnnotationBetween(viewer, annotationToCollapse, "\tvoid b()", "\t}");
addAnnotationBetween(viewer, new ProjectionAnnotation(false), "\tvoid c()", "\t}");

shell.setVisible(true);
try {
viewer.getProjectionAnnotationModel().collapse(annotationToCollapse);

Position firstMethod= findPositionFromStartAndEndText(viewer, "\tvoid a()", "}");
viewer.setVisibleRegion(firstMethod.getOffset(), firstMethod.getLength() + 1);
Position secondMethod= findPositionFromStartAndEndText(viewer, "\tvoid b()", "}");
viewer.setVisibleRegion(secondMethod.getOffset(), secondMethod.getLength());

// the '}' is cut off because this test doesn't include it
assertEquals("""
void b() {
// ...
""", viewer.getVisibleDocument().get());

IDocumentInformationMapping mapping= ((ProjectionDocument) viewer.getVisibleDocument()).getDocumentInformationMapping();

// there should be no image regions outside of the visible region

assertEquals(0, mapping.toImageLine(5));
assertEquals(1, mapping.toImageLine(6));
for (int i= 0; i < documentContent.split("\n").length; i++) {
if (i < 5 || i > 6) {
assertEquals(-1, mapping.toImageLine(i));
}
}
} finally {
shell.dispose();
}
}

private void addAnnotationBetween(TestProjectionViewer viewer, ProjectionAnnotation annotationToCollapse, String startText, String endText) {
Position position= findPositionFromStartAndEndText(viewer, startText, endText);
viewer.getProjectionAnnotationModel().addAnnotation(annotationToCollapse, position);
}

private Position findPositionFromStartAndEndText(TestProjectionViewer viewer, String startText, String endText) {
String documentContent= viewer.getDocument().get();
int startIndex= documentContent.indexOf(startText);
int endIndex= documentContent.indexOf(endText, startIndex + 1);
Position position= new Position(startIndex, endIndex - startIndex);
return position;
}

@Test
public void testProjectionRegionsShownOnlyInVisibleRegion() {
Shell shell= new Shell(Display.getCurrent());
Expand Down
Loading