Summary: | [formatter] Code formatter should adapt edits instead of regions | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Eclipse Project] JDT | Reporter: | Frederic Fusier <frederic_fusier> | ||||||
Component: | Core | Assignee: | Frederic Fusier <frederic_fusier> | ||||||
Status: | VERIFIED FIXED | QA Contact: | |||||||
Severity: | normal | ||||||||
Priority: | P2 | CC: | benno.baumgartner, daniel_megert, david_audel, reto.urfer, schoenbach, zvikico | ||||||
Version: | 3.4 | ||||||||
Target Milestone: | 3.5 M2 | ||||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
Whiteboard: | |||||||||
Attachments: |
|
Description
Frederic Fusier
2008-05-29 07:19:28 EDT
I think this must be fixed for 3.4.1 This adaptation was needed by the fact that the whole snippet is formatted and only edits belonging to the given regions should be applied. The problem was for edits which overlap the regions... The solution for this problem is in fact quite simple: the Scribe should apply only the part of the edit included in the region. The initial implementation of this solution was to adapt the region to include the edits completely... The new solution will be to split the concerned edits into 2 or 3 parts depending on how the overlapping occurs. + case 1 <------ edit ------> <------- region --------> then the edit needs to be split in 2 parts: <-edit1-><- edit2 -> <------- region --------> + case 2 <----- edit -----> <----- region ------> then the edit needs to be split in 2 parts: <-edit1-><-edit2-> <----- region ------> + case 3 <--------------- edit ----------------> <----- region ------> then the edit needs to be split in 2 parts: <-edit1-><-------edit2-------><-edit3-> <----- region ------> + all other cases <--- edit ---> <--- edit ---> <--- edit ---> <----- region ------> then there's no need to split the edit Note that the split of edits is not something obvious after having formatted the whole source (i.e while building the root edit). The difficulty is to retrieve the exact position of the region offset inside the edit as the original source is not stored in it, only its offset and length. To solve this, we can either split the edit at the time we create it or, if we want to keep the split at the end of the scribe formatting, accept to split only on line ends (as we have the lineEnds table in hand)... *** Bug 239447 has been marked as a duplicate of this bug. *** *** Bug 237592 has been marked as a duplicate of this bug. *** *** Bug 237453 has been marked as a duplicate of this bug. *** Created attachment 107111 [details]
First proposed patch
Not finalized yet but the approach used in this patch looks promising :-)
First it removes all previous regions adapting except for block and javadoc comments which unfortunately modify not only whitespaces.
For all other cases, we would have an adaption of edits when they overlap either the beginning or the end of a region.
When the overlap occurs at the beginning of the region, the rule would be to restart the edit at the beginning of the line which contains the region start and apply the end of the replacement string (after the last new line).
When the overlap occurs at the end of the region, the rule would be to apply the number of new lines between the region end and the edit end, but no more than the number contained in the replacement string.
This algorithm fixes this bug and all the duplicate ones, but introduces 37 failures while running all formatter tests. After a quick look, it looks like the output of the failing tests were not valid, but I lack of time to verify them one by one.
I'll restart on this when I come back from vacations...
Defer to 3.4.2 as code freeze for 3.4.1 is tomorrow... *** Bug 237592 has been marked as a duplicate of this bug. *** Created attachment 111293 [details] New proposed patch New patch fixing the problem and (real) duplicate without any failures on JDT tests (Core, UI and Text). The edits overlapping described in comment 2 is still valid, but the edit modification has been changed since the first proposed patch (comment 6). In this patch, it is done as follow: 1) start the edit from the region start if it overlaps the region's start 2) end the edit at the region end if it overlaps the region's end 3) remove from the replacement string the number of lines which are outside the region (before when overlapping region's start and after when overlapping region's end). The trailing indentation of the replacement string is not kept when the region's end is overlapped because it's always considered as outside the region. Change the target of this bug the patch is too big, hence too risky, to go into a maintenance stream. Released for 3.5M2 *** Bug 241470 has been marked as a duplicate of this bug. *** Verified for 3.5M2 using I20080914-2000 |