Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change commit to async when enter is pressed #75335

Merged
merged 28 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
9f49243
Add options
Cosifne Oct 1, 2024
61ca735
Use the featureFlag in rename session
Cosifne Oct 1, 2024
2c8a742
Async commit in view model
Cosifne Oct 1, 2024
9aeb065
Add comment
Cosifne Oct 1, 2024
89cf25a
Use primary constructor
Cosifne Oct 1, 2024
0f0029e
Prevent change when rename is in-progress
Cosifne Oct 1, 2024
0478d09
Move return key to async
Cosifne Oct 1, 2024
76c722d
Reset one change
Cosifne Oct 2, 2024
bc379f8
guard more command handlers
Cosifne Oct 2, 2024
4c4ec72
Revert "Use primary constructor"
Cosifne Oct 2, 2024
b26ac48
Merge remote-tracking branch 'upstream/main' into dev/shech/MoveEnter…
Cosifne Oct 3, 2024
2fb97f4
Remove not used options
Cosifne Oct 3, 2024
b764740
Add listener provider
Cosifne Oct 3, 2024
c731d9c
Add async rename tests
Cosifne Oct 3, 2024
7b30ea8
More async token
Cosifne Oct 3, 2024
b12a1e3
Reset a not needed change
Cosifne Oct 3, 2024
67e1a6d
Move the token to caller
Cosifne Oct 3, 2024
362f502
Modify comment
Cosifne Oct 3, 2024
f37bc82
Delete the option entry in VS storage
Cosifne Oct 4, 2024
f21f843
Comment the storage
Cosifne Oct 4, 2024
c716abd
Modify the comment
Cosifne Oct 4, 2024
f0a2321
Remove one else clause and change comment
Cosifne Oct 15, 2024
05887bd
Aggregate exception and async token handling
Cosifne Oct 16, 2024
2eeead6
Clean up unused listners
Cosifne Oct 16, 2024
cb101d6
Add comments
Cosifne Oct 16, 2024
6a6894f
Clean usings
Cosifne Oct 16, 2024
7aa8db2
Reset to default value in integration test
Cosifne Oct 17, 2024
5a8ed41
Pass the missing context
Cosifne Oct 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
using Microsoft.VisualStudio.Imaging.Interop;
using Microsoft.VisualStudio.PlatformUI.OleComponentSupport;
using Microsoft.VisualStudio.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename
{
Expand Down Expand Up @@ -228,7 +229,7 @@ public bool Submit()
}

SmartRenameViewModel?.Commit(IdentifierText);
Session.Commit();
Session.InitiateCommit();
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ private void Commit()
{
try
{
_model.Session.Commit();
_model.Session.InitiateCommit();
_textView.VisualElement.Focus();
}
catch (NotSupportedException ex)
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
3 changes: 3 additions & 0 deletions src/EditorFeatures/Core/IInlineRenameSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,8 @@ internal interface IInlineRenameSession
/// <summary>
/// Dismisses the rename session, completing the rename operation across all files.
/// </summary>
/// <remarks>
/// It will only be async when InlineRenameUIOptionsStorage.CommitRenameAsynchronously is set to true.
/// </remarks>
Task CommitAsync(bool previewChanges, IUIThreadOperationContext editorOperationContext = null);
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ private void HandlePossibleTypingCommand<TArgs>(TArgs args, Action nextHandler,
return;
}

if (renameService.ActiveSession.IsCommitInProgress)
{
return;
}

var selectedSpans = args.TextView.Selection.GetSnapshotSpansOnBuffer(args.SubjectBuffer);

if (selectedSpans.Count > 1)
Expand Down Expand Up @@ -104,4 +109,7 @@ private void Commit(IUIThreadOperationContext operationContext)
RoslynDebug.AssertNotNull(renameService.ActiveSession);
renameService.ActiveSession.Commit(previewChanges: false, operationContext);
}

private bool IsRenameCommitInProgress()
=> renameService.ActiveSession?.IsCommitInProgress is true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ public CommandState GetCommandState(MoveSelectedLinesUpCommandArgs args)

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

