Skip to content

Commit b213fb5

Browse files
SimonCroppSimonCropp
committed
Keep a regular literal on the call line
Only the raw form needs a line of its own, so its opening delimiter lines up with the content and the closing one. A regular literal has nothing to line up with, so it stays in the argument list.
1 parent 439c494 commit b213fb5

2 files changed

Lines changed: 47 additions & 34 deletions

File tree

‎src/DiffEngine.Tests/InlinePatcherTests.cs‎

Lines changed: 43 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public async Task ReplaceRegularLiteral()
2525
var source = Method(" await Snapshot(\"old\");");
2626
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"old\"", "new", out var newSource, out _);
2727
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
28-
await Assert.That(newSource).Contains(" await Snapshot(\n \"new\");");
28+
await Assert.That(newSource).Contains(" await Snapshot(\"new\");");
2929
}
3030

3131
[Test]
@@ -221,7 +221,7 @@ public async Task InsertIntoEmptyArgumentList()
221221
var source = Method(" await Snapshot();");
222222
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);
223223
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
224-
await Assert.That(newSource).Contains("await Snapshot(\n \"new\");");
224+
await Assert.That(newSource).Contains("await Snapshot(\"new\");");
225225
}
226226

227227
[Test]
@@ -230,7 +230,7 @@ public async Task InsertReplacesNullArgument()
230230
var source = Method(" await Snapshot(null, file, line);");
231231
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);
232232
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
233-
await Assert.That(newSource).Contains("await Snapshot(\n \"new\", file, line);");
233+
await Assert.That(newSource).Contains("await Snapshot(\"new\", file, line);");
234234
}
235235

236236
[Test]
@@ -283,11 +283,28 @@ public async Task AppendToABareVerify()
283283

284284
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);
285285

286+
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
287+
await Assert.That(newSource).Contains(
288+
" await Verify(value)\n" +
289+
" .Snapshot(\"new\");");
290+
}
291+
292+
// The raw form is the one that has to sit on its own line, indented under the call
293+
[Test]
294+
public async Task AppendMultiLineContent()
295+
{
296+
var source = Method(" await Verify(value);");
297+
298+
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "a\nb", out var newSource, out _);
299+
286300
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
287301
await Assert.That(newSource).Contains(
288302
" await Verify(value)\n" +
289303
" .Snapshot(\n" +
290-
" \"new\");");
304+
" \"\"\"\n" +
305+
" a\n" +
306+
" b\n" +
307+
" \"\"\");");
291308
}
292309

293310
// Snapshot terminates the chain, so it has to land after everything already chained on
@@ -304,8 +321,7 @@ public async Task AppendGoesAfterAnExistingChain()
304321
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
305322
await Assert.That(newSource).Contains(
306323
" .ScrubLinesContaining(\"x\")\n" +
307-
" .Snapshot(\n" +
308-
" \"new\");");
324+
" .Snapshot(\"new\");");
309325
}
310326

311327
[Test]
@@ -316,7 +332,7 @@ public async Task AppendToAnEntryPointOverload()
316332
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);
317333

318334
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
319-
await Assert.That(newSource).Contains("await VerifyXml(value)\n .Snapshot(\n \"new\");");
335+
await Assert.That(newSource).Contains("await VerifyXml(value)\n .Snapshot(\"new\");");
320336
}
321337

322338
// The verify call spans lines, so the closing paren is nowhere near the hint
@@ -333,7 +349,7 @@ public async Task AppendToAMultiLineVerifyCall()
333349
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);
334350

335351
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
336-
await Assert.That(newSource).Contains(" })\n .Snapshot(\n \"new\");");
352+
await Assert.That(newSource).Contains(" })\n .Snapshot(\"new\");");
337353
}
338354

339355
[Test]
@@ -403,8 +419,7 @@ public async Task AppendPrefersTheEntryPointOverANestedHelper()
403419
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
404420
await Assert.That(newSource).Contains(
405421
" await Verify(ContentValidation.Verify(value))\n" +
406-
" .Snapshot(\n" +
407-
" \"new\");");
422+
" .Snapshot(\"new\");");
408423
}
409424

410425
[Test]
@@ -415,7 +430,7 @@ public async Task AppendToAVerifierQualifiedCall()
415430
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);
416431

417432
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
418-
await Assert.That(newSource).Contains("await Verifier.Verify(value)\n .Snapshot(\n \"new\");");
433+
await Assert.That(newSource).Contains("await Verifier.Verify(value)\n .Snapshot(\"new\");");
419434
}
420435

421436
[Test]
@@ -426,7 +441,7 @@ public async Task AppendToAThisQualifiedCall()
426441
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);
427442

428443
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
429-
await Assert.That(newSource).Contains("await this.Verify(value)\n .Snapshot(\n \"new\");");
444+
await Assert.That(newSource).Contains("await this.Verify(value)\n .Snapshot(\"new\");");
430445
}
431446

432447
[Test]
@@ -522,9 +537,9 @@ public async Task RemoveWithNoSnapshotCall()
522537
public async Task TabIndentedFileUsesTabUnit()
523538
{
524539
var source = "class Tests\n{\n\tasync Task Test()\n\t{\n\t\tawait Snapshot();\n\t}\n}";
525-
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);
540+
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "a\nb", out var newSource, out _);
526541
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
527-
await Assert.That(newSource).Contains("Snapshot(\n\t\t\t\"new\");");
542+
await Assert.That(newSource).Contains("Snapshot(\n\t\t\t\"\"\"\n\t\t\ta\n\t\t\tb\n\t\t\t\"\"\");");
528543
}
529544

530545
[Test]
@@ -533,8 +548,7 @@ public async Task HintBeyondEndOfFile()
533548
var source = "await Snapshot();";
534549
var status = InlinePatcher.TryApply(source, 500, InlinePatchMode.Set, null, "new", out var newSource, out _);
535550
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
536-
// No newline in the file, so the splice falls back to the environment's
537-
await Assert.That(newSource).Contains($"Snapshot({Environment.NewLine} \"new\");");
551+
await Assert.That(newSource).Contains("Snapshot(\"new\");");
538552
}
539553

540554
[Test]
@@ -688,14 +702,14 @@ public async Task MixedEolFileLeavesUntouchedRegionsAlone()
688702
var suffix = "\r\n// trailing\n// mixed tail\n";
689703
var source = prefix + body + suffix;
690704

691-
var status = InlinePatcher.TryApply(source, 7, InlinePatchMode.Set, "\"old\"", "new", out var newSource, out _);
705+
var status = InlinePatcher.TryApply(source, 7, InlinePatchMode.Set, "\"old\"", "new1\nnew2", out var newSource, out _);
692706

693707
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
694708
// Untouched regions keep their original endings byte for byte
695709
await Assert.That(newSource.StartsWith(prefix, StringComparison.Ordinal)).IsTrue();
696710
await Assert.That(newSource.EndsWith(suffix, StringComparison.Ordinal)).IsTrue();
697711
// The spliced literal uses the file's dominant ending
698-
await Assert.That(newSource).Contains("Snapshot(\r\n \"new\");");
712+
await Assert.That(newSource).Contains("Snapshot(\r\n \"\"\"\r\n new1\r\n new2\r\n \"\"\");");
699713
}
700714

701715
[Test]
@@ -717,7 +731,7 @@ public async Task SameLiteralInTheVerifyArgumentIsNotPatched()
717731
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, "\"same\"", "changed", out var newSource, out _);
718732

719733
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
720-
await Assert.That(newSource).Contains("await Verify(\"same\").Snapshot(\n \"changed\");");
734+
await Assert.That(newSource).Contains("await Verify(\"same\").Snapshot(\"changed\");");
721735
}
722736

723737
// The expression search must match a whole argument, not the quoted part of a longer literal
@@ -759,7 +773,7 @@ public async Task CommentedOutCallIsSkipped()
759773

760774
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
761775
await Assert.That(newSource).Contains(" // await Verify(x).Snapshot(\"doc example\");\n");
762-
await Assert.That(newSource).Contains(" await Verify(x).Snapshot(\n \"new\");");
776+
await Assert.That(newSource).Contains(" await Verify(x).Snapshot(\"new\");");
763777
}
764778

765779
[Test]
@@ -773,7 +787,7 @@ public async Task CallInsideAStringIsSkipped()
773787

774788
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
775789
await Assert.That(newSource).Contains("var text = \"await Snapshot(\\\"x\\\")\";\n");
776-
await Assert.That(newSource).Contains("await Verify(x).Snapshot(\n \"new\");");
790+
await Assert.That(newSource).Contains("await Verify(x).Snapshot(\"new\");");
777791
}
778792

