Skip to content

Conversation

@davidwengier
Copy link
Member

@davidwengier davidwengier commented Dec 29, 2025

Fixes #12622 plus a few others from #12554
Fixes #12631

We already did most of this, just for explicit expressions (ie @( ... )) so this brings it to implicit expressions too (ie @ ...) when they're multiline. We also used to emit a /* */ at the end of the first line, in case there was some non-C# content on the line, and now we check for it so that we can handle Roslyn moving an open brace, or anything else really.

@davidwengier davidwengier requested a review from a team as a code owner December 29, 2025 00:41
@davidwengier davidwengier mentioned this pull request Jan 5, 2026
}
</div>
""",
expected: """
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expected

Seems odd to me that the input/expected strings are the same.

Maybe this is just the way the formatting tests work as this was the case in new tests added in another PR I reviewed today.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a lot of cases, formatting is not just about putting things in the right place, but also not moving things if they're already in the right spot. In this case, for object initializers, Roslyn doesn't touch them at all, and so the tests verify that we correctly honour that. The old formatting engine had a bad habit of constantly indenting these sorts of things, every time the formatter ran.

if (offsetFromEnd == 0)
{
// If we're not doing any extra emitting of our own, then we can safely check for newlines
return CreateLineInfo(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return CreateLineInfo(

This doesn't need to specify formattedLength similar to the other call?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Importantly, it can't specify a formattedLength, or the check for new lines wouldn't work.

Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@davidwengier davidwengier merged commit b326574 into dev/dawengie/LetsDoHtmlFormatting Jan 7, 2026
1 of 10 checks passed
@davidwengier davidwengier deleted the dev/dawengie/FixImplicitExpressionFormatting branch January 7, 2026 06:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants