Bug 252556

Summary:

[formatter] Spaces removed before formatted region of a compilation unit.

Product:

[Eclipse Project] JDT

Reporter:

Jerome Lanneluc <jerome_lanneluc>

Component:

Core

Assignee:

Frederic Fusier <frederic_fusier>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

daniel_megert, Olivier_Thomann, satyam.kandula

Version:

3.4

Target Milestone:

3.7 M2

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
JUnit test for this issue	none
Proposed patch	none
New proposed patch	none

Description Jerome Lanneluc

2008-10-29 05:21:37 EDT

I20081027-1800

The test case from bug 228652 comment 0 still fails because spaces are removed before the region that is being formatted (the region being a Javadoc in this test case).

Comment 1 Frederic Fusier

2009-12-17 11:07:59 EST

Created attachment 154671 [details]
JUnit test for this issue

Note that this test currently passes. That means it verifies that the problem still occurs and of course, it needs to be changed accordingly (see the comment line) when this issue will be fixed...

Comment 2 Frederic Fusier

2009-12-17 11:10:35 EST

(In reply to comment #1)
> Created an attachment (id=154671) [details]
> JUnit test for this issue
> 
> Note that this test currently passes. That means it verifies that the problem
> still occurs and of course, it needs to be changed accordingly (see the comment
> line) when this issue will be fixed...

Released in HEAD...

Comment 3 Frederic Fusier

2010-08-16 07:22:26 EDT

Created attachment 176666 [details]
Proposed patch

This patch makes the formatter not including the indentation spaces before the selection for javadoc comments and also add a field to valid the edits (OptimizedReplaceEdit) to remove them more easily when they are outside the given regions.

Comment 4 Frederic Fusier

2010-08-16 07:25:13 EDT

All JDT/Core, JDT/UI and JDT/Text tests passes with this patch as well as all massive regression tests.

Released for 3.7M2 in HEAD stream.

Comment 5 Frederic Fusier

2010-08-16 07:25:23 EDT

Comment 6 Frederic Fusier

2010-08-17 06:26:29 EDT

(In reply to comment #4)
> All JDT/Core, JDT/UI and JDT/Text tests passes with this patch as well as all
> massive regression tests.
> 
> Released for 3.7M2 in HEAD stream.

There were failures for 3 of the added tests on Linux tests box, hence I have reverted the patch from HEAD until I discover the reason of these failures...

Comment 7 Frederic Fusier

2010-09-03 10:48:25 EDT

Created attachment 178159 [details]
New proposed patch

This patch passes all JDT tests on Windows and all JDT/Core tests on Linux. It also has no performance impact on the formatter and no observed regression on massive tests...

Comment 8 Frederic Fusier

2010-09-03 10:50:30 EDT

(In reply to comment #7)
> Created an attachment (id=178159) [details]
> New proposed patch
> 
> This patch passes all JDT tests on Windows and all JDT/Core tests on Linux. It
> also has no performance impact on the formatter and no observed regression on
> massive tests...

So, released (again) for 3.7M2 in HEAD stream.

Comment 9 Satyam Kandula

2010-09-14 10:00:56 EDT

Verified for 3.7M2 using build I20100909-1700