Skip to content

Add missing paren to SyntaxSerializer#11968

Merged
jjonescz merged 1 commit intomainfrom
jjonescz-patch-1
Jun 24, 2025
Merged

Add missing paren to SyntaxSerializer#11968
jjonescz merged 1 commit intomainfrom
jjonescz-patch-1

Conversation

@jjonescz
Copy link
Member

The paren seems to be unintentionally dropped in #11853 cc @DustinCampbell

@jjonescz jjonescz requested a review from a team as a code owner June 22, 2025 16:04
protected virtual void WriteSpan(TextSpan span)
{
WriteValue($"[{span.Start}..{span.End}{Separator}Width: {span.End - span.Start}");
WriteValue($"[{span.Start}..{span.End}){Separator}Width: {span.End - span.Start}");
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean this?

Suggested change
WriteValue($"[{span.Start}..{span.End}){Separator}Width: {span.End - span.Start}");
WriteValue($"[{span.Start}..{span.End}]{Separator}Width: {span.End - span.Start}");

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the end is exclusive. I think we use [x..y) to demonstrate that in other places as well. Definitely we did that here previously:

Copy link
Member

Choose a reason for hiding this comment

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

Oh right, thanks. Looked weird out of context

Copy link
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

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

Thanks! I'm glad this has no product impact, since it's only used for debugging. Thankfully, I got it right in TestSyntaxSerializer. Sadly, this is one of the places where the serialization differs between debugging and test output.

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.

6 participants