Simplifications to help moving rename entirely OOP.#43573
Simplifications to help moving rename entirely OOP.#43573CyrusNajmabadi merged 15 commits intodotnet:masterfrom
Conversation
f7c5240 to
524dc0a
Compare
524dc0a to
e6b317c
Compare
ad2734e to
8eed02c
Compare
...rFeatures/Core.Wpf/InlineRename/AbstractEditorInlineRenameService.InlineRenameLocationSet.cs
Show resolved
Hide resolved
|
I did a high-level review but I don't have much insight into how rename works. So someone who has should provide a detailed review. |
| hashCode = hashCode * -1521134295 + IsReference.GetHashCode(); | ||
| hashCode = hashCode * -1521134295 + EqualityComparer<DocumentId>.Default.GetHashCode(DocumentId); | ||
| hashCode = hashCode * -1521134295 + ComplexifiedTargetSpan.GetHashCode(); | ||
| return hashCode; |
There was a problem hiding this comment.
i can't. not vailable on netstandard.
There was a problem hiding this comment.
There was a problem hiding this comment.
sure. i can do that in a followup.
Note: the benefit of the above approach is that the IDE detects it and will offer to upgrade it once you do have access to System.HashCode.
src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolution.cs
Show resolved
Hide resolved
|
|
||
| // List of All the Locations that were renamed and conflict-complexified | ||
| private readonly List<RelatedLocation> _relatedLocations; | ||
| public readonly List<RelatedLocation> RelatedLocations; |
There was a problem hiding this comment.
should this be exposed in an immutable way? Even if they can't set the list instance, they can modify the contents.
There was a problem hiding this comment.
no. this is actually mutate by callers. MutableConflictResolution is intended to be mutable. it changes as we're doing conflict resolution. At hte end though we snap the final immutbale state and pass that back out.
src/Workspaces/Core/Portable/Rename/ConflictEngine/ConflictResolution.cs
Outdated
Show resolved
Hide resolved
|
Not worth blocking, but I find the naming a little confusing. I'd rather see |
dpoeschl
left a comment
There was a problem hiding this comment.
LGTM, but the various follow-up tasks do seem like good ideas. :)
|
Thanks! |
Overall plan it to make ConflictResolution a mutable internal type, but then expose an immutable snapshot of the final form with the data that clients actually need.