Community
Participate
Working Groups
Steps: copy-paste the following fragment boolean a = true || false; boolean b = false; into method a() of the following class: public class A { void a() { } } Observed: The resulting code identation is public class A { void a() { boolean a = true || false; boolean b = false; } } Expected: public class A { void a() { boolean a = true || false; boolean b = false; } } Build: 200405280010
Created attachment 11464 [details] My code formatter profile
3.0 candidate
Removing target milestone, no further action for 3.0.
*** Bug 66390 has been marked as a duplicate of this bug. ***
See also bug 66390 for throws clause continuation.
*** Bug 72339 has been marked as a duplicate of this bug. ***
see also bug 72339 for another continuation case.
*** Bug 72247 has been marked as a duplicate of this bug. ***
*** Bug 107833 has been marked as a duplicate of this bug. ***
*** Bug 108481 has been marked as a duplicate of this bug. ***
*** Bug 108583 has been marked as a duplicate of this bug. ***
*** Bug 130526 has been marked as a duplicate of this bug. ***
Has to be shifted to 3.3.
Moving back to inbox.
*** Bug 153819 has been marked as a duplicate of this bug. ***
Another problematic fragment: for (int i = 0; i < array.length; i++) { x = 5; } Observed: for (int i = 0; i < array.length; i++) { x = 5; } Expected: for (int i = 0; i < array.length; i++) { x = 5; }
Could you please schedule this bug to be fixed before 3.3 goes out. People keep running into this problem again and again as the number of duplicate bugs indicates.
Sorry, we currently do not have time to work on this but gladly accept a good quality patch.
*** Bug 322913 has been marked as a duplicate of this bug. ***
Created attachment 178439 [details] Patch This patch seems to handle many of the samples in the various duplicates of this bug (there are a couple such as method declaration that still remain). Markus, can you please provide your comments on the approach/direction of the patch so far.
Created attachment 179248 [details] Fix This patch covers the common continuation scenarios described in this and other duplicate bugs. Specifically it covers Return statements, Assignments, Comparisons, For loops and a bug with pasting statements containing continuation.
Rajesh, the change causes the IndentActionTest to fail. Also, I quickly tried some examples which did not seem to work. I think the first thing to do, is to list the cases (with examples) that are broken and which we will fix with the patch and also add corresponding tests.
Created attachment 179443 [details] Fix (In reply to comment #22) > I think the first thing to do, is to > list the cases (with examples) that are broken and which we will fix with the > patch and also add corresponding tests. Following are the examples for the scenarios that the fix attempts to address - 1. Copy-paste the following fragment, it should indent the same- boolean a = true || false; boolean b = false; 2. Copy-paste the following and place the cursor anywhere after 'return' and press Enter. Should indent with 2 tabs(default setting) relative to this line. private String m() { return "I'm such a long string that you have to split me to see the whole line without scrolling around"; } 3. Copy-paste the following fragment and place cursor anywhere between parenthesis and press Enter. Should indent with 2 tabs relative to this line. int[] array= {1,2,3}; for (int i = 0; i < array.length; i++) { } 4. Copy-paste the following code it should maintain the indentation. Then place cursor somewhere in the 3rd line and press Enter - should indent correctly. Also, try 'Correct Indentation', it should maintain the indentation. boolean test(int thisIsAVeryLongName, int anotherVeryLongName) { return (thisIsAVeryLongName == 1 && anotherVeryLongName == 1) || thisIsAVeryLongName == 2; // 2 more tabs } 5. Enter the following and press Enter. Should indent with 2 tabs relative to this line. int i= or int i= 5 + Also, found a issue with the previous fix - attached new Fix.
Comment on attachment 179443 [details] Fix With this patch we get test failures in org.eclipse.jdt.text.tests.JavaHeuristicScannerTest and org.eclipse.jdt.text.tests.IndentActionTest. Also, please add the new tests for the cases we fix.
Created attachment 180194 [details] Fix Fixed the test breaks by modifying the tests to test continuation scenarios appropriately. Also, added many new Tests for the supported continuation scenarios.
>Fixed the test breaks by modifying the tests I could not see that. Only saw that you deleted some tests which for a reviewer looks like cheating ;-). Can't you adjust those tests? >org.eclipse.jdt.text.tests.IndentActionTest. I could not see changes in there but the failure is gone. Why, i.e. what was wrong with the previous patch and what did you change in the "real" code that fixed this? Also note that your patch introduces compile warnings. Please correct that.
(In reply to comment #26) > >Fixed the test breaks by modifying the tests > I could not see that. Only saw that you deleted some tests which for a reviewer > looks like cheating ;-). Can't you adjust those tests? I did not delete the tests, they have been adjusted and renamed (previous name didn't make sense anymore)- look at testContinuationIndentation2 to 5. Hope that doesn't fall under cheating :) > > >org.eclipse.jdt.text.tests.IndentActionTest. > I could not see changes in there but the failure is gone. Why, i.e. what was > wrong with the previous patch and what did you change in the "real" code that > fixed this? The change in the code was in isForStatement method where I removed skipping the scope of a RParen which was a bug under some scenarios.
> I did not delete the tests, they have been adjusted and renamed (previous name > didn't make sense anymore)- look at testContinuationIndentation2 to 5. Hope > that doesn't fall under cheating :) No, but it just makes the reviewer job harder because the compare editor is not able to show this.
Looks good. I've fixed the warnings that your patch introduced. Available in builds >= N20101006-2000.
Verified in I20101025-1800, reopened dups that have not been fixed yet.
Continuation of Method calls is also not working yet (filed bug Bug 331353).
All recently made indent fixes had to be reverted due to several regressions (bug 331028, bug 330556 and bug 331734).
Committed Rajesh's modified patch (patch will be attached soon).
Created attachment 187942 [details] Patch
*** Bug 332015 has been marked as a duplicate of this bug. ***
*** Bug 75573 has been marked as a duplicate of this bug. ***
