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

Async rename commit #74794

Draft
wants to merge 63 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
a540480
Link the cancellationToken
Cosifne Aug 14, 2024
09cc8e7
Add another entry
Cosifne Aug 14, 2024
d033ed5
Revise the meaning of rename async option
Cosifne Aug 14, 2024
0ad7ba7
Link call
Cosifne Aug 14, 2024
22881fe
Make it async in ExtractMethodCommandHandler
Cosifne Aug 16, 2024
51bac52
Update async in SaveHandler
Cosifne Aug 16, 2024
f6014e1
Update async in ReturnKey Handler
Cosifne Aug 16, 2024
02ebbc2
Update async in RenameHandler
Cosifne Aug 16, 2024
e87179c
Update in openline above
Cosifne Aug 16, 2024
879e719
Reset to sync
Cosifne Aug 16, 2024
0c6b511
Reset change
Cosifne Aug 16, 2024
5128476
Add test hook
Cosifne Aug 16, 2024
51ca38e
Reset
Cosifne Aug 16, 2024
0ec2779
Clean and remove usings
Cosifne Aug 16, 2024
55d45e4
Avoid commit a same session twice
Cosifne Aug 16, 2024
88ba81a
Cancel when background task is not completed
Cosifne Aug 17, 2024
4f1a7e7
Prevent input from user when commit in progress
Cosifne Aug 17, 2024
2116ed9
Introduce a simple way to hide the UI when commit in progress
Cosifne Aug 17, 2024
2ec6fac
Don't handle ESC
Cosifne Aug 17, 2024
3b199f9
Add a null check
Cosifne Aug 19, 2024
d8bb916
Wrap try catch
Cosifne Aug 19, 2024
7e1c13e
Reset the legacy UI to sync
Cosifne Aug 19, 2024
226cb7c
Code clean
Cosifne Aug 19, 2024
c2a6b1d
Clean up
Cosifne Aug 19, 2024
98d9f5c
Clean up
Cosifne Aug 19, 2024
9d06662
Add CommitState
Cosifne Aug 19, 2024
56708df
Revise using the state
Cosifne Aug 19, 2024
17efc48
Use State to cancel
Cosifne Aug 19, 2024
cd99b6b
Revert "Add CommitState"
Cosifne Aug 20, 2024
491e453
Revert "Revise using the state"
Cosifne Aug 20, 2024
39ac7cd
Revert "Use State to cancel"
Cosifne Aug 20, 2024
b61d2eb
Clean cancellationToken
Cosifne Aug 20, 2024
f2f3a29
Merge remote-tracking branch 'upstream/main' into dev/shech/RenameCom…
Cosifne Aug 20, 2024
a4b5f89
Fix errors after merge
Cosifne Aug 20, 2024
27e6efe
Add one more check
Cosifne Aug 20, 2024
9d029b9
Add commment
Cosifne Aug 21, 2024
2cd8e81
Simplify
Cosifne Aug 21, 2024
270b294
Fix test
Cosifne Aug 21, 2024
22ada15
Add comment
Cosifne Aug 21, 2024
89eb553
Also use async in legacy UI
Cosifne Aug 21, 2024
c676263
Bind to checkbox
Cosifne Aug 21, 2024
4360883
Add comment
Cosifne Aug 21, 2024
377be4e
Merge remote-tracking branch 'upstream/main' into dev/shech/RenameCom…
Cosifne Aug 22, 2024
5ccde93
Directly report errors
Cosifne Aug 22, 2024
ae55915
Wrap try-catch
Cosifne Aug 23, 2024
91089e9
Add comment and threadingContext
Cosifne Aug 23, 2024
6256a96
Also forward commitSync to StartCommitAsync
Cosifne Aug 23, 2024
66b289e
Add comment
Cosifne Aug 23, 2024
4b810d3
Clean up
Cosifne Aug 23, 2024
374f8e9
Fix test
Cosifne Aug 24, 2024
e8b26b9
Merge branch 'main' into dev/shech/RenameCommitAsync
Cosifne Aug 28, 2024
f4cf9a2
Add comment
Cosifne Aug 28, 2024
318eedd
Remove a duplicate TakeOwnerShip()
Cosifne Aug 29, 2024
5e4bb05
Change the behavior back to origin
Cosifne Aug 29, 2024
5d01d76
Add assert
Cosifne Aug 29, 2024
6117838
Add comment
Cosifne Aug 29, 2024
9dad713
Merge remote-tracking branch 'upstream/main' into dev/shech/RenameCom…
Cosifne Sep 19, 2024
37b496d
Fix missing code after merge
Cosifne Sep 19, 2024
82ae965
Fix missing code after merge
Cosifne Sep 19, 2024
50c2236
Pass globalOptionService
Cosifne Sep 20, 2024
f089e3b
Reset
Cosifne Sep 20, 2024
68f14ef
Add option
Cosifne Sep 23, 2024
c540ddf
Reset
Cosifne Sep 23, 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 @@ -5,6 +5,8 @@
using System;
using System.ComponentModel.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.VisualStudio.Commanding;
Expand Down Expand Up @@ -94,11 +96,11 @@ protected override void SetAdornmentFocusToPreviousElement(ITextView textView)
}
}

protected override void Commit(InlineRenameSession activeSession, ITextView textView)
protected override async Task CommitAndSetFocusAsync(InlineRenameSession activeSession, ITextView textView)
{
try
{
base.Commit(activeSession, textView);
await base.CommitAndSetFocusAsync(activeSession, textView).ConfigureAwait(false);
}
catch (NotSupportedException ex)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
Focusable="False"
UseLayoutRounding="True"
Cursor="Arrow"
Visibility="{Binding Path=Visibility}"
x:Name="control">

<UserControl.Resources>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,34 @@
using System.Diagnostics;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Interop;
using Microsoft.CodeAnalysis.Editor.InlineRename;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.EditorFeatures.Lightup;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.InlineRename;
using Microsoft.CodeAnalysis.InlineRename.UI.SmartRename;
using Microsoft.CodeAnalysis.Options;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Telemetry;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.Imaging;
using Microsoft.VisualStudio.Imaging.Interop;
using Microsoft.VisualStudio.PlatformUI.OleComponentSupport;
using Microsoft.VisualStudio.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Editor.Implementation.InlineRename
{
internal class RenameFlyoutViewModel : INotifyPropertyChanged, IDisposable
{
private readonly bool _registerOleComponent;
private readonly IGlobalOptionService _globalOptionService;
private readonly IAsynchronousOperationListener _asyncListener;
private OleComponent? _oleComponent;
private bool _disposedValue;
private bool _isReplacementTextValid = true;
Expand All @@ -52,8 +59,10 @@ public RenameFlyoutViewModel(
Session.ReplacementTextChanged += OnReplacementTextChanged;
Session.ReplacementsComputed += OnReplacementsComputed;
Session.ReferenceLocationsChanged += OnReferenceLocationsChanged;
Session.CommitStateChange += SessionOnCommitStateChange;
StartingSelection = selectionSpan;
InitialTrackingSpan = session.TriggerSpan.CreateTrackingSpan(SpanTrackingMode.EdgeInclusive);
_asyncListener = listenerProvider.GetListener(FeatureAttribute.Rename);
var smartRenameSession = smartRenameSessionFactory?.Value.CreateSmartRenameSession(Session.TriggerSpan);
if (smartRenameSession is not null)
{
Expand All @@ -63,6 +72,13 @@ public RenameFlyoutViewModel(
RegisterOleComponent();
}

private void SessionOnCommitStateChange(object sender, bool commitStarts)
{
// When commit in progress, we will use background indicator to show the progress.
// Rename flyout would hide the tooltip so we need to collapse it.
Visibility = commitStarts ? Visibility.Collapsed : Visibility.Visible;
}

public SmartRenameViewModel? SmartRenameViewModel { get; }

public string IdentifierText
Expand All @@ -73,7 +89,7 @@ public string IdentifierText
if (value != Session.ReplacementText)
{
Session.ApplyReplacementText(value, propagateEditImmediately: true, updateSelection: false);
NotifyPropertyChanged(nameof(IdentifierText));
NotifyPropertyChanged();
}
}
}
Expand Down Expand Up @@ -206,20 +222,64 @@ public bool IsRenameOverloadsEditable
public bool IsRenameOverloadsVisible
=> Session.HasRenameOverloads;

public bool AllowUserInput
=> !Session.IsCommitInProgress;

private Visibility _visibility;
public Visibility Visibility
{
get => _visibility;
set
{
if (value != _visibility)
{
Set(ref _visibility, value);
NotifyPropertyChanged();
}
}
}

public TextSpan StartingSelection { get; }

public bool Submit()
{
using var token = _asyncListener.BeginAsyncOperation(nameof(Submit));
if (StatusSeverity == Severity.Error)
{
return false;
}

SmartRenameViewModel?.Commit(IdentifierText);
Session.Commit();
_ = CommitAsync().ReportNonFatalErrorAsync();
return true;
}

private async Task CommitAsync()
{
try
{
await Session.CommitAsync(previewChanges: false).ReportNonFatalErrorAsync().ConfigureAwait(false);
}
catch (Exception ex) when (FatalError.ReportAndCatch(ex, ErrorSeverity.Critical))
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
{
// Show a nice error to the user via an info bar
var errorReportingService = Session.Workspace.Services.GetService<IErrorReportingService>();
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
if (errorReportingService is null)
{
return;
}

errorReportingService.ShowGlobalErrorInfo(
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
message: string.Format(EditorFeaturesWpfResources.Error_performing_rename_0, ex.Message),
TelemetryFeatureName.InlineRename,
ex,
new InfoBarUI(
WorkspacesResources.Show_Stack_Trace,
InfoBarUI.UIKind.HyperLink,
() => errorReportingService.ShowDetailedErrorInfo(ex), closeAfterAction: true));
}
}

public void Cancel()
{
SmartRenameViewModel?.Cancel();
Expand Down Expand Up @@ -310,6 +370,7 @@ protected virtual void Dispose(bool disposing)
{
Session.ReplacementTextChanged -= OnReplacementTextChanged;
Session.ReplacementsComputed -= OnReplacementsComputed;
Session.CommitStateChange -= SessionOnCommitStateChange;

if (SmartRenameViewModel is not null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,16 +145,16 @@

<CheckBox Content="{Binding ElementName=dashboard, Path=RenameOverloads}" Margin="0,0,0,0" IsChecked="{Binding Path=DefaultRenameOverloadFlag, Mode=TwoWay}"
Name="OverloadsCheckbox" Visibility="{Binding ElementName=dashboard, Path=RenameOverloadsVisibility}" IsEnabled="{Binding ElementName=dashboard, Path=IsRenameOverloadsEditable}" />
<CheckBox Name="CommentsCheckbox" Content="{Binding ElementName=dashboard, Path=SearchInComments}" Margin="0,0,0,0" IsChecked="{Binding Path=DefaultRenameInCommentsFlag, Mode=TwoWay}" />
<CheckBox Name="StringsCheckbox" Content="{Binding ElementName=dashboard, Path=SearchInStrings}" Margin="0,0,0,0" IsChecked="{Binding Path=DefaultRenameInStringsFlag, Mode=TwoWay}" />
<CheckBox Name="CommentsCheckbox" Content="{Binding ElementName=dashboard, Path=SearchInComments}" Margin="0,0,0,0" IsChecked="{Binding Path=DefaultRenameInCommentsFlag, Mode=TwoWay}" IsEnabled="{Binding Path=CommitNotStart}" />
<CheckBox Name="StringsCheckbox" Content="{Binding ElementName=dashboard, Path=SearchInStrings}" Margin="0,0,0,0" IsChecked="{Binding Path=DefaultRenameInStringsFlag, Mode=TwoWay}" IsEnabled="{Binding Path=CommitNotStart}" />
<CheckBox Name="FileRenameCheckbox"
Content="{Binding Path=FileRenameString}"
Margin="0"
IsChecked="{Binding Path=DefaultRenameFileFlag, Mode=TwoWay}"
IsEnabled="{Binding Path=AllowFileRename, Mode=OneWay}"
Visibility="{Binding Path=ShowFileRename, Converter={StaticResource BooleanToVisibilityConverter}, Mode=OneWay}"/>

<CheckBox Name="PreviewChangesCheckbox" Content="{Binding ElementName=dashboard, Path=PreviewChanges}" Margin="0,8,0,0" IsChecked="{Binding Path=DefaultPreviewChangesFlag, Mode=TwoWay}" />
<CheckBox Name="PreviewChangesCheckbox" Content="{Binding ElementName=dashboard, Path=PreviewChanges}" Margin="0,8,0,0" IsChecked="{Binding Path=DefaultPreviewChangesFlag, Mode=TwoWay}" IsEnabled="{Binding Path=CommitNotStart}" />

<!-- Summary: Includes the number of references to be updated and any conflict information -->

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 @@ -328,10 +329,15 @@ private void Apply_Click(object sender, RoutedEventArgs e)
=> Commit();

private void Commit()
{
_ = CommitAsync();
}

private async Task CommitAsync()
{
try
{
_model.Session.Commit();
await _model.Session.CommitAsync(previewChanges: false).ConfigureAwait(true);
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
_textView.VisualElement.Focus();
}
catch (NotSupportedException ex)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ internal class RenameDashboardViewModel : INotifyPropertyChanged, IDisposable
private int _resolvableConflictCount;
private int _unresolvableConflictCount;
private bool _isReplacementTextValid;
private bool _commitNotStart;

public RenameDashboardViewModel(InlineRenameSession session)
{
Expand All @@ -29,9 +30,11 @@ public RenameDashboardViewModel(InlineRenameSession session)
Session.ReferenceLocationsChanged += OnReferenceLocationsChanged;
Session.ReplacementsComputed += OnReplacementsComputed;
Session.ReplacementTextChanged += OnReplacementTextChanged;
Session.CommitStateChange += CommitStateChange;

// Set the flag to true by default if we're showing the option.
_isReplacementTextValid = true;
_commitNotStart = true;
}

public event PropertyChangedEventHandler PropertyChanged;
Expand Down Expand Up @@ -140,11 +143,30 @@ private void UpdateSeverity()
}
}

private void CommitStateChange(object sender, bool commitStart)
=> CommitNotStart = !commitStart;

public bool CommitNotStart
{
get => _commitNotStart;
Cosifne marked this conversation as resolved.
Show resolved Hide resolved
set
{
if (_commitNotStart != value)
{
_commitNotStart = value;
// Disable/Enable these checkbox in UI based on if commit is in-progress or not
NotifyPropertyChanged();
NotifyPropertyChanged(nameof(IsRenameOverloadsEditable));
NotifyPropertyChanged(nameof(AllowFileRename));
}
}
}

public InlineRenameSession Session { get; }

public RenameDashboardSeverity Severity => _severity;

public bool AllowFileRename => Session.FileRenameInfo == InlineRenameFileRenameInfo.Allowed && _isReplacementTextValid;
public bool AllowFileRename => Session.FileRenameInfo == InlineRenameFileRenameInfo.Allowed && _isReplacementTextValid && _commitNotStart;
public bool ShowFileRename => Session.FileRenameInfo != InlineRenameFileRenameInfo.NotAllowed;
public string FileRenameString => Session.FileRenameInfo switch
{
Expand Down Expand Up @@ -226,7 +248,7 @@ public Visibility RenameOverloadsVisibility
=> Session.HasRenameOverloads ? Visibility.Visible : Visibility.Collapsed;

public bool IsRenameOverloadsEditable
=> !Session.MustRenameOverloads;
=> !Session.MustRenameOverloads && CommitNotStart;

public bool DefaultRenameOverloadFlag
{
Expand Down Expand Up @@ -292,6 +314,7 @@ public void Dispose()
Session.ReplacementTextChanged -= OnReplacementTextChanged;
Session.ReferenceLocationsChanged -= OnReferenceLocationsChanged;
Session.ReplacementsComputed -= OnReplacementsComputed;
Session.CommitStateChange -= CommitStateChange;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,6 @@ public CommandState GetCommandState(ExtractMethodCommandArgs args)

public bool ExecuteCommand(ExtractMethodCommandArgs args, CommandExecutionContext context)
{
// Finish any rename that had been started. We'll do this here before we enter the
// wait indicator for Extract Method
if (_renameService.ActiveSession != null)
{
_threadingContext.JoinableTaskFactory.Run(() => _renameService.ActiveSession.CommitAsync(previewChanges: false));
}

if (!args.SubjectBuffer.SupportsRefactorings())
return false;

Expand All @@ -106,7 +99,7 @@ public bool ExecuteCommand(ExtractMethodCommandArgs args, CommandExecutionContex
if (document is null)
return false;

_ = ExecuteAsync(view, textBuffer, document, span);
_ = ExecuteAsync(view, textBuffer, document, span).ReportNonFatalErrorAsync();
return true;
}

Expand All @@ -117,6 +110,15 @@ private async Task ExecuteAsync(
SnapshotSpan span)
{
_threadingContext.ThrowIfNotOnUIThread();

// Finish any rename that had been started. We'll do this here before we enter the
// wait indicator for Extract Method
if (_renameService.ActiveSession != null)
{
// ConfigureAwait(true) to make sure the next wait indicator would be created correctly.
await _renameService.ActiveSession.CommitAsync(previewChanges: false).ConfigureAwait(true);
}

var indicatorFactory = document.Project.Solution.Services.GetRequiredService<IBackgroundWorkIndicatorFactory>();
using var indicatorContext = indicatorFactory.Create(
view, span, EditorFeaturesResources.Applying_Extract_Method_refactoring, cancelOnEdit: true, cancelOnFocusLost: true);
Expand Down
1 change: 1 addition & 0 deletions src/EditorFeatures/Core/IInlineRenameSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,5 +50,6 @@ internal interface IInlineRenameSession
/// <summary>
/// Dismisses the rename session, completing the rename operation across all files.
/// </summary>
/// <remarks>The implementation would only be async when InlineRenameSessionOptionsStorage.RenameAsynchronously is set to true</remarks>
Task CommitAsync(bool previewChanges);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Linq;
using System.Threading;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Shared.TestHooks;
Expand Down Expand Up @@ -64,8 +65,13 @@ private void HandlePossibleTypingCommand<TArgs>(TArgs args, Action nextHandler,
return;
}

var selectedSpans = args.TextView.Selection.GetSnapshotSpansOnBuffer(args.SubjectBuffer);
// If commit in progress, don't do anything, as we don't want user change the text view
if (_renameService.ActiveSession.IsCommitInProgress)
{
return;
}

var selectedSpans = args.TextView.Selection.GetSnapshotSpansOnBuffer(args.SubjectBuffer);
if (selectedSpans.Count > 1)
{
// If we have multiple spans active, then that means we have something like box
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ public bool ExecuteCommand(EscapeKeyCommandArgs args, CommandExecutionContext co
{
_renameService.ActiveSession.Cancel();
SetFocusToTextView(args.TextView);
return true;
// When ESC is pressed, don't handle the command here because rename might rely on
// BackgroundWorkIndicator to show the progress. Let platform propagates ESC to let indicator also get cancelled.
}

return false;
Expand Down
Loading
Loading