Bugzilla – Full Text Bug Listing
| Summary: | [typing] Incorrect indentation in string continuation (press Enter in front of +) | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] JDT | Reporter: | Markus Keller <markus.kell.r> | ||||||||
| Component: | Text | Assignee: | Rajesh <rthakkar> | ||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||
| Severity: | normal | ||||||||||
| Priority: | P1 | CC: | daniel_megert, deepakazad | ||||||||
| Version: | 3.7 | Flags: | deepakazad:
review-
daniel_megert: review+ |
||||||||
| Target Milestone: | 3.7 M7 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Markus Keller
The wrong indent stays when I use Correct Indentation again. The code formatter indents correctly (can be seen when you edit the default profile and check Line Wrapping > Never join already wrapped lines). (In reply to comment #0) > HEAD (same problem with and without fix for bug 337150), was OK in 3.6.1 Bug 337150 was about 'pasting' a string, which is a slightly different problem and involves different code than this one. (In reply to comment #2) > (In reply to comment #0) > > HEAD (same problem with and without fix for bug 337150), was OK in 3.6.1 > Bug 337150 was about 'pasting' a string, which is a slightly different problem > and involves different code than this one. It's probably the indent fix from bug bug 65317 that broke both scenarios. Created attachment 189981 [details]
Patch with tests.
Have rewritten and made more generic the method that was handling extra indentation for second line of string continuation, for e.g -
String test =
"this is the 1st string"
+ "this is the 1st string"
+ "this is the 1st string";
This is the one that was causing the issue. All the existing smart paste and other indentation tests pass. Also added more tests and modified one.
Created attachment 190105 [details]
more readable tests
Firstly it was impossible to understand what the tests were testing without some comments. I have added some comments and made them more readable, please use the attached patch. This is also true for a number of older tests in JavaHeuristicScannerTest but I have not touched those.
testContinuationIndentationOfStrings1 – Why is indentation different at pos 73 and 84 ? Shouldn’t both be “\t\t\t” ?
testContinuationIndentationOfStrings2 – third line – Are you sure it is “string:\n” but not “string:\\n” ?
testContinuationIndentationOfStrings2 - Shouldn't "definedType.toString()" on the new line before you check for indentation ?
- Use this snippet (first snippet in test testContinuationIndentationOfStrings1)
-----------------------------------------------------------------------
String[] i = new String[] {
"X.java",
"public class X extends B{"
+ "test"
+ " public "};
----------------------------------------------------------------------
- Place caret after 'e' in 'test'
- Press enter
=> This is the result, the new line is indented too much.
---------------------------------------------------------------------
String[] i = new String[] {
"X.java",
"public class X extends B{"
+ "te" +
"st"
+ " public "};
---------------------------------------------------------------------
- Use this snippet (second snippet in test testContinuationIndentationOfStrings1)
---------------------------------------------------------------------
String[] i = new String[] {
"X.java",
"public class X extends B{" +
"test" +
" public " };
---------------------------------------------------------------------
- Place caret after "class X"
- Press Enter
- Press Crtl+I => Indentation changes!
Ping! (In reply to comment #6) > Ping! The issues described by snippets in comment 5 are separate pre-existing issues. This is because Newline in a String is handled by completely different code (JavaStringAutoIndentStrategy.java). This class computes the indent for newline cases where the caret was in a string and is completely independent of the JavaIndenter which computes the indent for Ctrl+I, etc. I spent time looking at that code and found various issues there. We should handle these cases separately as it might make sense to modify that code to call the common JavaIndenter instead of separately computing the indent there. Created attachment 191082 [details] Patch with tests. (In reply to comment #5) > testContinuationIndentationOfStrings1 – Why is indentation different at pos 73 > and 84 ? Shouldn’t both be “\t\t\t” ? because pos 73 is second line of string continuation it gets extra indent, whereas pos 84 being third line of string continuation only aligns with the previous line. > > testContinuationIndentationOfStrings2 – third line – Are you sure it is > “string:\n” but not “string:\\n” ? Actually neither - it should be "string:\\n\" > > testContinuationIndentationOfStrings2 - Shouldn't "definedType.toString()" on > the new line before you check for indentation ? nope. As mentioned in my previous comment, the issue described in the snippets is separate from this bug and requires a re-look at the way newline in a string is handled. I have still made a very small change to JavaStringAutoIndentStrategy.java to address most of the snippet scenarios. Accepted the additions of comments to the test. Rajesh, the patch looks good. Committed to HEAD. Available in builds >= N20110413-2000. Verified in I20110425-1800. |