Skip to content

Follow ups to Generate Method #12972

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
davidwengier merged 5 commits into from
Mar 31, 2026
Merged

Follow ups to Generate Method #12972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
519c609
Use the string builder more to avoid a bunch of intermediate strings
davidwengier Mar 30, 2026
5a2d63c
Avoid substring creation in TryGetMappedEdit
davidwengier Mar 30, 2026
c5c4146
Avoid iterator
davidwengier Mar 30, 2026
defb7f0
Oops, tab size 0 is just going to throw
davidwengier Mar 30, 2026
9f547b0
PR Feedback
davidwengier Mar 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 18 additions & 3 deletions src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/DocumentMapping/RazorEditService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ private static bool StrictlyContains(TextSpan outer, TextSpan inner)
// important, so we make sure that the text before the last line is all whitespace. If not, we'll let the
// skipped edit handling deal with this change. The C# formatter doesn't produce this sort of change due
// to the other processes the edits go through before they get to us.
if (lastNewLine != 0 && newText[..(lastNewLine - 1)].Any(static c => !char.IsWhiteSpace(c)))
if (lastNewLine != 0 && ContainsNonWhitespaceBefore(newText, lastNewLine))
{
return null;
}
Expand Down Expand Up @@ -374,8 +374,8 @@ private static bool StrictlyContains(TextSpan outer, TextSpan inner)

// Otherwise, add a newline and the real content, and remember where we added it
lastNewLineAddedToLine = startLine;
// Tab size is not used since we only want spaces, so passing 0 is fine.
var indent = FormattingUtilities.GetIndentationString(startChar, insertSpaces: true, tabSize: 0);
// Tab size is irrelevant when inserting spaces, but the helper requires a positive value.
var indent = FormattingUtilities.GetIndentationString(startChar, insertSpaces: true, tabSize: 1);
return new RazorTextChange()
{
Span = new RazorTextSpan
Expand All @@ -391,6 +391,21 @@ private static bool StrictlyContains(TextSpan outer, TextSpan inner)
return null;
}

private static bool ContainsNonWhitespaceBefore(string newText, int lastNewLine)
{
var span = newText.AsSpan();

for (var i = 0; i < lastNewLine; i++)
{
if (!char.IsWhiteSpace(span[i]))
{
return true;
}
}

return false;
}

/// <summary>
/// For all edits that are not mapped to using directives, map them directly to the Razor document.
/// Edits that don't map are skipped, and using directive changes are handled separately
Expand Down
74 changes: 32 additions & 42 deletions ...r/src/Microsoft.CodeAnalysis.Razor.Workspaces/DocumentMapping/RazorEditService_Methods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Text;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Components;
using Microsoft.AspNetCore.Razor.Language.Extensions;
Expand Down Expand Up @@ -44,11 +45,12 @@ private static void AddMethodChanges(ref PooledArrayBuilder<RazorTextChange> edi
var sourceText = source.Text;
var openBraceLine = openBrace.GetSourceLocation(source).LineIndex;
var closeBraceLocation = closeBrace.GetSourceLocation(source);
var closeBraceLine = closeBraceLocation.LineIndex;

var insertAbsoluteIndex = closeBraceLocation.AbsoluteIndex;
var insertLineIndex = closeBraceLocation.LineIndex;
var insertLineIndex = closeBraceLine;

if (openBraceLine != closeBraceLocation.LineIndex && closeBraceLocation.AbsoluteIndex > 0)
if (openBraceLine != closeBraceLine && closeBraceLocation.AbsoluteIndex > 0)
{
var previousLineAbsoluteIndex = closeBraceLocation.AbsoluteIndex - closeBraceLocation.CharacterIndex - 1;
var previousLinePosition = sourceText.GetLinePosition(previousLineAbsoluteIndex);
Expand All @@ -61,8 +63,8 @@ private static void AddMethodChanges(ref PooledArrayBuilder<RazorTextChange> edi
}
}

var methodsText = GetMethodsText(addedMethods, options);
var newText = FormatMethodsInExistingCodeBlock(sourceText, openBraceLine, closeBraceLocation.LineIndex, insertLineIndex, methodsText);
using var _ = StringBuilderPool.GetPooledObject(out var builder);
AddMethodsInExistingCodeBlock(builder, sourceText, addedMethods, options, openBraceLine, closeBraceLine, insertLineIndex);

edits.Add(new RazorTextChange()
{
Expand All @@ -71,7 +73,7 @@ private static void AddMethodChanges(ref PooledArrayBuilder<RazorTextChange> edi
Start = insertAbsoluteIndex,
Length = 0
},
NewText = newText
NewText = builder.ToString()
});
}

