Bugzilla – Full Text Bug Listing
| Summary: | [formatter] Comments in switch-case wrong aligned in some cases | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Benjamin Pasero <bpasero> | ||||||
| Component: | Core | Assignee: | Mateusz Matela <mateusz.matela> | ||||||
| Status: | CLOSED DUPLICATE | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P5 | CC: | daniel_megert, mateusz.matela, parker.david.a | ||||||
| Version: | 3.2 | ||||||||
| Target Milestone: | 4.5 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows 2000 | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
Indeed the formatter is not doing a specific treatment for return or continue statements. The second "/** My Comment */" should always have the same identation of the case statement regardless of the fact that the previous statement is a break, a return or anything else. In fact we do have a problem here.
I am not sure I can make the assumption that the comment should always be indented like a case statement.
For cases like:
switch (i) {
case 0:
// handling 0
System.out.println(\"Case was 0\");
// fall through
case 1:
// handling 1.
// some additional remark (also handles 0).
System.out.println(\"Case was 0 or 1\");
break;
case 2:
// fall through
default:
// some default action\n" +
System.out.println(\"Case was :\" + i);
}
How can we decide how the diffent comments should be indented compare to the other statements around them?
Daniel,
If you have any suggestions, they are welcome.
So I think the question is more.
Should a comment be considered as a statement in term of indentation?
This would mean that a comment would end up being indented compare to the case statements based on the preferences "Statement within switch body" or "Statement within case body" according to their positions relative to the cases inside the switch statements.
So your given test case would be:
switch (0) {
/** My Comment */
case 0:
break;
/** My Comment */
case 1:
break;
}
or:
switch (0) {
/** My Comment */
case 0:
return;
/** My Comment */
case 1:
break;
}
since this is still part of the "case" body. The first comment is not considered to be part of a case body, so it is indented as a switch body.
And the second example:
switch (i) {
case 0:
// handling 0
System.out.println(\"Case was 0\");
// fall through
case 1:
// handling 1.
// some additional remark (also handles 0).
System.out.println(\"Case was 0 or 1\");
break;
case 2:
// fall through
default:
// some default action\n" +
System.out.println(\"Case was :\" + i);
}
Would this make sense ?
Then we would end up with a consistent indentation in both cases.
Created attachment 59285 [details]
Proposed fix
Created attachment 59286 [details]
Regression tests
The patch implements the behavior described in comment 4. Right, I can see the problem now, thanks for the clarification. I agree that comments should be treated in a consistent way, though I am not very happy with the visual result (but I think i can live with it :-) ). Ben If you have a better proposal, please let me know. For me as long as the treatment is consistent, it is good enough. Well, I can see the problem for the case of a fall-through, but wouldn't it be possible to detect a return-statement (the same way a break statement is detected) and consider the following comment to be part of the following switch-block? Sure I could do that. But then we would need to rephrase the "break statement" indentation option to cover return, throw and continue statements as well. Daniel, Any thoughts on this? Comment handling is always fuzzy and depending on where we do it we have special rules (e.g. move refactoring). I think a natural solution to treat those comments is: 1. if below or above the comments are 0..n empty line(s) then align the comments to the statement that is not separated by empty lines (that would handle comment 0) 2. apply the strategy from comment 4 (In reply to comment #12) > 1. if below or above the comments are 0..n empty line(s) then align the > comments to the statement that is not separated by empty lines (that would > handle comment 0) To be honest, I don't like this one too much. Empty lines might be removed by the code formatter. So basing a behavior on empty lines looks awkwards. > 2. apply the strategy from comment 4 I'll go with this. At least the issue in comment 0 would be fixed and the second comment would be indented the same way in both cases. There is no perfect way to format comments. >So basing a behavior on empty lines looks awkwards.
Maybe for you as a coder but from a user point of view it would be natural to do this.
So what about lines breaks in the middle of the case body ? >So what about lines breaks in the middle of the case body ?
What do you mean with "in the middle"?
Ownership has changed for the formatter, but I surely will not have enough time to fix your bug during the 3.5 development process, hence set its priority to P5. Please finalize the posted patch if you definitely need the bug to be fixed in this version and I'll have a look at it... TIA *** Bug 437223 has been marked as a duplicate of this bug. *** My bug report for Eclipse 4.3.2 (bug 437223) was marked as a duplicate of this bug from Eclipse 3.5. The last comment on this bug was in 2008. Has this really been unfixed for 6 years, across several new releases? Can this please be fixed soon? *** This bug has been marked as a duplicate of bug 463590 *** |
When I run code-format on these two switch-case conditions, I get a different result for the alignment of comments, though I expect the same result. And I think in 3.1 I did get the same result: switch (0) { /** My Comment */ case 0: break; /** My Comment */ case 1: break; } and switch (0) { /** My Comment */ case 0: return; //Changed to return here! /** My Comment */ case 1: break; } The second comment is not correctly formatted. I think the formatter is not handling a return like a break, though it should I guess. Same for the keywork "continue" btw. Ben