Skip to content

Commit 24ab192

Browse files
Fix issue where we weren't properly preserving indentation when pasting into a string literal (#79401)
2 parents 1d8a729 + 6021f05 commit 24ab192

File tree

3 files changed

+73
-9
lines changed

3 files changed

+73
-9
lines changed

src/EditorFeatures/CSharp/StringCopyPaste/StringCopyPasteHelpers.cs

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@
77
using System.Diagnostics.CodeAnalysis;
88
using System.Globalization;
99
using System.Linq;
10+
using Microsoft.CodeAnalysis.Collections;
1011
using Microsoft.CodeAnalysis.CSharp;
1112
using Microsoft.CodeAnalysis.CSharp.Syntax;
1213
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
1314
using Microsoft.CodeAnalysis.PooledObjects;
1415
using Microsoft.CodeAnalysis.Text;
1516
using Microsoft.CodeAnalysis.Text.Shared.Extensions;
17+
using Microsoft.VisualStudio.Debugger.Contracts.EditAndContinue.VsdbgIntegration;
1618
using Microsoft.VisualStudio.Text;
1719

1820
namespace Microsoft.CodeAnalysis.Editor.CSharp.StringCopyPaste;
@@ -497,19 +499,39 @@ static bool WillEscapeAnyCharacters(bool isInterpolated, string value)
497499
string? commonIndentPrefix = null;
498500
var first = true;
499501

502+
using var allLines = TemporaryArray<TextLine>.Empty;
503+
500504
foreach (var change in textChanges)
501505
{
502506
var text = SourceText.From(change.NewText);
503507
foreach (var line in text.Lines)
504508
{
505-
if (first)
506-
{
507-
first = false;
508-
continue;
509-
}
510-
511509
var nonWhitespaceIndex = GetFirstNonWhitespaceIndex(text, line);
512-
if (nonWhitespaceIndex >= 0)
510+
511+
// For the first line, we only want to consider its indentation if it *has* any. That's because
512+
// people often copy by avoiding indentation on the first line and starting their selection on the
513+
// first real construct. e.g.:
514+
//
515+
// <ws>[|Goo
516+
// <ws >Bar
517+
// <ws >Baz|]
518+
//
519+
// In this case, we don't want to say that there is no common indentation to trim since there is
520+
// no indentation on the selection's first line. However, we do want to trim if the user selected
521+
// whitespace a-la:
522+
//
523+
// [|<ws>Goo
524+
// <ws >Bar
525+
// <ws >Baz|]
526+
//
527+
// In this case, we really only want to trim the common whitespace of all three lines, not the whitespace
528+
// of the second/third lines. If we do the latter, we'd end up with Goo and Bar being aligned, which
529+
// doesn't match the original intent.
530+
531+
var minimumStartColumn = first ? 1 : 0;
532+
first = false;
533+
534+
if (nonWhitespaceIndex >= minimumStartColumn)
513535
commonIndentPrefix = GetCommonIndentationPrefix(commonIndentPrefix, text, TextSpan.FromBounds(line.Start, nonWhitespaceIndex));
514536
}
515537
}
@@ -554,6 +576,11 @@ public static bool RawContentMustBeMultiLine(SourceText text, ImmutableArray<Tex
554576
return true;
555577

556578
// or contains a newline
579+
return SpansContainsNewLine(text, spans);
580+
}
581+
582+
public static bool SpansContainsNewLine(SourceText text, ImmutableArray<TextSpan> spans)
583+
{
557584
foreach (var span in spans)
558585
{
559586
for (var i = span.Start; i < span.End; i++)

src/EditorFeatures/CSharp/StringCopyPaste/UnknownSourcePasteProcessor.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,15 @@ private void AdjustWhitespaceAndAddTextChangesForSingleLineRawStringLiteral(Arra
168168
if (mustBeMultiLine)
169169
edits.Add(new TextChange(new TextSpan(StringExpressionBeforePasteInfo.StartDelimiterSpan.End, 0), NewLine + IndentationWhitespace));
170170

171-
SourceText? textOfCurrentChange = null;
172-
var commonIndentationPrefix = GetCommonIndentationPrefix(Changes) ?? "";
171+
// Only if we're ending with a multi-line raw string do we want to consider the first line when determining
172+
// the common indentation prefix to trim out. If we don't have a multi-line raw string, then that means we
173+
// pasted a boring single-line string into a single-line raw string, and in that case, we don't want to touch
174+
// the contents at all.
175+
var commonIndentationPrefix = SpansContainsNewLine(TextAfterPaste, TextContentsSpansAfterPaste)
176+
? GetCommonIndentationPrefix(Changes) ?? ""
177+
: "";
173178

179+
SourceText? textOfCurrentChange = null;
174180
foreach (var change in Changes)
175181
{
176182
// Create a text object around the change text we're making. This is a very simple way to get

src/EditorFeatures/CSharpTest/StringCopyPaste/PasteUnknownSourceIntoMultiLineRawStringTests.cs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,37 @@ public void TestNormalTextIntoMultiLineRawStringMultiLine13()
753753
"""");
754754
}
755755

756+
[WpfFact, WorkItem("https://github.com/dotnet/roslyn/issues/74661")]
757+
public void TestNormalTextIntoMultiLineRawStringMultiLine14()
758+
{
759+
TestPasteUnknownSource(
760+
pasteText: """
761+
abc
762+
def
763+
ghi
764+
""",
765+
""""
766+
var x = """
767+
[||]
768+
"""
769+
"""",
770+
""""
771+
var x = """
772+
abc
773+
def
774+
ghi[||]
775+
"""
776+
"""",
777+
afterUndo:
778+
""""
779+
var x = """
780+
abc
781+
def
782+
ghi[||]
783+
"""
784+
"""");
785+
}
786+
756787
[WpfFact]
757788
public void TestNormalTextIntoMultiLineRawStringHeader1()
758789
{

0 commit comments

Comments
 (0)