Community
Participate
Working Groups
Since bug 208451 has been fixed, regions are tentatively adapted to include edits before the region beginning and after the region's end to have a better output while formatting. Unfortunately, the applied fix has several potential issues: 1) more than the selected part of the code can be formatted 2) not all possible cases are addressed 3) some regions might be not considered The symptoms for the problems above are: 1) Format the initial bug 208541 comment 0 test case as described. See that the selection at the end of the format includes the line separator before the selected line. Changing this test case a little (spaces are replaced by points to see the difference): public class Test {..... ........................int i= 1;..................... } Then it will be formatted as: public class Test { int i= 1; } Note that trailing spaces after the opening brace are removed although they were outside the selection. 2) The following test case is not formatted properly while selecting the entire line of field declaration: public class X01 { int i= 1; } Although removing one space or tab anywhere between the opening brace and the selection beginning will make the correct formatting happening again (and shows the problem 1) as the 3 empty lines before the selected line are replace by only one). 3) In the adaptRegions() method of Scribe version v_869, if the condition at line 243 is false: if (offset != upperBound || regionEnd != lowerBound) { // ... then, the adaptedRegions[i] will not be set
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...
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