CommitIfActive(args, context.OperationContext);
return false;
}
Expand All @@ -24,6 +27,9 @@ public CommandState GetCommandState(MoveSelectedLinesDownCommandArgs args)

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

CommitIfActive(args, context.OperationContext);
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ public CommandState GetCommandState(ReorderParametersCommandArgs args)

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

CommitIfActive(args, context.OperationContext);
return false;
}
Expand All @@ -27,6 +30,9 @@ public CommandState GetCommandState(RemoveParametersCommandArgs args)

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

CommitIfActive(args, context.OperationContext);
return false;
}
Expand All @@ -36,6 +42,9 @@ public CommandState GetCommandState(ExtractInterfaceCommandArgs args)

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

CommitIfActive(args, context.OperationContext);
return false;
}
Expand All @@ -45,6 +54,9 @@ public CommandState GetCommandState(EncapsulateFieldCommandArgs args)

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

CommitIfActive(args, context.OperationContext);
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,15 @@ private async Task ExecuteCommandAsync(RenameCommandArgs args, IUIThreadOperatio
// If there is already an active session, commit it first
if (renameService.ActiveSession != null)
{
// Is the caret within any of the rename fields in this buffer?
// If so, focus the dashboard
if (renameService.ActiveSession.IsCommitInProgress)
{
return;
}

if (renameService.ActiveSession.TryGetContainingEditableSpan(caretPoint.Value, out _))
{
// Is the caret within any of the rename fields in this buffer?
// If so, focus the dashboard
SetFocusToAdornment(args.TextView);
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public bool ExecuteCommand(ReturnKeyCommandArgs args, CommandExecutionContext co

protected virtual void CommitAndSetFocus(InlineRenameSession activeSession, ITextView textView, IUIThreadOperationContext operationContext)
{
Commit(operationContext);
activeSession.InitiateCommit(operationContext);
SetFocusToTextView(textView);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,31 +18,43 @@ public CommandState GetCommandState(RedoCommandArgs args)

public bool ExecuteCommand(UndoCommandArgs args, CommandExecutionContext context)
{
if (renameService.ActiveSession != null)
if (renameService.ActiveSession == null)
{
for (var i = 0; i < args.Count && renameService.ActiveSession != null; i++)
{
renameService.ActiveSession.UndoManager.Undo(args.SubjectBuffer);
}
return false;
}

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

return false;
for (var i = 0; i < args.Count && renameService.ActiveSession != null; i++)
{
renameService.ActiveSession.UndoManager.Undo(args.SubjectBuffer);
}

return true;
}

public bool ExecuteCommand(RedoCommandArgs args, CommandExecutionContext context)
{
if (renameService.ActiveSession != null)
if (renameService.ActiveSession == null)
{
for (var i = 0; i < args.Count && renameService.ActiveSession != null; i++)
{
renameService.ActiveSession.UndoManager.Redo(args.SubjectBuffer);
}
return false;
}

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

return false;
for (var i = 0; i < args.Count && renameService.ActiveSession != null; i++)
{
renameService.ActiveSession.UndoManager.Redo(args.SubjectBuffer);
}

return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ private bool HandleWordDeleteCommand(ITextBuffer subjectBuffer, ITextView view,
return false;
}

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

var caretPoint = view.GetCaretPoint(subjectBuffer);
if (caretPoint.HasValue)
{
Expand Down
21 changes: 20 additions & 1 deletion src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -744,12 +744,25 @@ private bool CommitSynchronously(bool previewChanges, IUIThreadOperationContext
return _threadingContext.JoinableTaskFactory.Run(() => CommitWorkerAsync(previewChanges, canUseBackgroundWorkIndicator: false, operationContext));
}

/// <summary>
/// Start to commit the rename session.
/// Session might be committed sync or async, depends on the value of InlineRenameUIOptionsStorage.CommitRenameAsynchronously.
/// If it is committed async, method will only kick off the task.
/// </summary>
/// <param name="editorOperationContext"></param>
public void InitiateCommit(IUIThreadOperationContext editorOperationContext = null)
{
var token = _asyncListener.BeginAsyncOperation(nameof(InitiateCommit));
_ = CommitAsync(previewChanges: false, editorOperationContext)
.ReportNonFatalErrorAsync().CompletesAsyncOperation(token);
}

/// <remarks>
/// Caller should pass in the IUIThreadOperationContext if it is called from editor so rename commit operation could set up the its own context correctly.
/// </remarks>
public async Task CommitAsync(bool previewChanges, IUIThreadOperationContext editorOperationContext = null)
{
if (this.RenameService.GlobalOptions.GetOption(InlineRenameSessionOptionsStorage.RenameAsynchronously))
if (this.RenameService.GlobalOptions.ShouldCommitAsynchronously())
{
await CommitWorkerAsync(previewChanges, canUseBackgroundWorkIndicator: true, editorOperationContext).ConfigureAwait(false);
}
Expand Down Expand Up @@ -782,6 +795,12 @@ private async Task<bool> CommitWorkerAsync(bool previewChanges, bool canUseBackg
return false;
}

// Don't dup commit.
if (this.IsCommitInProgress)
{
return false;
}

previewChanges = previewChanges || PreviewChanges;

if (editorUIOperationContext is not null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,10 @@ internal static class InlineRenameSessionOptionsStorage
public static readonly Option2<bool> RenameInComments = new("dotnet_rename_in_comments", defaultValue: false);
public static readonly Option2<bool> RenameFile = new("dotnet_rename_file", defaultValue: true);
public static readonly Option2<bool> PreviewChanges = new("dotnet_preview_inline_rename_changes", defaultValue: false);
public static readonly Option2<bool> RenameAsynchronously = new("dotnet_rename_asynchronously", defaultValue: true);

public static readonly Option2<bool?> CommitRenameAsynchronously = new("dotnet_commit_rename_asynchronously", defaultValue: null);
public static readonly Option2<bool> CommitRenameAsynchronouslyFeatureFlag = new("dotnet_commit_rename_asynchronously_feature_flag", defaultValue: false);

public static bool ShouldCommitAsynchronously(this IGlobalOptionService globalOptionService)
=> globalOptionService.GetOption(CommitRenameAsynchronously) ?? globalOptionService.GetOption(CommitRenameAsynchronouslyFeatureFlag);
}
3 changes: 2 additions & 1 deletion src/EditorFeatures/Test2/Rename/RenameViewModelTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -561,6 +561,8 @@ class D : B
Dim configService = workspace.ExportProvider.GetExportedValue(Of TestWorkspaceConfigurationService)
configService.Options = New WorkspaceConfigurationOptions(SourceGeneratorExecution:=executionPreference)

Dim listenerProvider = workspace.ExportProvider.GetExport(Of IAsynchronousOperationListenerProvider)().Value

Dim cursorDocument = workspace.Documents.Single(Function(d) d.CursorPosition.HasValue)
Dim cursorPosition = cursorDocument.CursorPosition.Value

Expand Down Expand Up @@ -625,7 +627,6 @@ class D : B
End Using

Dim TestQuickInfoBroker = New TestQuickInfoBroker()
Dim listenerProvider = workspace.ExportProvider.GetExport(Of IAsynchronousOperationListenerProvider)().Value
Dim editorFormatMapService = workspace.ExportProvider.GetExport(Of IEditorFormatMapService)().Value

Using flyout = New RenameFlyout(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,10 @@ public AdvancedOptionPageControl(OptionStore optionStore, IComponentModel compon
BindToOption(Always_use_default_symbol_servers_for_navigation, MetadataAsSourceOptionsStorage.AlwaysUseDefaultSymbolServers);

// Rename
BindToOption(Rename_asynchronously_exerimental, InlineRenameSessionOptionsStorage.RenameAsynchronously);
BindToOption(Rename_asynchronously_exerimental, InlineRenameSessionOptionsStorage.CommitRenameAsynchronously, () =>
{
return optionStore.GetOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronouslyFeatureFlag);
});
BindToOption(Rename_UI_setting, InlineRenameUIOptionsStorage.UseInlineAdornment, label: Rename_UI_setting_label);

// Using Directives
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,11 @@ public bool TryFetch(LocalUserRegistryOptionPersister persister, OptionKey2 opti
{"dotnet_rename_use_inline_adornment", new RoamingProfileStorage("TextEditor.RenameUseInlineAdornment")},
{"dotnet_preview_inline_rename_changes", new RoamingProfileStorage("TextEditor.Specific.PreviewRename")},
{"dotnet_collapse_suggestions_in_inline_rename_ui", new RoamingProfileStorage("TextEditor.CollapseRenameSuggestionsUI")},
{"dotnet_commit_rename_asynchronously", new RoamingProfileStorage("TextEditor.Specific.CommitRenameAsynchronously")},
{"dotnet_commit_rename_asynchronously_feature_flag", new FeatureFlagStorage("Roslyn.CommitRenameAsynchronously")},
{"dotnet_rename_get_suggestions_automatically", new FeatureFlagStorage(@"Editor.AutoSmartRenameSuggestions")},
{"dotnet_rename_asynchronously", new RoamingProfileStorage("TextEditor.Specific.RenameAsynchronously")},
// Option is deprecated, don't use the same RoamingProfileStorage key
// {"dotnet_rename_asynchronously", new RoamingProfileStorage("TextEditor.Specific.RenameAsynchronously")},
{"dotnet_rename_file", new RoamingProfileStorage("TextEditor.Specific.RenameFile")},
{"dotnet_rename_in_comments", new RoamingProfileStorage("TextEditor.Specific.RenameInComments")},
{"dotnet_rename_in_strings", new RoamingProfileStorage("TextEditor.Specific.RenameInStrings")},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public override async Task InitializeAsync()
globalOptions.SetGlobalOption(InlineRenameSessionOptionsStorage.RenameOverloads, false);
globalOptions.SetGlobalOption(InlineRenameSessionOptionsStorage.RenameFile, true);
globalOptions.SetGlobalOption(InlineRenameSessionOptionsStorage.PreviewChanges, false);
globalOptions.SetGlobalOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronously, false);
}

[IdeFact]
Expand Down Expand Up @@ -805,4 +806,51 @@ void Method()
// Make sure the file is renamed. If the file is not found, this call would throw exception
await TestServices.SolutionExplorer.GetProjectItemAsync(projectName, "MyTestClass.cs", HangMitigatingCancellationToken);
}

[CombinatorialData]
[IdeTheory]
public async Task VerifyAsyncRename(bool useInlineRename)
{
var globalOptions = await TestServices.Shell.GetComponentModelServiceAsync<IGlobalOptionService>(HangMitigatingCancellationToken);
globalOptions.SetGlobalOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronously, true);

if (!useInlineRename)
globalOptions.SetGlobalOption(InlineRenameUIOptionsStorage.UseInlineAdornment, false);

var markup = """
class Program
{
static void Main(string[] args)
{
int x = 100;
Te$$stMethod(x);
}

static void TestMethod(int y)
{

}
}
""";
await SetUpEditorAsync(markup, HangMitigatingCancellationToken);
await TestServices.InlineRename.InvokeAsync(HangMitigatingCancellationToken);
await TestServices.Input.SendWithoutActivateAsync(["AsyncRenameMethod", VirtualKeyCode.RETURN], HangMitigatingCancellationToken);
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
await TestServices.Workspace.WaitForRenameAsync(HangMitigatingCancellationToken);
await TestServices.EditorVerifier.TextEqualsAsync(
"""
class Program
{
static void Main(string[] args)
{
int x = 100;
AsyncRenameMethod$$(x);
}

static void AsyncRenameMethod(int y)
{

}
}
""", HangMitigatingCancellationToken);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,10 @@ Namespace Microsoft.VisualStudio.LanguageServices.VisualBasic.Options
End Function)

' Rename
BindToOption(Rename_asynchronously_exerimental, InlineRenameSessionOptionsStorage.RenameAsynchronously)
BindToOption(Rename_asynchronously_exerimental, InlineRenameSessionOptionsStorage.CommitRenameAsynchronously,
Function()
Return optionStore.GetOption(InlineRenameSessionOptionsStorage.CommitRenameAsynchronouslyFeatureFlag)
End Function)
BindToOption(Rename_UI_setting, InlineRenameUIOptionsStorage.UseInlineAdornment, label:=Rename_UI_setting_label)

' Import directives
Expand Down
Loading