Expand Down Expand Up @@ -102,7 +104,8 @@ private static void AddMethodsInNewCodeBlock(ref PooledArrayBuilder<RazorTextCha

builder.Append('{');
builder.AppendLine();
builder.AppendLine(GetMethodsText(methods, options));
AppendMethodsText(builder, methods, options);
builder.AppendLine();
builder.Append('}');

edits.Add(new RazorTextChange()
Expand All @@ -116,59 +119,51 @@ private static void AddMethodsInNewCodeBlock(ref PooledArrayBuilder<RazorTextCha
});
}

private static string FormatMethodsInExistingCodeBlock(SourceText sourceText, int openBraceLineIndex, int closeBraceLineIndex, int insertLineIndex, string methodsText)
private static void AddMethodsInExistingCodeBlock(StringBuilder builder, SourceText sourceText, ImmutableArray<CSharpMethod> addedMethods, RazorFormattingOptions options, int openBraceLineIndex, int closeBraceLineIndex, int insertLineIndex)
{
if (openBraceLineIndex == closeBraceLineIndex)
{
return $"{Environment.NewLine}{methodsText}{Environment.NewLine}";
}

if (insertLineIndex == closeBraceLineIndex)
var lineAboveInsertionIsNotEmpty = insertLineIndex - 1 != openBraceLineIndex &&
!IsLineEmpty(sourceText.Lines[insertLineIndex - 1]);
if (openBraceLineIndex == closeBraceLineIndex || lineAboveInsertionIsNotEmpty)
{
methodsText += Environment.NewLine;
builder.AppendLine();
}

if (insertLineIndex - 1 == openBraceLineIndex)
{
return methodsText;
}
AppendMethodsText(builder, addedMethods, options);

var previousLine = sourceText.Lines[insertLineIndex - 1];
if (!IsLineEmpty(previousLine))
if (openBraceLineIndex == closeBraceLineIndex || insertLineIndex == closeBraceLineIndex)
{
methodsText = $"{Environment.NewLine}{methodsText}";
builder.AppendLine();
}

return methodsText;
}

private static string GetMethodsText(ImmutableArray<CSharpMethod> methods, RazorFormattingOptions options)
private static void AppendMethodsText(StringBuilder builder, ImmutableArray<CSharpMethod> methods, RazorFormattingOptions options)
{
using var _ = StringBuilderPool.GetPooledObject(out var builder);

var first = true;
foreach (var method in methods)
{
if (builder.Length > 0)
if (!first)
{
builder.AppendLine();
builder.AppendLine();
}

first = true;

AppendIndentedMethod(builder, method, options);
}

return builder.ToString();
}

private static void AppendIndentedMethod(System.Text.StringBuilder builder, CSharpMethod method, RazorFormattingOptions options)
private static void AppendIndentedMethod(StringBuilder builder, CSharpMethod method, RazorFormattingOptions options)
{
// Roslyn will have indented the method by an appropriate amount for the generated file, but we need it to be placed nicely in the Razor
// file, so we add each line of the method one at a time, adjusting the indentation as we go.
int? initialIndentation = null;
var sourceText = method.Text;

foreach (var line in method.EnumerateLines())
var endLine = method.GetEndLineNumber();
for (var i = method.GetStartLineNumber(); i <= endLine; i++)
{
var line = sourceText.Lines[i];
var currentIndentation = line.GetIndentationSize(options.TabSize);

if (initialIndentation is null)
Expand Down Expand Up @@ -247,17 +242,12 @@ public override int GetHashCode()
return 0;
}

public IEnumerable<TextLine> EnumerateLines()
{
// We don't want trivia, because it will include generated artifacts like #line directives, so using Span instead of FullSpan is deliberate

var firstLineNumber = Text.Lines.GetLineFromPosition(Method.SpanStart).LineNumber;
var lastLineNumber = Text.Lines.GetLineFromPosition(Math.Max(Method.SpanStart, Method.Span.End - 1)).LineNumber;
// We don't want trivia, because it will include generated artifacts like #line directives, so using Span instead of FullSpan in the two
// methods below is deliberate
public int GetStartLineNumber()
=> Text.Lines.GetLineFromPosition(Method.SpanStart).LineNumber;

for (var i = firstLineNumber; i <= lastLineNumber; i++)
{
yield return Text.Lines[i];
}
}
public int GetEndLineNumber()
=> Text.Lines.GetLineFromPosition(Math.Max(Method.SpanStart, Method.Span.End - 1)).LineNumber;
}
}