Comment 1 Deepak Azad 2011-02-14 13:25:35 EST

This works correctly in 3.6. 

I guess, if the first line does not contain anything else other than the string there is no need to indent the second line.

Comment 2 Dani Megert 2011-02-15 06:03:11 EST

Another (new) regression caused by fix for bug 65317.

Comment 3 Rajesh 2011-02-16 21:40:45 EST

Dani - based on our discussion, I looked at this further and it still remains tricky. Here is why -

1. As per formatter, second line of string continuation should be indented.
2. The Indenter does that in some cases but not all (which is a known issue that needs to be fixed)

The 2 options we have are either to not indent the second line(just need to make a small change) or handle SmartPaste as a special condition (as we had discussed).

Comment 4 Dani Megert 2011-02-17 10:06:29 EST

Looked at it in more detailed and discussed with Markus. We should update the logic when pasting as follows:
1.  compare the indent of first and second line of pasted stuff
2a. first line indent >= second line ==> only use first line to compute indent
2b. first line indent < second line use second line to compute indent and
    also adjust/indent first line according to formatter rules

For the example in comment 0 it means that depending on the string that's pasted, the result from comment 0 might or might not be expected:

If the selection contains:
                "this is the new 2nd string"+
                "this is the new 2nd string"+
                "this is the new 2nd string"+
                "this is the new 2nd string"+
                "this is the new 2nd string"
(note the leading indent) then the result should be as Deepak expects. Same with this selection:
"this is the new 2nd string"+
"this is the new 2nd string"+
"this is the new 2nd string"+
"this is the new 2nd string"+
"this is the new 2nd string"

However, if the selection contains:
"this is the new 2nd string"+
          "this is the new 2nd string"+
          "this is the new 2nd string"+
          "this is the new 2nd string"+
          "this is the new 2nd string"
then the (expected) result is as outlined in comment 0.

Comment 5 Dani Megert 2011-02-17 10:13:15 EST

Note that we can also see/test this with a simpler example:

class A {
	void foo() {
				String array = "this is the 1st string"
				+ "this is the 1st string"
				+ "this is the 1st string";
	}
}

1. fully select all three lines of foo's body
2. copy
3. paste
==> the result in this case should be:
class A {
	void foo() {
		String array = "this is the 1st string"
		+ "this is the 1st string"
		+ "this is the 1st string";
	}
}

In case the user selects from the first non-whitespace character to the last non-whitespace character in the body, the result should be:
class A {
	void foo() {
		String array = "this is the 1st string"
				+ "this is the 1st string"
				+ "this is the 1st string";
	}
}

Comment 6 Rajesh 2011-02-23 00:46:41 EST

Created attachment 189577 [details]
Patch with tests.

Comment 7 Deepak Azad 2011-02-23 09:49:14 EST

The patch works as outlined in comment 4.

Code review
- rename firstLineCurrentIndent to firstLineIndent

- In JavaAutoIndentStrategyTest the already existing tests- testPasteDefaultAtEnd(), testPasteFooAtEnd(), testPasteLongStringWithContinuations() - need an assert statement.

- testPasteLongStringWithContinuations and the new tests should have similar names.

- javaAutoIndentStrategy, documentCommand, accessor can be converted to fields and initialized in the constructor - reduces lines of code

- accessor.invoke(...) calls can be extracted to a method e.g. performPaste() - reduces code duplication and makes it easy to read the tests.

Comment 8 Rajesh 2011-02-24 15:24:13 EST

Created attachment 189742 [details]
Patch with tests.

Comment 9 Deepak Azad 2011-02-25 00:54:12 EST

Thanks for the patch!

Few minor things
- Fields should be prefixed with ‘f’
- In testPasteAndIndentOfLongStringWithContinuations1() “\n” inside the string should be escaped as per the test case in bug 330556
- It is also a good practice to mention bug numbers in the tests.
- Added one more test for the case when first line indent < second line.

Committed the patch to HEAD with these changes.

Comment 10 Dani Megert 2011-03-02 05:09:24 EST

Verified that pasting works as outlined in this bug, but there's now bug 338229 regarding the auto-indent on 'Enter'.

Comment 11 Dani Megert 2011-03-15 10:28:07 EDT

Verified in I20110310-1119 (there's still bug 338229).

Comment 12 Olivier Thomann 2011-05-04 14:02:00 EDT

I still have issues regarding indentation.
Go to: org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest.test293()
Copy the expected output string and replace it with "".
Now select "" and paste the string you copied before.
I still get an identation that I don't expect.

Comment 13 Dani Megert 2011-05-05 02:40:38 EDT

(In reply to comment #12)
> I still have issues regarding indentation.
> Go to: org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest.test293()
> Copy the expected output string and replace it with "".
> Now select "" and paste the string you copied before.
> I still get an identation that I don't expect.

Olivier, can you please file a new bug. Best with a little snippet, so that one doesn't have to load the whole JDT Core test project. Thanks.

Comment 14 Dani Megert 2011-06-01 02:35:10 EDT

See bug 347840 for a remaining issue.