779793
// A declaration is a name followed by parens too, so it has to be told apart by what precedes it
@@ -809,8 +823,7 @@ public async Task AppendSkipsAVerifyPrefixedDeclaration()
809823
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
810824
await Assert.That(newSource).Contains(
811825
" Task VerifyThing(string value) => Verify(value)\n" +
812-
" .Snapshot(\n" +
813-
" \"new\");");
826+
" .Snapshot(\"new\");");
814827
}
815828

816829
// Snapshot terminates the chain, so a comment in the middle of one must not end the walk
@@ -826,8 +839,7 @@ public async Task AppendGoesAfterACommentInTheChain()
826839
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
827840
await Assert.That(newSource).Contains(
828841
" .UseDirectory(\"snapshots\")\n" +
829-
" .Snapshot(\n" +
830-
" \"new\");");
842+
" .Snapshot(\"new\");");
831843
}
832844

833845
[Test]
@@ -841,7 +853,7 @@ public async Task LiteralInACommentIsNotPatched()
841853

842854
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
843855
await Assert.That(newSource).Contains("// was \"old\"\n");
844-
await Assert.That(newSource).Contains("Snapshot(\n \"new\");");
856+
await Assert.That(newSource).Contains("Snapshot(\"new\");");
845857
}
846858

847859
[Test]
@@ -861,7 +873,7 @@ public async Task LiteralInAnotherMethodIsNotPatched()
861873

862874
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
863875
await Assert.That(newSource).Contains("void Helper() => Log(\"old\");");
864-
await Assert.That(newSource).Contains("Snapshot(\n \"new\");");
876+
await Assert.That(newSource).Contains("Snapshot(\"new\");");
865877
}
866878

867879
// The empty literal is two characters, and the opening of every raw literal holds a pair
@@ -885,7 +897,7 @@ public async Task EmptyOriginalIsNotMatchedInsideARawDelimiter()
885897

886898
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
887899
await Assert.That(newSource).Contains(" content\n \"\"\");");
888-
await Assert.That(newSource).Contains("Verify(b).Snapshot(\n \"new\");");
900+
await Assert.That(newSource).Contains("Verify(b).Snapshot(\"new\");");
889901
}
890902

891903
[Test]
@@ -896,7 +908,7 @@ public async Task GenericSnapshotCall()
896908
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);
897909

898910
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
899-
await Assert.That(newSource).Contains(".Snapshot<Thing>(\n \"new\");");
911+
await Assert.That(newSource).Contains(".Snapshot<Thing>(\"new\");");
900912
}
901913

902914
[Test]
@@ -907,7 +919,7 @@ public async Task AppendToAGenericVerify()
907919
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Append, null, "new", out var newSource, out _);
908920

909921
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
910-
await Assert.That(newSource).Contains("await Verify<Thing>(value)\n .Snapshot(\n \"new\");");
922+
await Assert.That(newSource).Contains("await Verify<Thing>(value)\n .Snapshot(\"new\");");
911923
}
912924

913925
[Test]
@@ -918,7 +930,7 @@ public async Task CommentInTheArgumentListIsNotTheArgument()
918930
var status = InlinePatcher.TryApply(source, 5, InlinePatchMode.Set, null, "new", out var newSource, out _);
919931

920932
await Assert.That(status).IsEqualTo(PatchStatus.Applied);
921-
await Assert.That(newSource).Contains("Snapshot(/* keep */\n \"new\");");
933+
await Assert.That(newSource).Contains("Snapshot(/* keep */\"new\");");
922934
}
923935

924936
[Test]

‎src/DiffEngine/Inline/InlinePatcher.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -744,11 +744,12 @@ static string RenderArgument(string source, List<int> lineStarts, int spanStart,
744744
}
745745

746746
/// <summary>
747-
/// Puts the literal on its own line rather than trailing the open paren, so a raw string's
748-
/// opening delimiter sits with its content and its closing one.
747+
/// Puts a raw literal on its own line rather than trailing the open paren, so its opening
748+
/// delimiter sits with its content and its closing one. A regular literal stays where it is,
749+
/// since it has nothing to line up with.
749750
/// </summary>
750751
static string OnOwnLine(string rendered, string indent, string eol) =>
751-
$"{eol}{indent}{rendered}";
752+
rendered.IndexOf('\n') == -1 ? rendered : $"{eol}{indent}{rendered}";
752753

753754
/// <summary>
754755
/// True when only whitespace precedes the offset on its line.

0 commit comments

Comments
 (0)