Make the extract-method command async (using the background work indicator).#61375
Conversation
| [ContentType(ContentTypeNames.VisualBasicContentType)] | ||
| [Name(PredefinedCommandHandlerNames.ExtractMethod)] | ||
| [Order(After = PredefinedCommandHandlerNames.DocumentationComments)] | ||
| internal sealed class ExtractMethodCommandHandler : ICommandHandler<ExtractMethodCommandArgs> |
There was a problem hiding this comment.
no need for abstract base class. There was no lang specific code in the subclasses. so just merged up into a single concrete command handler exported for C# and VB.
|
|
||
| var document = args.SubjectBuffer.CurrentSnapshot.GetOpenDocumentInCurrentContextWithChanges(); | ||
| if (document is null) | ||
| return false; |
There was a problem hiding this comment.
moved bail out logic up.
| var snapshotAfterFormatting = textBuffer.CurrentSnapshot; | ||
| var documentAfterFormatting = snapshotAfterFormatting.GetOpenDocumentInCurrentContextWithChanges(); | ||
| _renameService.StartInlineSession(documentAfterFormatting, methodNameAtInvocation.Span, cancellationToken); | ||
| await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); |
There was a problem hiding this comment.
it makes sense to me, but just wanted to confirm the intention that changes could be applied, then the indicator cancelled before the rename starts.
There was a problem hiding this comment.
actually... you raise a good point. we want the indicator dismissed once we start inline-rename. so i need to change this logic.
There was a problem hiding this comment.
oh no. this is correct. inline rename will start, and all this code will return and we'll dismiss the BWI.
There was a problem hiding this comment.
it makes sense to me, but just wanted to confirm the intention that changes could be applied, then the indicator cancelled before the rename starts.
technically it should not be possible due to there being no opportunity for the user to do anything that would cause the cancellation. but technically, yes. if somehow some path canceled that token (again, shouldn't be possible) then we'd apply teh extract method, but not start the inline rename session.
|
|
||
| // We're about to make an edit ourselves. so disable the cancellation that happens on editing. | ||
| waitContext.CancelOnEdit = false; | ||
| formattedDocument.Project.Solution.Workspace.ApplyDocumentChanges(formattedDocument, cancellationToken); |
There was a problem hiding this comment.
- what happens if we cancel in the middle of the undo transaction? Do we need to explicitly complete it / throw it away if we cancel?
- Is it possible to partially apply changes in ApplyDocumentChanges? Or if it cancels does it do all or nothing
There was a problem hiding this comment.
so we're on the UI thread now. so it shouldn't be possible to have a cancel happen at this point since only UI affinitized ops can cause cancellation. it's a bit of an odd duck here i admit as we could also effectively have the same effect by passing CancellationToken.None here :-/
There was a problem hiding this comment.
interesting - but we still need the cancelOnEdit = false so that it doesn't immediately cancel after we finish applying changes, switch off the UI thread and cancel?
There was a problem hiding this comment.
yes, we still need the cancelOnEdit false. but not because of any switching. because if we don't, we'll make the edit, and the cancellation token will get signaled. ANd then any further work we do (on any thread) that checks the token will think we are cancled and will abort.
No description provided.