Bug 231053 - [formatter] Comment formatter fails to format long strings in @see references
Summary: [formatter] Comment formatter fails to format long strings in @see references
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4 RC1   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-08 03:48 EDT by Frederic Fusier CLA
Modified: 2008-05-13 09:32 EDT (History)
3 users (show)

See Also:
Olivier_Thomann: review+

Attachments
Test case to illustrate the problem (305 bytes, text/x-java)
2008-05-08 03:48 EDT, Frederic Fusier CLA 		no flags Details
Proposed patch (10.01 KB, patch)
2008-05-08 08:20 EDT, Frederic Fusier CLA 		no flags Details | Diff
New proposed patch (13.13 KB, patch)
2008-05-08 14:10 EDT, Frederic Fusier CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Frederic Fusier CLA 2008-05-08 03:48:49 EDT 
Created attachment 99231 [details]
Test case to illustrate the problem

Using 3.4M7 (e.g. the new comment formatter).

Attached test case is not formatted properly: a star is added after the @see tag and each reformatting add a new line between the tag and the string. This definitely looks weird.
Comment 1 Frederic Fusier CLA 2008-05-08 03:49:42 EDT 
I think this must be fixed for 3.4
Comment 2 Frederic Fusier CLA 2008-05-08 08:20:51 EDT 
Created attachment 99259 [details]
Proposed patch
Comment 3 Frederic Fusier CLA 2008-05-08 08:21:31 EDT 
With this patch, I only get one failure on Eclipse 3.0 full source workspace test.
Olivier, could you please review?
Comment 4 Olivier Thomann CLA 2008-05-08 13:13:18 EDT 
The patch for:
Lines 3205-3213

should be cleaned up. The if statement seems useless if the block is commented out.

Also the closing */ in the last test case doesn't seem to be inlined with the rest of the comment.
Comment 5 Frederic Fusier CLA 2008-05-08 13:50:49 EDT 
(In reply to comment #4)
> The patch for:
> Lines 3205-3213
> 
> should be cleaned up. The if statement seems useless if the block is commented
> out.
> 
Correct, the case TerminalTokens.TokenNameStringLiteral can completely be removed.

> Also the closing */ in the last test case doesn't seem to be inlined with the
> rest of the comment.
> 
Correct again, for X32.java test case output, it seems that the next line is not properly indented, I investigate...
Comment 6 Olivier Thomann CLA 2008-05-08 13:56:07 EDT 
Once you submit a new patch, I'll review it again.
Comment 7 Frederic Fusier CLA 2008-05-08 14:10:26 EDT 
Created attachment 99343 [details]
New proposed patch

I also realized that some tests were not activated...
Thanks for the smart review
Comment 8 Olivier Thomann CLA 2008-05-08 14:59:42 EDT 
Looks good.
+1 for 3.4RC1.
Comment 9 Frederic Fusier CLA 2008-05-08 15:04:54 EDT 
Released for 3.4RC1 in HEAD stream.
Comment 10 Eric Jodet CLA 2008-05-13 09:09:02 EDT 
Verified for 3.4RC1 using build I20080510-2000.
Comment 11 Eric Jodet CLA 2008-05-13 09:32:36 EDT 
(In reply to comment #10)
> Verified for 3.4RC1 using build I20080510-2000.
> 
Though this bug is fixed, I opened bug 231800 to address a little formatting difference between "old" and "new" formatter