Skip to content

Commit

Permalink
Merge pull request #75562 from Cosifne/dev/shech/RenameCommandHandler…
Browse files Browse the repository at this point in the history
…Cancel

Prefer Cancel() rather than Commit() when rename is async.
  • Loading branch information
Cosifne authored Oct 22, 2024
2 parents 0c91541 + 72c29b2 commit 42c572c
Show file tree
Hide file tree
Showing 10 changed files with 72 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Telemetry;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Options;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename
{
Expand All @@ -33,8 +34,9 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename
internal partial class RenameCommandHandler(
IThreadingContext threadingContext,
InlineRenameService renameService,
IGlobalOptionService globalOptionService,
IAsynchronousOperationListenerProvider asynchronousOperationListenerProvider)
: AbstractRenameCommandHandler(threadingContext, renameService, asynchronousOperationListenerProvider.GetListener(FeatureAttribute.Rename))
: AbstractRenameCommandHandler(threadingContext, renameService, globalOptionService, asynchronousOperationListenerProvider.GetListener(FeatureAttribute.Rename))
{
protected override bool AdornmentShouldReceiveKeyboardNavigation(ITextView textView)
=> GetAdornment(textView) switch
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
using System.Linq;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.InlineRename;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Text;
Expand All @@ -19,6 +21,7 @@ namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename;
internal abstract partial class AbstractRenameCommandHandler(
IThreadingContext threadingContext,
InlineRenameService renameService,
IGlobalOptionService globalOptionService,
IAsynchronousOperationListener listener)
{
public string DisplayName => EditorFeaturesResources.Rename;
Expand Down Expand Up @@ -57,6 +60,7 @@ private void HandlePossibleTypingCommand<TArgs>(TArgs args, Action nextHandler,

if (renameService.ActiveSession.IsCommitInProgress)
{
// When rename commit is in progress, swallow the command so it won't change the workspace
return;
}

Expand All @@ -78,9 +82,7 @@ private void HandlePossibleTypingCommand<TArgs>(TArgs args, Action nextHandler,
}
else if (renameService.ActiveSession.IsInOpenTextBuffer(singleSpan.Start))
{
// It's in a read-only area that is open, so let's commit the rename
// and then let the character go through
CommitIfActive(args, operationContext);
HandleTypingOutsideEditableSpan(args, operationContext);
nextHandler();
}
else
Expand All @@ -90,20 +92,36 @@ private void HandlePossibleTypingCommand<TArgs>(TArgs args, Action nextHandler,
}
}

private void CommitIfActive(EditorCommandArgs args, IUIThreadOperationContext operationContext)
private void HandleTypingOutsideEditableSpan(EditorCommandArgs args, IUIThreadOperationContext operationContext)
=> CommitIfSynchronousOrCancelIfAsynchronous(args, operationContext, placeCaretAtTheEndOfIdentifier: true);

private void CommitIfActive(EditorCommandArgs args, IUIThreadOperationContext operationContext, bool placeCaretAtTheEndOfIdentifier = true)
{
if (renameService.ActiveSession != null)
{
var selection = args.TextView.Selection.VirtualSelectedSpans.First();

Commit(operationContext);

var translatedSelection = selection.TranslateTo(args.TextView.TextBuffer.CurrentSnapshot);
args.TextView.Selection.Select(translatedSelection.Start, translatedSelection.End);
args.TextView.Caret.MoveTo(translatedSelection.End);
if (placeCaretAtTheEndOfIdentifier)
{
var translatedSelection = selection.TranslateTo(args.TextView.TextBuffer.CurrentSnapshot);
args.TextView.Selection.Select(translatedSelection.Start, translatedSelection.End);
args.TextView.Caret.MoveTo(translatedSelection.End);
}
}
}

/// <summary>
/// Commit() will be called if rename commit is sync. Editor command would be executed after the rename operation complete and it is our legacy behavior.
/// Cancel() will be called if rename is async. It also matches the other editor's behavior (like VS LSP and VSCode).
/// </summary>
private void CommitIfSynchronousOrCancelIfAsynchronous(EditorCommandArgs args, IUIThreadOperationContext operationContext, bool placeCaretAtTheEndOfIdentifier)
{
if (globalOptionService.ShouldCommitAsynchronously())
renameService.ActiveSession?.Cancel();
else
CommitIfActive(args, operationContext, placeCaretAtTheEndOfIdentifier);
}

private void Commit(IUIThreadOperationContext operationContext)
{
RoslynDebug.AssertNotNull(renameService.ActiveSession);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Text.Editor.Commanding;
using Microsoft.VisualStudio.Text.Editor.Commanding.Commands;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename;
Expand All @@ -14,23 +15,21 @@ public CommandState GetCommandState(MoveSelectedLinesUpCommandArgs args)
=> CommandState.Unspecified;

public bool ExecuteCommand(MoveSelectedLinesUpCommandArgs args, CommandExecutionContext context)
{
if (IsRenameCommitInProgress())
return true;

CommitIfActive(args, context.OperationContext);
return false;
}
=> HandleMoveSelectLinesUpOrDownCommand(args, context);

public CommandState GetCommandState(MoveSelectedLinesDownCommandArgs args)
=> CommandState.Unspecified;

public bool ExecuteCommand(MoveSelectedLinesDownCommandArgs args, CommandExecutionContext context)
=> HandleMoveSelectLinesUpOrDownCommand(args, context);

private bool HandleMoveSelectLinesUpOrDownCommand(EditorCommandArgs args, CommandExecutionContext context)
{
// When rename commit is in progress, swallow the command so it won't change the workspace
if (IsRenameCommitInProgress())
return true;

CommitIfActive(args, context.OperationContext);
CommitIfSynchronousOrCancelIfAsynchronous(args, context.OperationContext, placeCaretAtTheEndOfIdentifier: true);
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ public void ExecuteCommand(OpenLineAboveCommandArgs args, Action nextHandler, Co
{
HandlePossibleTypingCommand(args, nextHandler, context.OperationContext, (activeSession, operationContext, span) =>
{
Commit(operationContext);
// Caret would be moved to the new line when editor command is handled, so we don't need to move it.
CommitIfSynchronousOrCancelIfAsynchronous(args, operationContext, placeCaretAtTheEndOfIdentifier: false);
nextHandler();
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ public void ExecuteCommand(OpenLineBelowCommandArgs args, Action nextHandler, Co
{
HandlePossibleTypingCommand(args, nextHandler, context.OperationContext, (activeSession, operationContext, span) =>
{
Commit(operationContext);
// Caret would be moved to the new line when editor command is handled, so we don't need to move it.
CommitIfSynchronousOrCancelIfAsynchronous(args, operationContext, placeCaretAtTheEndOfIdentifier: false);
nextHandler();
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Microsoft.CodeAnalysis.InlineRename;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Text.Editor.Commanding;
using Microsoft.VisualStudio.Text.Editor.Commanding.Commands;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename;
Expand All @@ -17,47 +19,32 @@ public CommandState GetCommandState(ReorderParametersCommandArgs args)
=> CommandState.Unspecified;

public bool ExecuteCommand(ReorderParametersCommandArgs args, CommandExecutionContext context)
{
if (IsRenameCommitInProgress())
return true;

CommitIfActive(args, context.OperationContext);
return false;
}
=> HandleRefactoringCommands(args, context);

public CommandState GetCommandState(RemoveParametersCommandArgs args)
=> CommandState.Unspecified;

public bool ExecuteCommand(RemoveParametersCommandArgs args, CommandExecutionContext context)
{
if (IsRenameCommitInProgress())
return true;

CommitIfActive(args, context.OperationContext);
return false;
}
=> HandleRefactoringCommands(args, context);

public CommandState GetCommandState(ExtractInterfaceCommandArgs args)
=> CommandState.Unspecified;

public bool ExecuteCommand(ExtractInterfaceCommandArgs args, CommandExecutionContext context)
{
if (IsRenameCommitInProgress())
return true;

CommitIfActive(args, context.OperationContext);
return false;
}
=> HandleRefactoringCommands(args, context);

public CommandState GetCommandState(EncapsulateFieldCommandArgs args)
=> CommandState.Unspecified;

public bool ExecuteCommand(EncapsulateFieldCommandArgs args, CommandExecutionContext context)
=> HandleRefactoringCommands(args, context);

private bool HandleRefactoringCommands(EditorCommandArgs args, CommandExecutionContext context)
{
if (IsRenameCommitInProgress())
return true;

CommitIfActive(args, context.OperationContext);
CommitIfSynchronousOrCancelIfAsynchronous(args, context.OperationContext, placeCaretAtTheEndOfIdentifier: true);
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Editor.BackgroundWorkIndicator;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.InlineRename;
using Microsoft.CodeAnalysis.Notification;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.TestHooks;
Expand Down Expand Up @@ -81,8 +82,9 @@ private async Task ExecuteCommandAsync(RenameCommandArgs args, IUIThreadOperatio
}
else
{
// Otherwise, commit the existing session and start a new one.
Commit(editorOperationContext);
// Otherwise, commit or cancel the existing session and start a new one.
// Set placeCaretAtTheEndOfIdentifier to false because a new rename session will be created based on caret's location.
CommitIfSynchronousOrCancelIfAsynchronous(args, editorOperationContext, placeCaretAtTheEndOfIdentifier: false);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Microsoft.CodeAnalysis.InlineRename;
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Text.Editor.Commanding.Commands;

Expand All @@ -14,7 +15,9 @@ public CommandState GetCommandState(SaveCommandArgs args)

public bool ExecuteCommand(SaveCommandArgs args, CommandExecutionContext context)
{
if (renameService.ActiveSession != null)
// If commit is async, just let editor save the document.
// If call async commit here, it could finish after the save command so the workspace would still be dirty.
if (renameService.ActiveSession != null && !globalOptionService.ShouldCommitAsynchronously())
{
Commit(context.OperationContext);
SetFocusToTextView(args.TextView);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,13 @@ private bool ExecuteSelectAll(ITextBuffer subjectBuffer, ITextView view)
}

var caretPoint = view.GetCaretPoint(subjectBuffer);
if (caretPoint.HasValue)
if (caretPoint.HasValue && renameService.ActiveSession.TryGetContainingEditableSpan(caretPoint.Value, out var span))
{
if (renameService.ActiveSession.TryGetContainingEditableSpan(caretPoint.Value, out var span))
if (view.Selection.Start.Position != span.Start.Position ||
view.Selection.End.Position != span.End.Position)
{
if (view.Selection.Start.Position != span.Start.Position ||
view.Selection.End.Position != span.End.Position)
{
view.SetSelection(span);
return true;
}
view.SetSelection(span);
return true;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.VisualStudio.Commanding;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding;
using Microsoft.VisualStudio.Text.Editor.Commanding.Commands;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename;
Expand All @@ -22,13 +23,14 @@ public CommandState GetCommandState(WordDeleteToEndCommandArgs args)
=> GetCommandState();

public bool ExecuteCommand(WordDeleteToStartCommandArgs args, CommandExecutionContext context)
=> HandleWordDeleteCommand(args.SubjectBuffer, args.TextView, deleteToStart: true);
=> HandleWordDeleteCommand(args, context, args.TextView, deleteToStart: true);

public bool ExecuteCommand(WordDeleteToEndCommandArgs args, CommandExecutionContext context)
=> HandleWordDeleteCommand(args.SubjectBuffer, args.TextView, deleteToStart: false);
=> HandleWordDeleteCommand(args, context, args.TextView, deleteToStart: false);

private bool HandleWordDeleteCommand(ITextBuffer subjectBuffer, ITextView view, bool deleteToStart)
private bool HandleWordDeleteCommand(EditorCommandArgs args, CommandExecutionContext context, ITextView view, bool deleteToStart)
{
var subjectBuffer = args.SubjectBuffer;
if (renameService.ActiveSession == null)
{
return false;
Expand Down Expand Up @@ -69,6 +71,10 @@ private bool HandleWordDeleteCommand(ITextBuffer subjectBuffer, ITextView view,

return true;
}
else
{
CommitIfSynchronousOrCancelIfAsynchronous(args, context.OperationContext, placeCaretAtTheEndOfIdentifier: true);
}
}

return false;
Expand Down

0 comments on commit 42c572c

Please sign in to comment.