-
Notifications
You must be signed in to change notification settings - Fork 229
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.
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
base: master
Are you sure you want to change the base?
Prevent IllegalStateException setting visible region with collapsed projection region #3456
Conversation
8067f5c to
883f03e
Compare
|
Test failures might be #2708 (comment) and #1517 |
This is actually (at least partially) fixed by #3457 (not sure why and it wasn't intentional but if it works, it works). However, it is still a bit weird in my tests so I'm keeping it as a draft. |
883f03e to
dde2668
Compare
5041a5d to
17cd4dd
Compare
1b4b381 to
c8b2d47
Compare
c8b2d47 to
ae256ad
Compare
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
94e8ec5 to
0b67a7f
Compare
projection region
6b907a8 to
bc02784
Compare
|
@iloveeclipse Would you or someone else like to review this and #3585? |
When collapsing projection regions and then setting the visible region in a certain way and then changing it to the surrounding visible region, it is possible that consecutive
ProjectionMapping#toImageLinecalls throw anIllegalStateException. This is becauseProjectionDocument#internalRemoveMasterDocumentRangeadds aPositionto the document when aPositionwith the same offset but a length of0can be present already. In this case,ISynchronizableDocument#addPosition(which is supposed to insert it into the sorted array) might insert it before thePositionresulting in the fragments being sorted incorrectly (the 0-length fragment after the positive-length fragment with both having the same offset). This can then result inIllegalStateExceptions when callingProjectionMapping#toImageLine(which happens while drawing line numbers for example).A similar issue also happens in
ProjectionDocument#internalAddMasterDocumentRange. There, it is possible that a region is expanded to the right in a way that it fully surrounds the region on the right. While expansion is normally handled bySegments (ranges in the slave document), theFragments are not correctly reordered in this case causingcomputeProjectedMasterRegions(int offsetInMaster, int lengthInMaster)not finding the region (at least that's what happened with my reproducer) and hence not being able to remove it from the editor. In the situation where oneFragmentincludes (is a "superset" of) another, it should be safe to delete the smaller one fixing the invalid state.This change fixes these problems.
Standalone reproducer/test
To reproduce the issue, run the test added with this PR without the changes to
ProjectionDocument.Without the fix, the tests throw exceptions similar to the following:
and
Reproducing with JDT (patch)
This can be reproduced with JDT using the code from this PR I wrote following these steps:
b()methoda()in the outline (does not happen with the other methods) to only show that methodTMclass in the outlineTMclass in the outline but instead theb()method, thec()method will be shown incorrectly.