Bug 65531 - out of the box formatter settings need to be improved
Summary: out of the box formatter settings need to be improved
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.0 RC2   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords: polish
: 51959 (view as bug list)
Depends on:
Blocks:
 
Reported: 2004-06-03 09:19 EDT by Erich Gamma CLA
Modified: 2004-06-11 07:45 EDT (History)
5 users (show)

See Also:


Attachments
Failures in JDT/Core tests (2.01 KB, application/octet-stream)
2004-06-03 12:15 EDT, Olivier Thomann CLA
no flags Details
Failures in the JDT/UI refactoring tests (8.93 KB, application/octet-stream)
2004-06-03 13:38 EDT, Olivier Thomann CLA
no flags Details
Failures in JDT/UI tests (4.00 KB, application/octet-stream)
2004-06-03 13:55 EDT, Olivier Thomann CLA
no flags Details
Patch to apply on HEAD (2.32 KB, patch)
2004-06-03 14:34 EDT, Olivier Thomann CLA
no flags Details | Diff
Patch for the formatter tests (2.08 KB, patch)
2004-06-03 16:15 EDT, Olivier Thomann CLA
no flags Details | Diff
Apply on top of version v_436 (3.91 KB, patch)
2004-06-04 11:00 EDT, Olivier Thomann CLA
no flags Details | Diff
Also fixed javadoc for 3.0. (5.50 KB, patch)
2004-06-07 10:04 EDT, Olivier Thomann CLA
no flags Details | Diff
Patch to apply on HEAD (5.88 KB, application/octet-stream)
2004-06-07 11:19 EDT, Olivier Thomann CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Erich Gamma CLA 2004-06-03 09:19:42 EDT
The "Default" profile should not be the default. This profile isn't helpful. 
We need good out of the box formatter settings.

Suggestions:
* out of the box use the Java Conventions profile
* can we remove the "Default" profile?

(CCing Dirk since changing the default formatter settings affects the 
refactoring junit tests)
Comment 1 Olivier Thomann CLA 2004-06-03 09:29:41 EDT
The default profile uses the previous default settings. This was done to keep
some  settings from the previous formatter. From my point of view, it is too
late to change this now. Most of the refactoring tests would be affected.
This should be reported earlier in the process.
Comment 2 Erich Gamma CLA 2004-06-03 09:40:48 EDT
Both Dirk and I agree that this change is important for an improved out of the 
box experience. Dirk even considers it critical and he would be willing to 
update the test cases. 

Please consider it for RC2
Comment 3 Olivier Thomann CLA 2004-06-03 11:33:03 EDT
I don't see why this is suddenly critical. This has been like this since M6. So
why this is critical?
I will try to see how many failures I get by changing this.

Note that the changes could be done only on the UI side. If you set a set of
options the default one is not used. You can set the default options to be the
Java conventions settings.
Comment 4 Olivier Thomann CLA 2004-06-03 11:40:11 EDT
Ok, the default JavaCore options are affected by this. So at least the JavaCore
default options need to be updated if we want to change this.
Comment 5 Olivier Thomann CLA 2004-06-03 12:04:20 EDT
Add CC'
Comment 6 Olivier Thomann CLA 2004-06-03 12:15:34 EDT
Created attachment 11546 [details]
Failures in JDT/Core tests

This is a list of the failures in JDT/Core tests (mostly AST rewriting tests).
I will run the latest JDT/UI tests.
Comment 7 Olivier Thomann CLA 2004-06-03 13:38:01 EDT
Created attachment 11550 [details]
Failures in the JDT/UI refactoring tests

We have 187 failures in the JDT/UI refactorings tests. I am clearly against
such a change that late in the game.
Comment 8 Olivier Thomann CLA 2004-06-03 13:55:12 EDT
Created attachment 11551 [details]
Failures in JDT/UI tests

29 failures in the JDT/UI tests.
Comment 9 Olivier Thomann CLA 2004-06-03 14:34:37 EDT
Created attachment 11555 [details]
Patch to apply on HEAD
Comment 10 Philipe Mulet CLA 2004-06-03 15:45:55 EDT
Wonder if #getDefaultSettings() shouldn't be renamed then, if this isn't the 
default anymore... 

