Bug 230944 - [formatter] Formatter does not respect /*-
Summary: [formatter] Formatter does not respect /*-
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 critical (vote)
Target Milestone: 3.4 RC2   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-07 13:38 EDT by Steven Schwab CLA
Modified: 2008-05-23 05:37 EDT (History)
5 users (show)

See Also:
Olivier_Thomann: review+
kent_johnson: review+


Attachments
Proposed patch (4.39 KB, patch)
2008-05-08 14:47 EDT, Frederic Fusier CLA
no flags Details | Diff
Additional patch to fix the remaining issue (3.95 KB, patch)
2008-05-22 14:12 EDT, Frederic Fusier CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steven Schwab CLA 2008-05-07 13:38:30 EDT
Build ID: I20080502-0100

Steps To Reproduce:
1. Create a block comment with whitespace formatting that starts with /*-
2. Run the Eclipse [built-in] formatter
3. Whitespace will be stripped out, rendering comments that contain lists or tree diagrams unreadable


More information:
Sun's code conventions (http://java.sun.com/docs/codeconv/html/CodeConventions.doc4.html#385) say the following:

Block comments can start with /*-, which is recognized by indent(1) as the beginning of a block comment that should not be reformatted. Example:

/*- 
 * Here is a block comment with some very special
 * formatting that I want indent(1) to ignore.
 *
 *    one
 *        two
 *            three
 */


The above example is being turned into:

/*
 * - Here is a block comment with some very special formatting that I want
 * indent(1) to ignore.
 * 
 * one two three
 */


Eclipse 3.3's formatter respected /*-.
Comment 1 Frederic Fusier CLA 2008-05-08 14:47:13 EDT
Created attachment 99349 [details]
Proposed patch
Comment 2 Frederic Fusier CLA 2008-05-08 14:50:37 EDT
No regression noticed with this patch on Eclipse 3.0 and Ganymede workspaces tests.
Olivier, could you please review?
Comment 3 Olivier Thomann CLA 2008-05-08 14:55:22 EDT
Patch looks good.
+1 for 3.4RC1.
Comment 4 Frederic Fusier CLA 2008-05-08 15:05:08 EDT
Released for 3.4RC1 in HEAD stream.
Comment 5 Eric Jodet CLA 2008-05-13 09:03:46 EDT
Verified for 3.4RC1 using build I20080510-2000.
Comment 6 Steven Schwab CLA 2008-05-22 13:24:53 EDT
I have found a problem with the fix in I20080516-1333.

In the following code, the first block is prefixed "/*-", and the second is prefixed with "/*- " (with a space after the dash):

/*-
 * Foo           bar
 */

/*- 
 * Foo           bar
 */

When run through the formatter, it strips out the line with "/*-":

 * Foo           bar
 */

/*- 
 * Foo           bar
 */

Any character (except for a return) after the - will prevent the line from being stripped.

Comment 7 Frederic Fusier CLA 2008-05-22 13:56:18 EDT
Reproduced. The problem comes from the fact that in this case, the scanner need to be reset from the comment start (e.g. before the /*) to indent properly the entire comment. Unfortunately, it works when there's a space and this made me missed this point...
Comment 8 Frederic Fusier CLA 2008-05-22 14:12:52 EDT
Created attachment 101574 [details]
Additional patch to fix the remaining issue
Comment 9 Frederic Fusier CLA 2008-05-22 14:13:51 EDT
Olivier, Jerome, could you please review?
Comment 10 Olivier Thomann CLA 2008-05-22 14:58:07 EDT
+1. Current implementation is losing code. So this is critical (data loss).
Comment 11 Kent Johnson CLA 2008-05-22 15:48:46 EDT
looks good - obvious fix
Comment 12 Philipe Mulet CLA 2008-05-22 15:49:07 EDT
I also feel this needs to be addressed for RC2, since this is a regression introduced by the 1st change.
Comment 13 Frederic Fusier CLA 2008-05-22 16:23:09 EDT
Released for 3.4RC2 in HEAD stream.
Comment 14 Eric Jodet CLA 2008-05-23 05:37:05 EDT
Verified for 3.4RC2 using build I20080523-0100