Bug 236406 - [formatter] The comments flags should work for all kind of snippet
Summary: [formatter] The comments flags should work for all kind of snippet
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.6 M6   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2008-06-10 06:47 EDT by Frederic Fusier CLA
Modified: 2010-03-09 06:30 EST (History)
3 users (show)

See Also:


Attachments
Proposed patch (41.32 KB, patch)
2010-02-16 11:19 EST, Frederic Fusier CLA
no flags Details | Diff
Proposed patch + Marking obsolete code deprecated (56.55 KB, patch)
2010-02-17 03:37 EST, Frederic Fusier CLA
no flags Details | Diff
New proposed patch (134.70 KB, patch)
2010-02-18 07:15 EST, 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 Frederic Fusier CLA 2008-06-10 06:47:02 EDT
Follow-up of bug 102780 comment 23.

While implementing the new comment formatter for 3.4 version, a new flag has been added to include comments while formatting a compilation unit. It means that currently this flag only works either when K_COMPILATION_UNIT kind is used or when K_UNKNOWN flag is used and the snippet is a compilation unit...

As there's no design reason to have this limitation, next version should fix it and make this flag meaningful for all kinds of snippet.

Note that for comment kinds, this flag has and still will have no effect as it's redundant...
Comment 1 Frederic Fusier CLA 2009-03-11 07:39:34 EDT
Unfortunately, I missed this one for 3.5 => reset the target
Comment 2 Olivier Thomann CLA 2009-11-10 14:22:00 EST
Frédéric, this would be a good candidate in your current work on the formatter.
Comment 3 Frederic Fusier CLA 2010-02-16 11:19:44 EST
Created attachment 159206 [details]
Proposed patch

This patch makes the flag F_INCLUDE_COMMENTS also working for K_EXPRESSION, K_CLASS_BODY_DECLARATIONS and K_STATEMENTS.

Note that it also removes the internal possibility to deactivate the 'new' comment formatter (as it should have been done in 3.5...). Hence, there won't be any possibility to retrieve the "old" comment formatter behavior...
Comment 4 Olivier Thomann CLA 2010-02-16 11:28:45 EST
As long as we get a proper comment formatting, this is fine. Once the last dependency is removed, we should remove the corresponding old code.
Comment 5 Frederic Fusier CLA 2010-02-17 03:37:18 EST
Created attachment 159275 [details]
Proposed patch + Marking obsolete code deprecated

Same implementation in this patch than the previous but I just deprecated classes used by the "old" comment formatter and warned that this code will be removed for 3.6M6.

Olivier, if you agree with this more "aggressive" patch, then I'll also post a warning on the jdt-core-dev mailing list...
Comment 6 Frederic Fusier CLA 2010-02-17 03:38:27 EST
comment #5 should read: "...and warned that this code will be removed for 3.6."
Comment 7 Olivier Thomann CLA 2010-02-17 10:04:51 EST
(In reply to comment #5)
> Olivier, if you agree with this more "aggressive" patch, then I'll also post a
> warning on the jdt-core-dev mailing list...
Looking at use scan results, we don't have any references to this internal code. Now to be safe, tagging as deprecated might be good enough for 3.6.
Daniel ?
Comment 8 Dani Megert CLA 2010-02-17 10:19:17 EST
>Looking at use scan results, we don't have any references to this internal
>code. Now to be safe, tagging as deprecated might be good enough for 3.6.
>Daniel ?
Assuming you checked the scans, simply remove the internal code (no deprecation) right away. Products working against 3.6 will immediately see it and can react. References from products based on 3.5 or 3.4 would surface in the API scans.

Do we have to adjust some API Javadoc?
Comment 9 Frederic Fusier CLA 2010-02-17 10:28:14 EST
(In reply to comment #8)
> >Looking at use scan results, we don't have any references to this internal
> >code. Now to be safe, tagging as deprecated might be good enough for 3.6.
> >Daniel ?
> Assuming you checked the scans, simply remove the internal code (no
> deprecation) right away. Products working against 3.6 will immediately see it
> and can react. References from products based on 3.5 or 3.4 would surface in
> the API scans.
> 
> Do we have to adjust some API Javadoc?

I'll check that and repost a patch with obsolete code removed...
Comment 10 Frederic Fusier CLA 2010-02-18 07:15:50 EST
Created attachment 159416 [details]
New proposed patch

This patch removes all the obsolete internal code. It also refresh the CodeFormatter javadoc comments...
Comment 11 Frederic Fusier CLA 2010-02-22 06:06:35 EST
(In reply to comment #10)
> Created an attachment (id=159416) [details]
> New proposed patch
> 
Released for 3.6M6 in HEAD stream.
Comment 12 Srikanth Sankaran CLA 2010-03-09 06:30:37 EST
Verified for 3.6M6 using build I20100305-1011