Enhance unused reference feature to support undo/redo

[JoeRobich]

We would like the API implemented in #6692 enhanced with the ability to re-add a reference that has been removed.

One possibility is a new UpdateAction.Add
In particular the call to IProjectSystemReferenceCleanupService.GetProjectReferenceAsync would need to be enhanced so that we capture enough information about each reference that it could be recreated from the ProjectSystemReferenceInfo model. This information may need to include the position in the project file where the reference would be inserted. Next the IProjectSystemReferenceCleanupService.TryUpdateReferenceAsync would need to be updated to support an ProjectSystemUpdateAction.Add scenario where, given this enhacned ReferenceInfo model, a reference is added to the project file.

Another possibility is that we share the same undo context
The IProjectSystemReferenceCleanupService.TryUpdateReferenceAsync could return a OleUndoUnit or we could pass in a UndoTransaction of sorts.

Tracked in DevOps 1348735

[ocallesp]

@JoeRobich
In this scenario, the Undo/Redo action will come from Roslyn ?

• If a user does ctrl+z, Roslyn will detect it and then Roslyn will call ProjectSystem to execute the undo/redo in the IProjectSystemReferenceCleanupService ?

The undo/redo should keep the history of all actions or only the last action ?

[JoeRobich]

Roslyn would be responsible for adding the undo/redo action. We need to work out the best way to support undo/redo.

• Should Roslyn capture the project file text prior to updating references so that it could be restored in an undo operation?
• Should the project system return child undo operations for each call to UpdateRefrences which Roslyn could batch together into a transaction?
• or some other method of supporting undo/redo in this scenario?

[ocallesp]

I think the interface should add undo/redo methods and call dotnet ProjectSystem to do IProjectSystemReferenceCleanupService.Undo() or IProjectSystemReferenceCleanupService.Redo()

[JoeRobich]

@ocallesp Can you describe how this would work end-to-end? Would you be storing state in the CleanupService or would we be passing some state when calling Undo/Redo?

[ocallesp]

ProjectSystem can store the state in the CleanupService

[mikadumont]

UX Proposal:

Typing Ctrl+Z/Ctrl+Y and selecting undo/redo:

Will either undo/redo remove unused references. And the Remove Unused References commit should appear in the undo/redo history stack:

[vatsalyaagrawal]

@jjmew, @ocallesp please see the UX proposal above.

[jjmew]

@vatsalyaagrawal Thanks for sharing. I assume your team is hooking up to the undo/redo stack and you will call us when needed. Is that correct? cc: @ocallesp

[mikadumont]

UX update: I met with UX and they mentioned that we dont need a dialog - just having Remove Unused References show up in the undo/redo history stack is good enough.

[JoeRobich]

I assume your team is hooking up to the undo/redo stack and you will call us when needed. Is that correct?

@jjmew Yes, Roslyn will manage the undo/redo stack and we can call into the cleanup service as @ocallesp proposes.

[jjmew]

@mikadumont what is the expectation on this scenario?

1. User removes un-used references A,B,C
2. They modify a file
3. They copy some text into a file
4. User removes un-used references X,Y,Z
5. User does "Ctrl Z"

What is the result?
What happens after another "Ctrl Z"

[JoeRobich]

@jjmew Ctrl-Z should only undo the last operation. In this case it should restore the X, Y, Z references.

[jjmew]

@jodavis what about the 4th "Ctrl Z" . Should it re-add references A,B,C?

[JoeRobich]

@jjmew That is correct. I feel like we will need to keep some state information with each undo action that is pushed on the stack, so that we can properly track which changes need to be undone.

[jjmew]

@jodavis the expectation then is that the productivity team will request the project system to undo the last action on the "Remove Unused References" stack. We will add the references that we previously removed. Is there any error handling that you expect? for example if we were not able to re-add the references.

[JoeRobich]

@jjmew It would be coarse-grained but we could have the proposed Undo() and Redo() methods return bool indicating success. We would then inform the user on our end.

[ocallesp]

The itemSpecification returned by GetProjectReferencesAsync() are different from the TryUpdateReferenceAsync()
i.e "..\ClassLibrary1\ClassLibrary1.csproj" and "ClassLibrary1"

Roslyn needs to fix the itemSpecification to use the one from GetProjectReferencesAsync()
and add the missing methods Undo(), and Redo() to IProjectSystemReferenceCleanupService

This PR already implements the undo/redo #7065

[JoeRobich]

I met with Osvaldo and to discuss the implementation and we have agreed on adding the following interface changes.

interface IProjectSystemUpdateReferenceOperation
{
    Task<bool> ApplyAsync();
    Task<bool> RevertAsync();
}
 
interface IProjectSystemReferenceCleanupService2
{
    Task<IProjectSystemUpdateReferenceOperation> GetUpdateReferenceOperationAsync(
        string projectPath,
        ProjectSystemReferenceUpdate referenceUpdate,
        CancellationToken canellationToken);
}

Roslyn will request update actions for each of the changes from the Project System. We will then apply the actions and push an Undo action onto the UndoManager which will contain all the actions for the Remove Reference invocation.

[ocallesp]

Implementations of IProjectSystemUpdateReferenceOperation

• RemoveReferenceCommand
• SetTreatAsUsedAttributeCommand
• SetAttributeCommand
• NullCommand
• UnsetAttributeCommand

[drewnoakes]

@ocallesp do you recall which version this was fixed in?