Skip to content
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.

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

Don't pass RazorCodeDocument and SourceText around together #10719

Merged
merged 3 commits into from
Aug 8, 2024

Conversation

davidwengier
Copy link
Contributor

@davidwengier davidwengier commented Aug 8, 2024

because it's really easy to get one from the other.

While working on rename in cohosting, I noticed this unnecessary extra bit of work while calling GetPositionInfo, so I fixed it. Then I thought I'd see where else GetSourceTextAsync was used in an non-ideal way, and that was clearly going to be too big for the rename PR so I separated it out into it's own PR. Then I heard Dustin say he was changing DocumentSnapshot so the unnecessary work will end up being much less wasteful anyway, plus we'd probably just have conflicts, so I pared the whole thing right back to just this minor removal of a couple of unnecessary parameters. Enjoy.

@davidwengier davidwengier requested a review from a team as a code owner August 8, 2024 11:18
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.

Nice!

…ommit message, and I didn't like any of them.
@davidwengier davidwengier merged commit d5cfe11 into dotnet:main Aug 8, 2024
12 checks passed
@davidwengier davidwengier deleted the SimplifyGettingSourceText branch August 8, 2024 23:15
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Aug 8, 2024
@dibarbet dibarbet modified the milestones: Next, 17.12 P2 Aug 26, 2024
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