Community
Participate
Working Groups
Build Identifier: I20090611-1540 Many people (including me) don't use JDT's Java Formatter, because they don't like one or another detail of its formatting strategy. Since the Java formatter is not extensible (extension point "org.eclipse.jdt.core.codeFormatter" only worked for Eclipse 2, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=309787 ), it is not possible to modify or enhance it. I envisage a set of extension points that allow one to, e.g., add "line wrapping policies", or tree items on the "Line Wrapping" tab of the profile editor. Reproducible: Always
Created attachment 166177 [details] Example modification of the Java Formatter strategy This is an example of how I patched "CodeFormatterVisitor" and "Alignment" to conform to my enterprise's formatting rules.
(In reply to comment #1) > Created an attachment (id=166177) [details] > Example modification of the Java Formatter strategy > > This is an example of how I patched "CodeFormatterVisitor" and "Alignment" to > conform to my enterprise's formatting rules. Please use a CVS diff format... from a well-identified JDT/Core version (or Eclipse build ID). Thanks
Unfortunately I don't have access to the ECLIPSE CVS repository from here, so I cannot do CVS DIFF. Which DIFF formats are acceptable for you? I can do -c -C NUM --context[=NUM] Output NUM (default 3) lines of copied context. -u -U NUM --unified[=NUM] Output NUM (default 3) lines of unified context. --normal Output a normal diff. -n --rcs Output an RCS format diff. (For the patch that I attached previously, I used "-c".)
To make it clear: My patch does NOT implement what I requested; it is merely an example of how I modified the JDT Java formatter to suit MY personal (line wrapping) needs. In other words: It demonstrates what formatting functionality I want, not how the formatter would be made extensible.
(In reply to comment #3) > Unfortunately I don't have access to the ECLIPSE CVS repository from here, so I > cannot do CVS DIFF. > > Which DIFF formats are acceptable for you? I can do > > -c -C NUM --context[=NUM] Output NUM (default 3) lines of copied context. > -u -U NUM --unified[=NUM] Output NUM (default 3) lines of unified context. > --normal Output a normal diff. > -n --rcs Output an RCS format diff. > > (For the patch that I attached previously, I used "-c".) RCS diff would be great (I think it's the one used by CVS...)
Created attachment 166608 [details] patch_for_Alignment.java_rev_1.32.rcsdiff Patch for org/eclipse/jdt/internal/formatter/align/Alignment.java of ECLIPSE R3_5
Created attachment 166609 [details] patch_for_CodeFormatterVisitor.java_rev_1.214.rcsdiff Patch for org/eclipse/jdt/internal/formatter/CodeFormatterVisitor.java of ECLIPSE R3_5
After investigating in alternative Java formatting tools (namely JALOPY and JINDENT), I feel that it is better to leave the JDT Java Formatter a monolithic tool (with its own set of formatting options), and offer the user to CHOOSE between different formatters, which are all configured on their own preference pages. For example, the preference page "Java Code Style / Formatter" would be pushed down one level and renamed to "JDT" (or "Default", or "Standard"), and the former "Formatter" page would look like this: _Configure project specific settings..._ Active formatter: JDT JALOPY JINDENT Analogous, the project properties page "Java Code Style / Formatter" would be pushed down one level and renamed as above, and the new "Formatter" project properties page would look like: [x] Enable project specific settings _Configure workspace settings..._ Active formatter: JDT JALOPY JINDENT I believe that the management of formatting profiles should be left to the individual formatters, i.e. you'd first choose the formatter, and then deal with the formatter-specific profile management. Effectively, we'd only need ONE extension point: A successor of the retired "org.eclipse.jdt.core.codeFormatter" (see Bug 309787). Then "ToolFactory.createFormatter(Map options, int mode)" would create the "org.eclipse.jdt.core.formatter.CodeFormatter"-derived class of the formatter that was selected on the "Formatter" preference/project properties page. This would tremendously boost the usefulness of third-party formatting tools, because then they are automatically applied to all AST-based code modifications, i.e. most of the fantastic JDT refactorings!
Does anybody care about this one?
Arno, let me apologize for the delay in replying to you. Things have been extremely busy in the JDT land because of the java 7 work and then resource crunch with developers in charge of the formatter taking up other responsibilities. IMHO, your proposal is very useful and we will surely look at it as soon as we can. Will keep you posted. Thanks for your interest. Feel free to post a patch meanwhile, that will surely accelerate things on this bug.
Btw, bug 309787 comment 3 elucidates one problem in extending the formatter. So, this may not be doable in a clean way.
Actually, I feel more tempted to contribute "my formatting" to the current Java formatter, than creating a brand new Java formatter from scratch. Would that be OK for you? After all, is there a discussion anywhere as to which styles of formatting should be implemented, and which not, i.e. about "good Java formatting"? Or is the policy to be as flexible as possible and implement as many formatting styles as possible?
(In reply to comment #12) > Actually, I feel more tempted to contribute "my formatting" to the current Java > formatter, than creating a brand new Java formatter from scratch. Would that be > OK for you? Umm..when you say you want to contribute your own formatting, do you mean you want to introduce new formatter options other than what we have? Or configure those options in a way that creates your style of formatting? For the latter, creating a new formatter profile is the way to go. So assuming you're talking of the former here, please let us know what exactly do you want to change/add.
(In reply to comment #13) > (In reply to comment #12) > Umm..when you say you want to contribute your own formatting, do you mean you > want to introduce new formatter options other than what we have? Yes, that's what I meant. E.g. I want to wrap function call arguments like this: class Example { void foo() { Other.bar( 100, nested( 200, 300, 400, 500, 600, 700, 800, 900 ) // <= Closing parenthesis on its own line ); // <= Closing parenthesis on its own line } } This is a very logical way, because parentheses are aligned just like everybody aligns the Java braces. This formatting optional could be implemented with an additional checkbox "Also wrap closing parenthesis", in analogy (and neighborhood) with the "Force split, even if..." checkbox. Is this kind of enhancements acceptable for you? I've heard from many users that JDT's formatter does not format their source code EXACTLY as they want it, so they don't use it. My conclusion is that there should be as much flexibility as possible for the formatter configuration to make as many persons happy... Regards, Arno
Created attachment 208252 [details] Enhanced Java Formatter The attached patch is a prototype implementation of an enhanced "Line Wrapping" tab, based on R3_7_1: * Adds a "Line break after last element" checkbox. * In category "Expressions": * Adds new category "Parenthesized expressions" after category "Expressions". * Adds new categories "Parenthesized conditionals" and "Parenthesized conditional chains" after category "Conditionals". This makes it possible to have different formatting for parenthesized and unparenthesized binary and ternary expressions. So what do you think about this?
Hi Arno, Thanks for the patch. Note that we're in the 3.8 release so it would be great if the patch is rebased on the 'master' branch in the git repo. That said, our plate is alreafy full with planned items for 3.8M5 so I'm not sure if we'll have enough time to look at your patch. Also note that we are considering bug 303519 in the formatter area, so you may want to wait and see how it pans out. You may have to rework your patch if that gets released. I'll keep you posted Markus, whats your take on the proposal?
Hi Ayushman, thanks for the quick response! To get on the 3.8 "master branch", I'd have to use GIT, right? There's really A LOT going on on Bug 303519 ;-) But than one's mostly about bugs, not new features, right?
(In reply to comment #17) > Hi Ayushman, > > thanks for the quick response! To get on the 3.8 "master branch", I'd have to > use GIT, right? Yup! http://wiki.eclipse.org/Platform-releng/Git_Workflows has the list of repos. > There's really A LOT going on on Bug 303519 ;-) But than one's mostly about > bugs, not new features, right? Well it does introduce some fundamental changes in the formatter, but yeah its not really about a new feature.
Hello Ayushman, it is very good that Ankush refactors the Formatter! I like his document and his concept. Unfortunately "ssh://git.eclipse.org" seems to be down today (it times out), so I cannot look at the new code. ("http://git.eclipse.org", however, works.) Do you know anybody to turn to? How are we going to proceed with this ticket? I still plan to enhance the (new) formatter, but the title is now quite wrong. Should I close it and open a new one when I have re-implemented the patch? CU Arno
(In reply to comment #19) > Unfortunately "ssh://git.eclipse.org" seems to be down today (it > times out), so I cannot look at the new code. ("http://git.eclipse.org", > however, works.) Do you know anybody to turn to? DO you have a eclipse committer id and password? If not then you should use http and not ssh. Should work. > How are we going to proceed with this ticket? I still plan to enhance the (new) > formatter, but the title is now quite wrong. Should I close it and open a new > one when I have re-implemented the patch? I would suggest to keep this one open and keep a watch on the formatter bug. We can then focus on this issue. Thanks!
(In reply to comment #12) > Or is the policy to be as flexible as possible and implement as many > formatting styles as possible? The policy is rather "be as flexible as necessary and implement support for other formatting styles if there is a well-supported proposal". Not all clients use the formatter as a black box. Some also read certain settings directly to generate matching code. Since all these clients would be broken if an external formatter would be used, I agree that improving our formatter is a better strategy. I didn't look at the patch closely nor played with it, but the "Line break after last element" option sounds reasonable. I'm not so sure about "Parenthesized binary expressions", "Parenthesized conditionals", and "Parenthesized conditional chains". I would have to see compelling examples that convince me that these very special options are really necessary and that the concrete examples cannot be supported in a more general way. It's not clear to me why we should have special options for these 3 kinds of parenthesized expressions but not for all of them.
> DO you have a eclipse committer id and password? If not then you should use > http and not ssh. Should work. That's surprising... the instructions http://wiki.eclipse.org/Platform-releng/Git_Workflows told me to use SSH!? Or did I miss the right document? Anyway, when I use http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/ , I get Caused by: java.io.IOException: http://git.eclipse.org/c/jdt /eclipse.jdt.core.git/objects/b9/69413a0908a2d32ba401c83f7ad6d79c1971d9: 403 Forbidden , even if I use my eclipse bugs account for authentication (user name "arno@unkrig.de" and password). Notice that the "Branch selection" list appears; but regardles of which branch(es) I select and hit FINISH, I get the error message mentioned above. What am I doing wrong? Sorry for all those GIT newbie hassle... CU Arno
(In reply to comment #21) > I'm not so sure about "Parenthesized binary expressions", "Parenthesized > conditionals", and "Parenthesized conditional chains". I would have to see > compelling examples that convince me that these very special options are really > necessary and that the concrete examples cannot be supported in a more general > way. It's not clear to me why we should have special options for these 3 kinds > of parenthesized expressions but not for all of them. I forgot to explain this... The idea is that expressions that are too long for one line should be wrapped exactly like blocks, method parameters and function call arguments: After the opening brace, between the fragments (typically before that operator), and before the closing brace. This yields MUCH better readability that the commonly used "arbitrary" wrapping style. Examples: // Parenthesized expression: this.compileError(( "Statement \"break " + bs.optionalLabel + "\" is not enclosed by a breakable statement with label \"" + bs.optionalLabel + "\"" ), bs.getLocation()); // ... // Parenthesized expression: if ( s instanceof BlockStatement && (es instanceof Block || es instanceof FunctionDeclarator) ) { // ... // Parenthesized expression: if ( !this.tryIdentityConversion(resultType, lhsType) && !this.tryConversion((Locatable) a, resultType, lhsType) && !this.tryBoxingConversion((Locatable) a, resultType, lhsType) ) this.compileError("Operand unsuitable for '" + a.operator + "'"); // ... // Parenthesized expression: throw new CompileException(( this.compileErrorCount + " error(s) while compiling unit \"" + this.compilationUnit.optionalFileName + "\"" ), null); // Parenthesized conditional: short innerNameIndex = ( this instanceof NamedTypeDeclaration ? cf.addConstantUtf8Info(((NamedTypeDeclaration) this).getName()) : (short) 0 ); // Parenthesized conditional chain - no line break before '?': int opIdx = ( bo.op == "==" ? 0 : bo.op == "!=" ? 1 : bo.op == "<" ? 2 : bo.op == ">=" ? 3 : bo.op == ">" ? 4 : bo.op == "<=" ? 5 : Integer.MIN_VALUE ); // Also: Wrapped FOR statement: for ( Scope s = bs.getEnclosingScope(); s instanceof Statement || s instanceof CatchClause; s = s.getEnclosingScope() ) { // ...
Examples 4.71 through 4.75 on http://jalopy.sourceforge.net/existing/wrapping.html illustrate this.
Now it's working again, all of a sudden!? A bit cranky for a GIT repository ;-) Comment #22 is now obsolete. (In reply to comment #22) > Anyway, when I use > > http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/ > > , I get > > Caused by: java.io.IOException: http://git.eclipse.org/c/jdt > /eclipse.jdt.core.git/objects/b9/69413a0908a2d32ba401c83f7ad6d79c1971d9: > 403 Forbidden
