Bug 264112 - [Formatter] Wrap when necessary too aggressive on short qualifiers
Summary: [Formatter] Wrap when necessary too aggressive on short qualifiers
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 M5   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 83915 99026 104004 104774 114639 117896 130471 143634 147623 189983 196563 197155 214743 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-02-08 19:21 EST by Christopher Oezbek CLA
Modified: 2010-05-02 08:31 EDT (History)
17 users (show)

See Also:

Attachments
Proposed patch (15.83 KB, patch)
2009-12-30 11:56 EST, Frederic Fusier CLA 		no flags Details | Diff
Proposed patch to fix the failure in JDT-UI test (8.81 KB, patch)
2009-12-30 11:58 EST, Frederic Fusier CLA 		no flags Details | Diff
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 Christopher Oezbek CLA 2009-02-08 19:21:06 EST 
Build ID: I20080617-2000

Steps To Reproduce:
1. Go to Preferences -> Java -> Code Style -> Formatter -> Edit on the "Eclipse [built-in]" profile
2. Go to Tab "Line Wrapping", Select "Function Calls"
3. On the Preview Tab Set Line Width for 25 or smaller

The original line: 

Other.bar(100, 200,....)

will be formatted

Other.
        bar(
                100,
                200,

Obviously wrapping is necessary because the original line is longer than the set line width, but wrapping after Other. and bar( doesn't do any good because the identifiers Other and bar are shorter than the indentation depth (2 tabs == 8 spaces).

Suggested fix:

Add an additional Line Wrapping Policy "Wrap when benefitial" which works like "Wrap when necessary" but avoids wrapping if the leading line is shorter than the indentation width and thus wrapping is counter-productive.

Result:

Other.bar(
        100,
        200,...

Thanks for taking note!
  Christopher

P.S.: Note that with "Wrapping when benefitial" the resulting format is less Wide than with "Wrapping if necessary".
Comment 1 Olivier Thomann CLA 2009-12-09 10:38:43 EST 
This goes in the lines of what I would like to see improved in the line wrapping for 3.6.
Comment 2 Frederic Fusier CLA 2009-12-30 11:56:38 EST 
Created attachment 155131 [details]
Proposed patch

This patch fixes the problem in the peculiar case of comment 0 and does not need any new preference. I preferred to fix it this way as I think the formatter already has enough preferences and adding a new one should be motivated by a real need.

I've run all JDT Core, UI and Text tests using this patch and I only observed one failure in the UI test which is a logical consequence of the fix. I'll add a specific patch to fix the UI test...

Note that I also ran formatter massive tests, but there are too many changes to verify them all (a little bit more than 5,000 CUs had a different output with this patch...!). However, for all CUs I have verified (around half of them!), the difference were also logical consequence of the fix and definitely improved the formatter output :-)
Comment 3 Frederic Fusier CLA 2009-12-30 11:58:33 EST 
Created attachment 155132 [details]
Proposed patch to fix the failure in JDT-UI test
Comment 4 Markus Keller CLA 2010-01-04 08:17:56 EST 
(In reply to comment #3)
> Created an attachment (id=155132) [details] [diff]
> Proposed patch to fix the failure in JDT-UI test

Looks good, thanks. Please ping me when you release the Core part, so I can follow for the same N-build.
Comment 5 Frederic Fusier CLA 2010-01-04 12:10:43 EST 
(In reply to comment #2)
> Created an attachment (id=155131) [details]
> Proposed patch
> 
Released for 3.6M5 in HEAD stream.
Comment 6 Satyam Kandula CLA 2010-01-25 03:50:05 EST 
Verified for 3.6M5 using Build id: I20100122-0800
Comment 7 Srikanth Sankaran CLA 2010-01-25 04:24:36 EST 
Verified.
Comment 8 Christopher Oezbek CLA 2010-01-25 05:03:18 EST 
Thanks guys for fixing this issue! Looking forward for 3.6M5

Take care and greetings from Berlin,
  Christopher
Comment 9 Frederic Fusier CLA 2010-02-02 06:20:50 EST 
*** Bug 214743 has been marked as a duplicate of this bug. ***
Comment 10 Frederic Fusier CLA 2010-04-14 05:38:40 EDT 
*** Bug 130471 has been marked as a duplicate of this bug. ***
Comment 11 Frederic Fusier CLA 2010-04-14 05:59:08 EDT 
*** Bug 83915 has been marked as a duplicate of this bug. ***
Comment 12 Frederic Fusier CLA 2010-04-14 05:59:26 EDT 
*** Bug 143634 has been marked as a duplicate of this bug. ***
Comment 13 Frederic Fusier CLA 2010-04-14 06:04:17 EDT 
*** Bug 99026 has been marked as a duplicate of this bug. ***
Comment 14 Frederic Fusier CLA 2010-04-14 06:55:41 EDT 
*** Bug 189983 has been marked as a duplicate of this bug. ***
Comment 15 Frederic Fusier CLA 2010-04-14 06:55:44 EDT 
*** Bug 104004 has been marked as a duplicate of this bug. ***
Comment 16 Frederic Fusier CLA 2010-04-14 06:57:39 EDT 
*** Bug 104774 has been marked as a duplicate of this bug. ***
Comment 17 Frederic Fusier CLA 2010-04-14 06:59:06 EDT 
*** Bug 114639 has been marked as a duplicate of this bug. ***
Comment 18 Frederic Fusier CLA 2010-04-14 07:05:10 EDT 
*** Bug 117896 has been marked as a duplicate of this bug. ***
Comment 19 Frederic Fusier CLA 2010-05-02 08:30:31 EDT 
*** Bug 147623 has been marked as a duplicate of this bug. ***
Comment 20 Frederic Fusier CLA 2010-05-02 08:30:53 EDT 
*** Bug 196563 has been marked as a duplicate of this bug. ***
Comment 21 Frederic Fusier CLA 2010-05-02 08:31:03 EDT 
*** Bug 197155 has been marked as a duplicate of this bug. ***