-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Update filenames in file banners when moving a type to a new file. #76270
Conversation
@@ -125,38 +119,6 @@ private static ImmutableArray<SyntaxTrivia> GetBannerFromEquivalenceKey(string e | |||
return bannerService.GetFileBanner(token); | |||
} | |||
|
|||
private Task<Document> AddBannerAsync( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved to a common location so it can be used in MoveType.
ImmutableArray<SyntaxTrivia> banner, | ||
Func<SyntaxTrivia, string, SyntaxTrivia> createTrivia) | ||
{ | ||
var sourceName = IOUtilities.PerformIO(() => Path.GetFileName(sourceDocument.FilePath)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll do a pass later to address these
@@ -146,7 +145,10 @@ private async Task<Document> AddNewDocumentWithSingleTypeDeclarationAsync(Docume | |||
// get the updated document, give it the minimal set of imports that the type | |||
// inside it needs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this comment confuses me, it doesn't look like imports are handled in this method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. Seems incorrect. Was already like that, so I'll move to another pr in this space to cleanup.
|
||
/// <summary> | ||
/// Looks at <paramref name="banner"/> to see if it contains the name of <paramref name="sourceDocument"/> | ||
/// in it. If so, those names will be replaced with <paramref name="destinationFilePath"/>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes #74703