-
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
WIP: Initial work to support bulk renaming of files to match types contained within. #77239
base: main
Are you sure you want to change the base?
Conversation
...able/CodeRefactorings/EnableNullable/EnableNullableCodeRefactoringProvider.FixAllProvider.cs
Outdated
Show resolved
Hide resolved
…nableNullableCodeRefactoringProvider.FixAllProvider.cs
@@ -1263,12 +1262,29 @@ protected override void ApplyDocumentInfoChanged(DocumentId documentId, Document | |||
// By setting this property, Visual Studio will perform the file rename, which | |||
// will cause the workspace's current solution to update and will fire the | |||
// necessary workspace changed events. | |||
projectItemForDocument.Name = uniqueName; | |||
try |
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.
@jasonmalinowski i'm not sure what to do about these. trying to run this on Roslyn and i def ran into cases where tehse threw. i think it's all the linked files and shared-code-projects.
Unfortunately, on the feature-side, i'm not sure if there's any way to detect these cases up front and figure out how to avoid them. Any thoughts from you?
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.
@CyrusNajmabadi Do you remember more what the failure was? I could imagine we may have an issue if your refactoring is trying to rename the same file more than once (but it's in different projects), and maybe that's what could be happening. But I have a vague memory of @ryzngard fixing some things there. But that's making a very random guess.
I don't even know if a feature could do filtering for other reasons like shared code projects, since we don't expose that in any way in the workspace layer. And frankly, we'd have to go figure out how to even know those ourselves since that's abstracted away from us too without calling a bunch of legacy VS APIs.
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.
tl;dr: let's figure out why it failed, since maybe that's something this layer can deal with. If it can't, then I don't imagine we can push it elsewhere, so maybe just catching the exception would be the right thing to do. It's probably better to apply most of the renames instead of crashing.
} | ||
catch (KeyNotFoundException ex) |
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 wasn't sure if i should move this into TryGetFullPatah (as that's an extension method we own). this feels like something messed up in the project system that we need this.
Note: i can provide you with file paths this happens for if that would be helpful.
No description provided.