Community
Participate
Working Groups
Build Identifier: 20110916-0149 Examples of my preferred formatting/wrapping style: If max line length not exceeded: void exampleMethod(ExampleClass1 exampleparam1, ExampleClass2 exampleParam2) { // ... } If max line length exceeded: void exampleMethod( ExampleClass1 exampleparam1, ExampleClass2 exampleParam2 ) { // ... } The closest that I can currently get to that with Eclipse's formatter is this: void exampleMethod( ExampleClass1 exampleparam1, ExampleClass2 exampleParam2) { // ... } I can get it to put each parameter on a separate line and use regular indentation for them, but there doesn't seem to be an option to put the ending parenthesis on a new line. Having such an option would be greatly appreciated :) Another example with class declarations: class ChildClass extends ParentClass implements ExampleInterface { // ... } This is achievable if I set opening braces to always be on a new line, but I'd still like it to remain on the same line if the line does not exceed the max length: class ChildClass extends ParentClass { // ... } Plus, even if I set braces to be on a new line, closing parenthesis will still remain on the previous line when wrapped. More examples of preferred wrapping in other areas: if ( exampleValue1 == exampleValue2 && exampleValue3 == exampleValue4 ) { // ... } exampleObject.exampleMethod( exampleParam1, exampleParam2, exampleParam3, exampleParam4 ); try ( ExampleClass1 exampleObject1 = new ExampleClass1(); ExampleClass2 exampleObject2 = new ExampleClass2() ) { // ... } catch ( ExampleExceptionClass1 | ExampleExceptionClass2 | ExampleExceptionClass3 e ) { // ... } Reproducible: Always
We'll wait to see how bug 303519 pans out before considering this. (A part of it may be addressed there itself)
I'd love to have functionality described in this issue. I also prefer this style of argument formatting, but not being able to set it in preferences is really annoying. Has this already been implemented in some plugins?
Any updates on this?
*** Bug 151768 has been marked as a duplicate of this bug. ***
This needs a new settings tab for parenthesis. It's similar to "Braces" page, with each item having the following selection: - same line - next line if not empty - next line if wrapped - next line - do not touch Available categories would be 'Closing parenthesis in ...' - method declaration - method invocation - [x] same line after another parenthesis - enum constant declaration - if & while statement - for statement - switch statement - try clause - catch clause - annotation - lambda declaration Let me know if you see any potential problems/improvements. Markus, please advise on a shortcut key for the new tab. Every letter is already used in another tab. Can a new tab have ho shortcut? Duplicate? Rearrange existing tab?
*** Bug 373627 has been marked as a duplicate of this bug. ***
New Gerrit change created: https://git.eclipse.org/r/67998
New Gerrit change created: https://git.eclipse.org/r/67999
This is quite a big change, but I hope we'll manage to review, polish and push it in before the API freeze. For now I won't push jdt.core changes directly in case there are some general design issues. I've dropped the "same line after another parenthesis" option. I feel something like that can be useful, but I'm not sure how exactly it should work, so maybe for later.
Gerrit change https://git.eclipse.org/r/67998 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=f9a46f3b03f3a4f0a3f6d3a22503ef1fa89762cc
Temporarily moving to JDT UI for preferences page part. The JDT Core part has been commited to M6 without review as there was no time before API freeze. The UI part is a new feature so it must be commited before M7 or wait for another year. Will you find the time to look at it?
(In reply to Mateusz Matela from comment #11) > Temporarily moving to JDT UI for preferences page part. See comments on Gerrit.
Moving this bug back to jdt.core. We always need two separate bugs, one for jdt.core and one for jdt.ui which depends on the core bug, otherwise the discussion gets mixed up.
(In reply to Noopur Gupta from comment #13) > We always need two separate bugs, one for jdt.core and one for jdt.ui which > depends on the core bug, otherwise the discussion gets mixed up. Oh, OK. From my perspective all this discussion is about a single set of features and the core/ui separation seems rather artificial... Do you want me to submit a separate bug or is it too late for that? (In reply to Noopur Gupta from comment #12) > See comments on Gerrit. Thank you for the remarks, although is some cases I wish they were more constructive :) > The new options under each position look confusing. I am not sure > what is meant by "Common lines" Yeah, I didn't know how to put it. "Separate lines" is rather clear - it means there's a line break between the opening paren and the parens' contents, and also between the contents and the closing paren. So what's the opposite of that - when there are no line breaks? "Same line(s)" also doesn't sound good, because there's more than one line if the contents are wrapped. > and the use of "Preserve positions" which should be same as the default. It preserves line breaks that were there before formatting. It's hard to represent in preview because the user doesn't see the original source. So I've just put a few line breaks here and there hoping users can figure it out, but for most items it looks like "Preserve positions" == "Common line". What can we do? There's the same problem with "Never join already wrapped lines" option. > "Separate lines if not empty" > doesn't clearly specify what should not be empty for a listed position. Well, the contents between parentheses should not be empty, what else could it be? Change it to "Separate lines if parameters list is not empty"? That's too long... > Further, these options not only change the position of > the closing parenthesis but also move the arguments, which may be > desired as per the bug reports, but doesn't match the title. My first draft in comment 5 used "closing parenthesis" in title, but now it's just "Parentheses positions". It's kind of correct - even if you can say the arguments are moved relative to opening paren, you can also say it's opening paren moved relative to the arguments :) > Preview should have examples for all the positions and their values > from the drop-down, preferably in the order of the positions listed > on the left. If it is still not clear, having separate preview for > each position would be good (like the ones for options under each > node on 'White Space' tab). I see I've missed 'switch' statement and lambda declaration. Is it enough if I add them? Having more complex UI like 'White space' tab would make preview a bit more clear, but will require user to click more (click once to see preview and available options, click again to select you option). > Additional comments: > - All the options should be in singular i.e. "lines" => "line" and > "positions" => "position". > - The group title on left should be "Parenthesis positions". Are you sure? I think it's about opening paren and closing paren, so plural. > - Have a separate option for 'if' statement. Too late for that. We can add it in a next version if requested. > - There can be a combined option for 'while' & 'do while' > statements, with preview examples for both. OK, there are examples for both. > - Mnemonics are missing for all the positions and tab title. About tab title - every letter of 'parentheses' is already used in another tab. Can I add a duplicated shortcut? Should I rearrange existing tabs?
(In reply to Mateusz Matela from comment #14) > (In reply to Noopur Gupta from comment #13) > > We always need two separate bugs, one for jdt.core and one for jdt.ui which > > depends on the core bug, otherwise the discussion gets mixed up. > > Oh, OK. From my perspective all this discussion is about a single set of > features and the core/ui separation seems rather artificial... Do you want > me to submit a separate bug or is it too late for that? Yes please. The separation makes sense and is not artificial. It's how JDT works since day 1. > > The new options under each position look confusing. I am not sure > > what is meant by "Common lines" I agree with Noopur that the whole UI is confusing and hard to understand without good preview. I'd love to give better labels, but I haven't fully understood yet what each of the options is really supposed to do. Even after reading comment 14. > > Additional comments: > > - All the options should be in singular i.e. "lines" => "line" and > > "positions" => "position". > > - The group title on left should be "Parenthesis positions". > Are you sure? I think it's about opening paren and closing paren, so plural. Plural is fine here, same as Braces. > > - Mnemonics are missing for all the positions and tab title. > About tab title - every letter of 'parentheses' is already used in another > tab. Can I add a duplicated shortcut? Should I rearrange existing tabs? No, not rearrange tabs, but you can try whether one of the other tab mnemonics can be freed for the 'Parentheses' tab by using a different mnemonic that's not used on its page. Changing mnemonics on the page to achieve that is fine, but make sure it does not collide with existing tab mnemonics.
Submitted bug 491731 for the preferences page, so I think we can close this one.
In order to understand the UI options better, I checked the new constants added in DefaultCodeFormatterConstants in M6 and I see the following problems: - All formatter options have the default as 'END_OF_LINE' in Javadoc which is not even a possible value for these options. - Same Javadoc has been copied for all the formatter values (except the last one i.e. PRESERVE_POSITIONS). - Keep only one dot at the end of the Javadoc comments. - SEPARATE_LINES_IF_NOT_EMPY should not specify options of try, catch etc. in the @see tag which are not applicable here. Please update the Javadocs with correct and useful description. Further, from UI, I see COMMON_LINES used as the default for all the options, which may result in changes in the existing source, so maybe it should be set to PRESERVE_POSITIONS. I would suggest Jay/Manoj to also have a look at this and reconsider these new constants, the default values, and whether all these values will actually be useful. The UI labels and preview can be updated after that in bug 491731.
(In reply to Noopur Gupta from comment #17) > In order to understand the UI options better, I checked the new constants > added in DefaultCodeFormatterConstants in M6 and I see the following > problems: Fixed with http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=a8b6a8d76efee58d7c7ebb7b8172e6680af4ebc2 > Further, from UI, I see COMMON_LINES used as the default for all the > options, which may result in changes in the existing source, so maybe it > should be set to PRESERVE_POSITIONS. If the existing source has been formatted with the previous version of Eclipse, then parentheses have already been moved to common lines. So I think we should keep the previous behavior as default.
Keeping it open until we have a go from the reviewer
Manoj, please review the jdt.core changes.
(In reply to Noopur Gupta from comment #17) > I would suggest Jay/Manoj to also have a look at this and reconsider these > new constants, the default values, and whether all these values will > actually be useful. > Usefulness is very subjective since One man's meat is another man's poison - so take this with a pinch of salt :) I understand that "preserve_positions" helps when you are adding code into an already existing code and do not want to disturb the existing code format (probably for minimal diff) - However, I don't see this option for "Braces" - is there any compelling reason to give this option for parenthesis when compared to "Braces"? -minor comment: SEPARATE_LINES_IF_NOT_EMPY -> SEPARATE_LINES_IF_NOT_EMPTY
(In reply to Manoj Palat from comment #21) > I understand that "preserve_positions" helps when you are adding code into > an already existing code and do not want to disturb the existing code format > (probably for minimal diff) - However, I don't see this option for "Braces" > - is there any compelling reason to give this option for parenthesis when > compared to "Braces"? I think it could be added for braces too, but that's for another task :) The main motivation is bug 471578, to preserve positions in method invocations which are very diverse and sometimes it makes sense to keep parens separated and sometimes not. For other items it's definitely less useful, but I see no particular reason not to include them. > -minor comment: SEPARATE_LINES_IF_NOT_EMPY -> SEPARATE_LINES_IF_NOT_EMPTY Oh, right, I'll fix it when I get back from work.
(In reply to Mateusz Matela from comment #22) > (In reply to Manoj Palat from comment #21) > > -minor comment: SEPARATE_LINES_IF_NOT_EMPY -> SEPARATE_LINES_IF_NOT_EMPTY > Oh, right, I'll fix it when I get back from work. Done with http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=9acb9fa15df23002caee282c972b7a5d0b9fdb55
(In reply to Manoj Palat from comment #21) > -minor comment: SEPARATE_LINES_IF_NOT_EMPY -> SEPARATE_LINES_IF_NOT_EMPTY This was an API change after the freeze, which should have been posted to the PMC list for approval. Anyway, I don't see any issues with this change, hence directly approving here.
(In reply to Dani Megert from comment #24) > (In reply to Manoj Palat from comment #21) > > -minor comment: SEPARATE_LINES_IF_NOT_EMPY -> SEPARATE_LINES_IF_NOT_EMPTY > > This was an API change after the freeze, which should have been posted to > the PMC list for approval. Anyway, I don't see any issues with this change, > hence directly approving here. Thanks Dani for the approval. @Noopur: Do you see any more blockers? If not please approve.
(In reply to Manoj Palat from comment #25) > (In reply to Dani Megert from comment #24) > > (In reply to Manoj Palat from comment #21) > > > -minor comment: SEPARATE_LINES_IF_NOT_EMPY -> SEPARATE_LINES_IF_NOT_EMPTY > > > > This was an API change after the freeze, which should have been posted to > > the PMC list for approval. Anyway, I don't see any issues with this change, > > hence directly approving here. > > Thanks Dani for the approval. > @Noopur: Do you see any more blockers? If not please approve. ^^^^^^ Clarifying: Noopur - what I meant is put a + in the review if you don't find any more blockers.
(In reply to Manoj Palat from comment #26) > > @Noopur: Do you see any more blockers? If not please approve. > > ^^^^^^ > Clarifying: Noopur - what I meant is put a + in the review if you don't find > any more blockers. The review- was for the UI patch. I'll remove it.
Verified for Eclipse Neon 4.6 M7 Build id: I20160427-2000
*** Bug 471578 has been marked as a duplicate of this bug. ***
