Community
Participate
Working Groups
It would be nice if the new code formatter supported "new line on wrap" way of placing left curly brace. The following is a quote from CheckStyle documentation: If the brace will fit on the first line of the statement, taking into account maximum line length, then apply eol rule [The brace must always be on the end of the line]. Otherwise apply the nl rule [The brace must always be on a new line]. nlow is a mnemonic for "new line on wrap". For the example above Checkstyle will enforce: if (condition) { ... But for a statement spanning multiple lines, Checkstyle will enforce: if (condition1 && condition2 && condition3 && condition4) { ... Thanks in advacena and sorry if this is a dup.
Created attachment 7281 [details] An implementation of requested enhancement
Created attachment 7282 [details] Patch for org.eclipse.jdt.ui
Created attachment 7283 [details] Patch for org.eclipse.jdt.core.tests.model.
Created attachment 7284 [details] Patch for org.eclipse.jdt.core
Created attachment 7285 [details] Patch for org.eclipse.jdt.core.tests.model
I have attached more or less complete implementation of the requested enhancement and hope you'll find this code useful (apply the patches to the corresponding plugins). I did not implement "new line on wrap" formatting of "catch" blocks, anonymous classes, "do" and "switch" statements and "synchronized" blocks as I was not sure about practical benefits of this. It should not be difficult to implement this missing formatting and I will be glad to do that if you think it is important. I also would like to thank you for the great code; it was real pleasure to work with it. One little suggestion – FormatterRegressionTests does need to be broken into a number of smaller test suites ;-) Regards, Igor Fedorenko PS: just noticed that tests patch contains extra file workspace/Converter/test0191/Test.java which I did not create. Looks like Eclipse lost CVS/Entries file.
Created attachment 7289 [details] Patch for org.eclipse.jdt.core Fixed a problem with comments preceding type/method declarations and statements.
Created attachment 7290 [details] Patch for org.eclipse.jdt.core.tests.model Fixed a problem with comments preceding type/method declarations and statements.
Created attachment 7291 [details] Patch for org.eclipse.jdt.ui Set line width for brace placement preview window. PS: sorry for the flurry of updates. will try not to do that in the future.
I will review your patch.
I agree with you for comment 6, but to be consistent we should do it for all kind of blocks: Class declaration Anonymous class declarations Method declaration Blocks 'switch...case' statement. So we could exclude: Anonymous class declarations 'switch...case' statement. But we would have to handle the three others. This means that catch, synchronized statement and do/while would be handled as well. What do you think?
This rule is supposed to make it easy to find where block code begins. So, for "one-world" blocks -- "try", "do", "finally" -- this rule does not seem to help at all because you cannot write a keyword on multiple lines. The only condition when this rule can be applied is page width, more precisely, when keyword fits within set limits but lcrurly does not. In my opinion moving lcurly to the next line in this case make code less readable. The other two types of block are "synchronized(...)" and "catch(...)". For these two blocks I agree, applying this rule makes sense, I even have sensibly examples (input for unit tests ;-) synchronized (object .method() .method() .method()) { ... and catch (com .ibm .tivoli .orchestrator .de .scriptlet .ExecutionException ex) { ... Note that the same logic can not be applied to "switch" statement because it does not directly contain any code, only case/default keywords. I am also not so sure about anonymous class declarations... It may actually make sense to apply this rule to something like the code below. What do you think? Object o = new com .thinkdynamics .kanaha .dataacquisitionengine .Driver() { ... Thanks for looking at this and I will update the patch to handle synchronized/catch blocks tomorrow. Btw, my sametime id is igorf@ca.ibm.com, don't hesitate to contact me if you have any further questions or concerns.
If I understood correctly, this would add some boolean options for each of the aforementioned blocks? No problem to add them to the preference page.
The patch adds new value to existing configuration options. For UI it means that new items are added to existing drop-downs, but otherwise layout of the preference page does not change.
So you mean that for XXX_ALIGNMENT, more options are possible? Does this apply to ANY XXX_ALIGNMENT option? Else I'd have to do a major rewrite of this specific tab page.
My understanding is that this is true only for the open brace positioning. So you would have some different combo box between different kinds of blocks. This is why I asked you how easy it was for you to handle this.
I don't know if you've noticed that, but my patch is supposed to deal with this. It shows "Next line on wrap" option only for applicable block types. Implementation is somewhat clumsy though.
I am only looking at the JDT/Core part of the patch. I will update your code to handle all types of blocks. Then Silvio will update the code formatter preference page.
Fixed and released in HEAD. Regression test added. Silvio, please update the code formatter preference page. The new value NEXT_LINE_ON_WRAP can be used for all brace positionning except for the 'switch...case' statement. Not all cases with blocks are covered, because for some of them it would not change anything. Hopefully this corresponds to the requested feature.
I've checked the latest integration build (200401130800) and everything works as requested. One minor suggestion is to improve preferance page to show what this new options means.
Verified for 3.0-M7 with build I200402102000.
