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.

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

ProjectionAnnotationModel.expandAll should tolerate the empty selection #2361

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

merks
Copy link
Contributor

@merks merks commented Oct 4, 2024

  • ITextSelection.emptySelection has offset and length -1 so those values need to be tolerated downstream, included in the recent-optimized ProjectionAnnotationModel.expandAll method.

#2257

- ITextSelection.emptySelection has offset and length -1 so those values
need to be tolerated downstream, included in the recent-optimized
ProjectionAnnotationModel.expandAll method.

eclipse-platform#2257
@merks
Copy link
Contributor Author

merks commented Oct 4, 2024

@iloveeclipse

FYI, the optimization to iterate over the range of annotations has caused test failures in the I-Builds where it's setting the empty selection into a text viewer/editor:

org.eclipse.core.runtime.AssertionFailedException: assertion failed:
at org.eclipse.core.runtime.Assert.isTrue(Assert.java:119)
at org.eclipse.core.runtime.Assert.isTrue(Assert.java:104)
at org.eclipse.jface.text.Position.<init>(Position.java:65)
at org.eclipse.jface.text.AbstractDocument.getPositions(AbstractDocument.java:1599)
at org.eclipse.core.internal.filebuffers.SynchronizableDocument.getPositions(SynchronizableDocument.java:246)
at org.eclipse.jface.text.source.AnnotationModel.getRegionAnnotationIterator(AnnotationModel.java:730)
at org.eclipse.jface.text.source.AnnotationModel.getAnnotationIterator(AnnotationModel.java:692)
at org.eclipse.jface.text.source.projection.ProjectionAnnotationModel.expandAll(ProjectionAnnotationModel.java:152)
at org.eclipse.jface.text.source.projection.ProjectionAnnotationModel.expandAll(ProjectionAnnotationModel.java:103)
at org.eclipse.jface.text.source.projection.ProjectionViewer.exposeModelRange(ProjectionViewer.java:1256)
at org.eclipse.ui.texteditor.AbstractTextEditor.adjustHighlightRange(AbstractTextEditor.java:6274)
at org.eclipse.ui.texteditor.AbstractTextEditor.selectAndReveal(AbstractTextEditor.java:6307)
at org.eclipse.ui.texteditor.AbstractTextEditor.selectAndReveal(AbstractTextEditor.java:6282)
at org.eclipse.ui.texteditor.AbstractTextEditor.doSetSelection(AbstractTextEditor.java:2947)
at org.eclipse.ant.internal.ui.editor.AntEditor.doSetSelection(AntEditor.java:1133)
at org.eclipse.ui.texteditor.AbstractTextEditor$SelectionProvider.setSelection(AbstractTextEditor.java:1442)
at org.eclipse.ant.tests.ui.editor.CodeCompletionTest.testEmptyBuildfileProposals(CodeCompletionTest.java:1128)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at org.eclipse.ant.tests.ui.testplugin.TestAgainExceptionRule$1.evaluate(TestAgainExceptionRule.java:39) 

Copy link
Contributor

github-actions bot commented Oct 4, 2024

Test Results

 1 815 files  ±0   1 815 suites  ±0   1h 34m 46s ⏱️ + 3m 31s
 7 704 tests ±0   7 475 ✅ ±0  228 💤 ±0  1 ❌ ±0 
24 273 runs  ±0  23 525 ✅ ±0  747 💤 ±0  1 ❌ ±0 

For more details on these failures, see this check.

Results for commit 4a4feb7. ± Comparison against base commit 73b5314.

@merks
Copy link
Contributor Author

merks commented Oct 4, 2024

@akurtakov

I don't think the test status has changed. Should this block merging or is it okay to merge?

@akurtakov
Copy link
Member

I check jenkins. The rest are unstable and no one pays attention to them.

@merks
Copy link
Contributor Author

merks commented Oct 4, 2024

I'll assume this is not a controversial change.

@merks merks merged commit ebbc32d into eclipse-platform:master Oct 4, 2024
15 of 17 checks passed
@merks merks deleted the pr-regression-2257 branch October 4, 2024 10:41
@iloveeclipse
Copy link
Member

FYI, the optimization to iterate over the range of annotations has caused test failures in the I-Builds where it's setting the empty selection into a text viewer/editor:

Thanks Ed for fixing, I'm currently only two days in the week online and didn't noticed that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants