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 10 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.CommitAsync(previewChanges: false).ReportNonFatalErrorAsync();
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Automation.Peers;
using System.Windows.Controls;
Expand Down Expand Up @@ -243,7 +244,7 @@ protected override void OnAccessKey(AccessKeyEventArgs e)
}
else if (string.Equals(e.Key, RenameShortcutKey.Apply, StringComparison.OrdinalIgnoreCase))
{
this.Commit();
_ = this.CommitAsync();
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down Expand Up @@ -318,13 +319,14 @@ private void CloseButton_Click(object sender, RoutedEventArgs e)
}

private void Apply_Click(object sender, RoutedEventArgs e)
=> Commit();
=> _ = CommitAsync();

private void Commit()
private async Task CommitAsync()
{
try
{
_model.Session.Commit();
//.ConfigureAwait(true) to make sure Exceptions could be shown in UI.
await _model.Session.CommitAsync(previewChanges: false).ConfigureAwait(true);
_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 @@ -66,6 +66,11 @@ private void HandlePossibleTypingCommand<TArgs>(TArgs args, Action nextHandler,
return;
}

if (_renameService.ActiveSession.IsCommitInProgress)
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
{
return;
}

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

if (selectedSpans.Count > 1)
Expand Down Expand Up @@ -121,4 +126,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,14 @@ 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.TryGetContainingEditableSpan(caretPoint.Value, out _))
if (_renameService.ActiveSession.IsCommitInProgress)
{
return;
}
else 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 @@ -6,6 +6,7 @@
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Editor.Commanding.Commands;
using Microsoft.VisualStudio.Utilities;
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename;

Expand All @@ -27,7 +28,7 @@ public bool ExecuteCommand(ReturnKeyCommandArgs args, CommandExecutionContext co

protected virtual void CommitAndSetFocus(InlineRenameSession activeSession, ITextView textView, IUIThreadOperationContext operationContext)
{
Commit(operationContext);
_ = activeSession.CommitAsync(previewChanges: false, operationContext).ReportNonFatalErrorAsync();
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
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, handle the command so it won't change the workspace
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

var caretPoint = view.GetCaretPoint(subjectBuffer);
if (caretPoint.HasValue)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ private bool CommitSynchronously(bool previewChanges, IUIThreadOperationContext
/// </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 +782,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 @@ -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 System;
using Microsoft.CodeAnalysis.Options;

namespace Microsoft.CodeAnalysis.InlineRename;
Expand All @@ -13,5 +14,13 @@ 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);

[Obsolete]
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
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);
}
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,6 +344,8 @@ 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")},
{"dotnet_rename_file", new RoamingProfileStorage("TextEditor.Specific.RenameFile")},
Expand Down
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