Simplify ProjectSnapshotManager #11291
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add factory methods for different ProjectChangeKind values that take the expected inputs. - Rename SolutionIsClosing to IsSolutionClosing. - Update ProjectSnapshotManager to use factory methods to create args. - Update calling code.
While there were excellent comments to describe the behavior of the notification queue, it was still a bit subtle to first peek the notification and only dequeue it when the listeners had been notified. This change introduces a bool field to make it clearer when notifications can be processed. In addition, this change introduces a catch around notification processing and logs any exceptions.
- Introduce TryAddProject method. - Rename Updater.ProjectAdded to Updater.AddProject
- Introduce TryAddRemove method. - Rename Updater.ProjectRemoved to Updater.RemoveProject
- Introduce TryUpdateProject methods. - Push SourceText version logic into ProjectState methods - Rename ProjectState.WithChangedDocumentText to WithDocumentText - Rename Updater.DocumentChanged to Updater.UpdateDocumentText - Rename Updater.DocumentOpened to Updater.OpenDocument - Rename Updater.DocumentClosed to Updater.CloseDocument
- Use TryUpdateProject - Rename ProjectState.WithAddedDocuemnt to AddDocument - Rename Updater.DocumentAdded to Updater.AddDocument
- Use TryUpdateProject - Rename ProjectState.WithRemovedHostDocument to RemoveDocument - Rename Updater.DocumentRemoved to Updater.RemoveDcument - Change RemoveDocument to take file path rather than HostDocument
- Use TryUpdateProject - Rename Updater.ProjectWorkspaceStateChanged to Updater.UpdateProjectWorkspaceState
- Use TryUpdateProject - Rename Updater.ProjectConfigurationChanged to Updater.UpdateProjectConfiguration
davidwengier
approved these changes
Dec 11, 2024
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectSnapshotManager.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectState.cs
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectSnapshotManager.cs
Show resolved
Hide resolved
- Quick audit of test names after renaming ProjectState and ProjectSnapshotManager APIs - Some small clean up and extra overloads to TestMocks.CreateTextLoader.
I performed a quick audit of the affected tests and updated any misnaming that I spotted. |
davidwengier
approved these changes
Dec 11, 2024
ryzngard
approved these changes
Dec 11, 2024
This was referenced Dec 14, 2024
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first of several pull requests that will be coming in over the next week. This change refactors the
ProjectSnapshotManagerto clarify and simplify the logic to prepare solution snapshots in the near future.It will likely be easier to review commit-by-commit.
Insertion PR for validation: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/597922