I approve the change, though agree it is a bit late in the game, but 
reasonable if broken clients are requesting it.

Comment 11 Olivier Thomann CLA 2004-06-03 15:49:02 EDT
getDefaultSettings() is an API. Not sure we can change that.
Comment 12 Olivier Thomann CLA 2004-06-03 16:15:01 EDT
Created attachment 11567 [details]
Patch for the formatter tests

There are more than 200 tests to fix.
Comment 13 Olivier Thomann CLA 2004-06-03 16:38:50 EDT
I made some changes to the Java convention settings.
		this.keep_guardian_clause_on_one_line = false;
		this.keep_simple_if_on_one_line = false;
These were set to true before.
Comment 14 Olivier Thomann CLA 2004-06-03 16:56:26 EDT
I also would like to change:
		this.blank_lines_before_first_class_body_declaration = 1;
to:
		this.blank_lines_before_first_class_body_declaration = 0;

This looks nicer for anonymous classes. This concerns some JDT/UI tests.
Checking the code conventions, this is not clearly specified. I should also add
a blank like before all block and line commments, but this would require too
many changes.
Comment 15 Olivier Thomann CLA 2004-06-03 17:01:08 EDT
With these changes we might need to update less tests.
Let me know what you want.
Comment 16 Olivier Thomann CLA 2004-06-04 11:00:27 EDT
Created attachment 11594 [details]
Apply on top of version v_436

That patch sets the Java conventions to be the default for the Java Code
options. It also the three changes.
Comment 17 Olivier Thomann CLA 2004-06-04 11:05:24 EDT
*** Bug 51959 has been marked as a duplicate of this bug. ***
Comment 18 Olivier Thomann CLA 2004-06-07 10:04:08 EDT
Created attachment 11659 [details]
Also fixed javadoc for 3.0.
Comment 19 Jim des Rivieres CLA 2004-06-07 11:18:33 EDT
The class in question, DefaultCodeFormatterConstants, also has a number of 
deprecated constants and methods. Since this class was new for 3.0, there is 
no good reason why it should be released with extraneous API. In addition to 
renaming getDefaultOptions() to getEclipse21Options() to better reflect what 
it does, the deadwood should also be trimmed.
Comment 20 Olivier Thomann CLA 2004-06-07 11:19:52 EDT
Created attachment 11664 [details]
Patch to apply on HEAD

Last set of changes for this bug.
- Renamed getDefaultSettings() to getEclipse21Settings()
- Updated some javadoc
- Removed all deprecated constants that were added for 3.0

I will attach another patch for the test suite.
Comment 21 Olivier Thomann CLA 2004-06-07 11:21:56 EDT
Jim,

It is not getEclipse21Options(), but getEclipse21Settings().
Comment 22 Jim des Rivieres CLA 2004-06-07 11:39:21 EDT
The likelihood of a party other than JDT and its test suites using this class 
is low. API cleanup of DefaultCodeFormatterConstants approved for 3.0 RC2.
Comment 23 Philipe Mulet CLA 2004-06-07 11:40:32 EDT
Ok for RC2.
Comment 24 Olivier Thomann CLA 2004-06-07 11:49:32 EDT
Fixed and released in HEAD.
Updated test cases.
Comment 25 Dirk Baeumer CLA 2004-06-07 11:56:43 EDT
Olivier, I hope this doesn't go into the build yet. We agreed on last Friday 
that we have to coordinate the release since otherwise JDT/UI test cases will 
fail. I am currently fixing our tests.
Comment 26 Olivier Thomann CLA 2004-06-07 12:00:39 EDT
I simply released the code in HEAD. That doesn't mean it will go inside next build.
Comment 27 Dirk Baeumer CLA 2004-06-07 17:12:33 EDT
Adapted the JDT/UI test cases to new default settings. The test cases are now 
independent from the default settings (they create their own default test 
settings; see bug 66048). The adapted test cases go into build I20040608_1200.
Comment 28 David Audel CLA 2004-06-11 07:45:50 EDT
Verified for 3.0RC2