From 645a1404e910a0cd565c5ec3340e36dc32969ea7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 25 May 2022 12:24:26 -0700 Subject: [PATCH 1/8] Unify error handling in rename --- .../Core/EditorFeaturesResources.resx | 3 + .../Core/InlineRename/InlineRenameSession.cs | 66 +++---- ...TaggerProvider.RenameTrackingCodeAction.cs | 10 +- ...gTaggerProvider.RenameTrackingCommitter.cs | 164 ++++++++++-------- .../Core/xlf/EditorFeaturesResources.cs.xlf | 5 + .../Core/xlf/EditorFeaturesResources.de.xlf | 5 + .../Core/xlf/EditorFeaturesResources.es.xlf | 5 + .../Core/xlf/EditorFeaturesResources.fr.xlf | 5 + .../Core/xlf/EditorFeaturesResources.it.xlf | 5 + .../Core/xlf/EditorFeaturesResources.ja.xlf | 5 + .../Core/xlf/EditorFeaturesResources.ko.xlf | 5 + .../Core/xlf/EditorFeaturesResources.pl.xlf | 5 + .../xlf/EditorFeaturesResources.pt-BR.xlf | 5 + .../Core/xlf/EditorFeaturesResources.ru.xlf | 5 + .../Core/xlf/EditorFeaturesResources.tr.xlf | 5 + .../xlf/EditorFeaturesResources.zh-Hans.xlf | 5 + .../xlf/EditorFeaturesResources.zh-Hant.xlf | 5 + 17 files changed, 201 insertions(+), 107 deletions(-) diff --git a/src/EditorFeatures/Core/EditorFeaturesResources.resx b/src/EditorFeatures/Core/EditorFeaturesResources.resx index 0198ff4cc9647..32f7993293630 100644 --- a/src/EditorFeatures/Core/EditorFeaturesResources.resx +++ b/src/EditorFeatures/Core/EditorFeaturesResources.resx @@ -1070,4 +1070,7 @@ Do you want to proceed? {0} (Esc to cancel) Placeholder represents an operating that is happening. Parenthesized expression indicates how you can cancel it. + + Rename operation could not complete due to external change to workspace + \ No newline at end of file diff --git a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs index df1f2faad400d..4082c3ab5a4e4 100644 --- a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs +++ b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs @@ -788,24 +788,31 @@ private void CommitCore(IUIThreadOperationContext operationContext, bool preview DismissUIAndRollbackEditsAndEndRenameSession( RenameLogMessage.UserActionOutcome.Committed, previewChanges, - () => ApplyRename(newSolution, operationContext)); + () => + { + var error = TryApplyRename(newSolution); + + if (error is not null) + { + operationContext.TakeOwnership(); + var notificationService = _workspace.Services.GetService(); + notificationService.SendNotification( + error, EditorFeaturesResources.Rename_Symbol, NotificationSeverity.Error); + } + }); } } - private void ApplyRename(Solution newSolution, IUIThreadOperationContext operationContext) + /// + /// Returns non-null error message if renaming fails. + /// + private string TryApplyRename(Solution newSolution) { var changes = _baseSolution.GetChanges(newSolution); var changedDocumentIDs = changes.GetProjectChanges().SelectMany(c => c.GetChangedDocuments()).ToList(); if (!_renameInfo.TryOnBeforeGlobalSymbolRenamed(_workspace, changedDocumentIDs, this.ReplacementText)) - { - var notificationService = _workspace.Services.GetService(); - notificationService.SendNotification( - EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid, - EditorFeaturesResources.Rename_Symbol, - NotificationSeverity.Error); - return; - } + return EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid; using var undoTransaction = _workspace.OpenGlobalUndoTransaction(EditorFeaturesResources.Inline_Rename); var finalSolution = newSolution.Workspace.CurrentSolution; @@ -837,31 +844,26 @@ private void ApplyRename(Solution newSolution, IUIThreadOperationContext operati finalSolution = finalSolution.WithDocumentName(id, newDocument.Name); } - if (_workspace.TryApplyChanges(finalSolution)) - { - // Since rename can apply file changes as well, and those file - // changes can generate new document ids, include added documents - // as well as changed documents. This also ensures that any document - // that was removed is not included - var finalChanges = _workspace.CurrentSolution.GetChanges(_baseSolution); + if (!_workspace.TryApplyChanges(finalSolution)) + return EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace; - var finalChangedIds = finalChanges - .GetProjectChanges() - .SelectMany(c => c.GetChangedDocuments().Concat(c.GetAddedDocuments())) - .ToList(); + // Since rename can apply file changes as well, and those file + // changes can generate new document ids, include added documents + // as well as changed documents. This also ensures that any document + // that was removed is not included + var finalChanges = _workspace.CurrentSolution.GetChanges(_baseSolution); - if (!_renameInfo.TryOnAfterGlobalSymbolRenamed(_workspace, finalChangedIds, this.ReplacementText)) - { - var notificationService = _workspace.Services.GetService(); - operationContext.TakeOwnership(); - notificationService.SendNotification( - EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated, - EditorFeaturesResources.Rename_Symbol, - NotificationSeverity.Information); - } + var finalChangedIds = finalChanges + .GetProjectChanges() + .SelectMany(c => c.GetChangedDocuments().Concat(c.GetAddedDocuments())) + .ToList(); - undoTransaction.Commit(); - } + var result = !_renameInfo.TryOnAfterGlobalSymbolRenamed(_workspace, finalChangedIds, this.ReplacementText) + ? EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated + : null; + + undoTransaction.Commit(); + return result; } internal bool TryGetContainingEditableSpan(SnapshotPoint point, out SnapshotSpan editableSpan) diff --git a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs index ed031d74baf1c..cf3295b8502ac 100644 --- a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs +++ b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs @@ -110,7 +110,15 @@ public RenameTrackingCommitterOperation(RenameTrackingCommitter committer) => _committer = committer; public override void Apply(Workspace workspace, CancellationToken cancellationToken) - => _committer.Commit(cancellationToken); + { + var error = _committer.TryCommit(cancellationToken); + if (error != null) + { + var notificationService = workspace.Services.GetService(); + notificationService.SendNotification( + error, EditorFeaturesResources.Rename_Symbol, NotificationSeverity.Error); + } + } } } } diff --git a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs index fbc28efe0c2ff..05b0c75d0dda8 100644 --- a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs +++ b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs @@ -49,16 +49,24 @@ public RenameTrackingCommitter( _renameSymbolResultGetter = new AsyncLazy(c => RenameSymbolWorkerAsync(c), cacheResult: true); } - public void Commit(CancellationToken cancellationToken) + /// + /// Returns non-null error message if renaming fails. + /// + public string TryCommit(CancellationToken cancellationToken) { _stateMachine.ThreadingContext.ThrowIfNotOnUIThread(); - ApplyChangesToWorkspace(cancellationToken); - - // Clear the state machine so that future updates to the same token work, - // and any text changes caused by this update are not interpreted as - // potential renames - _stateMachine.ClearTrackingSession(); + try + { + return TryApplyChangesToWorkspace(cancellationToken); + } + finally + { + // Clear the state machine so that future updates to the same token work, + // and any text changes caused by this update are not interpreted as + // potential renames + _stateMachine.ClearTrackingSession(); + } } public async Task RenameSymbolAsync(CancellationToken cancellationToken) @@ -82,7 +90,7 @@ private async Task RenameSymbolWorkerAsync(Cancellati return new RenameTrackingSolutionSet(symbol, solutionWithOriginalName, renamedSolution); } - private void ApplyChangesToWorkspace(CancellationToken cancellationToken) + private string TryApplyChangesToWorkspace(CancellationToken cancellationToken) { _stateMachine.ThreadingContext.ThrowIfNotOnUIThread(); @@ -120,48 +128,46 @@ private void ApplyChangesToWorkspace(CancellationToken cancellationToken) // update the state machine after undo/redo actions. var changedDocuments = renameTrackingSolutionSet.RenamedSolution.GetChangedDocuments(renameTrackingSolutionSet.OriginalSolution); - - // When this action is undone (the user has undone twice), restore the state - // machine to so that they can continue their original rename tracking session. - - var trackingSessionId = _stateMachine.StoreCurrentTrackingSessionAndGenerateId(); - UpdateWorkspaceForResetOfTypedIdentifier(workspace, renameTrackingSolutionSet.OriginalSolution, trackingSessionId); - - // Now that the solution is back in its original state, notify third parties about - // the coming rename operation. - if (!_refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocuments, renameTrackingSolutionSet.Symbol, newName, throwOnFailure: false)) + try { - var notificationService = workspace.Services.GetService(); - notificationService.SendNotification( - EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid, - EditorFeaturesResources.Rename_Symbol, - NotificationSeverity.Error); - - return; + // When this action is undone (the user has undone twice), restore the state + // machine to so that they can continue their original rename tracking session. + + var trackingSessionId = _stateMachine.StoreCurrentTrackingSessionAndGenerateId(); + var result = TryUpdateWorkspaceForResetOfTypedIdentifier(workspace, renameTrackingSolutionSet.OriginalSolution, trackingSessionId); + if (result is not null) + return result; + + // Now that the solution is back in its original state, notify third parties about + // the coming rename operation. + if (!_refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocuments, renameTrackingSolutionSet.Symbol, newName, throwOnFailure: false)) + return EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid; + + // move all changes to final solution based on the workspace's current solution, since the current solution + // got updated when we reset it above. + var finalSolution = workspace.CurrentSolution; + foreach (var docId in changedDocuments) + { + // because changes have already been made to the workspace (UpdateWorkspaceForResetOfTypedIdentifier() above), + // these calls can't be cancelled and must be allowed to complete. + var root = renameTrackingSolutionSet.RenamedSolution.GetDocument(docId).GetSyntaxRootSynchronously(CancellationToken.None); + finalSolution = finalSolution.WithDocumentSyntaxRoot(docId, root); + } + + // Undo/redo on this action must always clear the state machine + return TryUpdateWorkspaceForGlobalIdentifierRename( + workspace, + finalSolution, + _displayText, + changedDocuments, + renameTrackingSolutionSet.Symbol, + newName, + trackingSessionId); } - - // move all changes to final solution based on the workspace's current solution, since the current solution - // got updated when we reset it above. - var finalSolution = workspace.CurrentSolution; - foreach (var docId in changedDocuments) + finally { - // because changes have already been made to the workspace (UpdateWorkspaceForResetOfTypedIdentifier() above), - // these calls can't be cancelled and must be allowed to complete. - var root = renameTrackingSolutionSet.RenamedSolution.GetDocument(docId).GetSyntaxRootSynchronously(CancellationToken.None); - finalSolution = finalSolution.WithDocumentSyntaxRoot(docId, root); + RenameTrackingDismisser.DismissRenameTracking(workspace, changedDocuments); } - - // Undo/redo on this action must always clear the state machine - UpdateWorkspaceForGlobalIdentifierRename( - workspace, - finalSolution, - _displayText, - changedDocuments, - renameTrackingSolutionSet.Symbol, - newName, - trackingSessionId); - - RenameTrackingDismisser.DismissRenameTracking(workspace, changedDocuments); } private Solution CreateSolutionWithOriginalName(Document document, CancellationToken cancellationToken) @@ -203,7 +209,10 @@ private async Task TryGetSymbolAsync(Solution solutionWithOriginalName, return tokenRenameInfo.HasSymbols ? tokenRenameInfo.Symbols.First() : null; } - private void UpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, Solution newSolution, int trackingSessionId) + /// + /// Returns non-null error message if renaming fails. + /// + private string TryUpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, Solution newSolution, int trackingSessionId) { _stateMachine.ThreadingContext.ThrowIfNotOnUIThread(); @@ -213,22 +222,30 @@ private void UpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, Solut var undoHistory = _undoHistoryRegistry.RegisterHistory(_stateMachine.Buffer); using var localUndoTransaction = undoHistory.CreateTransaction(EditorFeaturesResources.Text_Buffer_Change); - var undoPrimitiveBefore = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: true); - localUndoTransaction.AddUndo(undoPrimitiveBefore); - - if (!workspace.TryApplyChanges(newSolution)) + try { - Contract.Fail("Rename Tracking could not update solution."); - } + var undoPrimitiveBefore = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: true); + localUndoTransaction.AddUndo(undoPrimitiveBefore); + + if (!workspace.TryApplyChanges(newSolution)) + return EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace; - // Never resume tracking session on redo - var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); - localUndoTransaction.AddUndo(undoPrimitiveAfter); + return null; + } + finally + { + // Never resume tracking session on redo + var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); + localUndoTransaction.AddUndo(undoPrimitiveAfter); - localUndoTransaction.Complete(); + localUndoTransaction.Complete(); + } } - private void UpdateWorkspaceForGlobalIdentifierRename( + /// + /// Returns non-null error message if renaming fails. + /// + private string TryUpdateWorkspaceForGlobalIdentifierRename( Workspace workspace, Solution newSolution, string undoName, @@ -250,26 +267,25 @@ private void UpdateWorkspaceForGlobalIdentifierRename( var undoPrimitiveBefore = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); localUndoTransaction.AddUndo(undoPrimitiveBefore); - if (!workspace.TryApplyChanges(newSolution)) + try { - Contract.Fail("Rename Tracking could not update solution."); - } + if (!workspace.TryApplyChanges(newSolution)) + return EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace; - if (!_refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: false)) - { - var notificationService = workspace.Services.GetService(); - notificationService.SendNotification( - EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated, - EditorFeaturesResources.Rename_Symbol, - NotificationSeverity.Information); - } + if (!_refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: false)) + return EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated; - // Never resume tracking session on redo - var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); - localUndoTransaction.AddUndo(undoPrimitiveAfter); + return null; + } + finally + { + // Never resume tracking session on redo + var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); + localUndoTransaction.AddUndo(undoPrimitiveAfter); - localUndoTransaction.Complete(); - workspaceUndoTransaction.Commit(); + localUndoTransaction.Complete(); + workspaceUndoTransaction.Commit(); + } } } } diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.cs.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.cs.xlf index f0491196ad61a..85291c3d70bad 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.cs.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.cs.xlf @@ -297,6 +297,11 @@ Přejmenovat _soubor (nepovoleno pro dílčí typy) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file Přejmenovat _soubor symbolu diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.de.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.de.xlf index 398190519ee95..79c08855b6446 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.de.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.de.xlf @@ -297,6 +297,11 @@ _Datei umbenennen (bei partiellen Typen nicht zulässig) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file _Datei des Symbols umbenennen diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.es.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.es.xlf index 3a0761aadbc69..a535733f97ec4 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.es.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.es.xlf @@ -297,6 +297,11 @@ Ca_mbiar nombre de archivo (no se permite en tipos parciales) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file Cambiar nombre del _archivo del símbolo diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.fr.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.fr.xlf index 733c8712c814f..1e12f48c74008 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.fr.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.fr.xlf @@ -297,6 +297,11 @@ Renommer le _fichier (non autorisé sur les types partiels) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file Renommer le _fichier du symbole diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.it.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.it.xlf index 0864424667360..1cf2e6a450400 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.it.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.it.xlf @@ -297,6 +297,11 @@ Rinomina _file (non consentito su tipi parziali) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file Rinomina il _file del simbolo diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ja.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ja.xlf index f717f8abcf849..e4f3ee42cb619 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ja.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ja.xlf @@ -297,6 +297,11 @@ ファイル名の変更 (部分型では許可されません)(_F) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file シンボルのファイルの名前を変更する(_F) diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ko.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ko.xlf index da6caad5df69b..aacab107a8f1a 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ko.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ko.xlf @@ -297,6 +297,11 @@ 파일 이름 바꾸기(부분 형식에서 허용되지 않음)(_F) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file 기호의 파일 이름 바꾸기(_F) diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.pl.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.pl.xlf index d2e6f9271e7a1..79520b1c73038 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.pl.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.pl.xlf @@ -297,6 +297,11 @@ Zmień nazwę _pliku (niedozwolone dla typów częściowych) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file Zmień nazwę _pliku symbolu diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.pt-BR.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.pt-BR.xlf index ea504baed48e2..779cd58a0ae0c 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.pt-BR.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.pt-BR.xlf @@ -297,6 +297,11 @@ Renomear _arquivo (não permitido em tipos parciais) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file Renomear o _arquivo do símbolo diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ru.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ru.xlf index 3951770f0f941..57264a328d2d6 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ru.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.ru.xlf @@ -297,6 +297,11 @@ Переименовать фай_л (недопустимо для разделяемых типов) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file Переименовать _файл с символом diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.tr.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.tr.xlf index 97722de5624d2..40cec259169ed 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.tr.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.tr.xlf @@ -297,6 +297,11 @@ _Dosya adını değiştir (kısmi türlerde izin verilmez) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file Sembolün _dosyasını yeniden adlandır diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.zh-Hans.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.zh-Hans.xlf index 00dda0369001a..cf012dfb5cad0 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.zh-Hans.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.zh-Hans.xlf @@ -297,6 +297,11 @@ 重命名文件(不允许在分部类型上使用)(_F) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file 重命名包含符号的文件(_F) diff --git a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.zh-Hant.xlf b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.zh-Hant.xlf index b82cfb2463100..24492bfac24c7 100644 --- a/src/EditorFeatures/Core/xlf/EditorFeaturesResources.zh-Hant.xlf +++ b/src/EditorFeatures/Core/xlf/EditorFeaturesResources.zh-Hant.xlf @@ -297,6 +297,11 @@ 重新命名檔案 (在部分類型上不允許)(_F) Disabled text status for file rename + + Rename operation could not complete due to external change to workspace + Rename operation could not complete due to external change to workspace + + Rename symbol's _file 為符號的檔案重新命名(_F) From 06e9da984a55c2a5a0b53bebbf60eb353eb29c6d Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 25 May 2022 12:28:10 -0700 Subject: [PATCH 2/8] Add comment --- .../RenameTrackingTaggerProvider.RenameTrackingCommitter.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs index 05b0c75d0dda8..db8adab9545a3 100644 --- a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs +++ b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs @@ -90,6 +90,9 @@ private async Task RenameSymbolWorkerAsync(Cancellati return new RenameTrackingSolutionSet(symbol, solutionWithOriginalName, renamedSolution); } + /// + /// Returns non-null error message if renaming fails. + /// private string TryApplyChangesToWorkspace(CancellationToken cancellationToken) { _stateMachine.ThreadingContext.ThrowIfNotOnUIThread(); From a3599ebaa835573159022435484ddeca4c05c763 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 25 May 2022 12:38:06 -0700 Subject: [PATCH 3/8] message severities --- .../Core/InlineRename/InlineRenameSession.cs | 41 +++++++++++-------- ...TaggerProvider.RenameTrackingCodeAction.cs | 2 +- ...gTaggerProvider.RenameTrackingCommitter.cs | 16 ++++---- 3 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs index 4082c3ab5a4e4..b190a65e2dd82 100644 --- a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs +++ b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs @@ -797,7 +797,7 @@ private void CommitCore(IUIThreadOperationContext operationContext, bool preview operationContext.TakeOwnership(); var notificationService = _workspace.Services.GetService(); notificationService.SendNotification( - error, EditorFeaturesResources.Rename_Symbol, NotificationSeverity.Error); + error.Value.message, EditorFeaturesResources.Rename_Symbol, error.Value.severity); } }); } @@ -806,13 +806,13 @@ private void CommitCore(IUIThreadOperationContext operationContext, bool preview /// /// Returns non-null error message if renaming fails. /// - private string TryApplyRename(Solution newSolution) + private (NotificationSeverity severity, string message)? TryApplyRename(Solution newSolution) { var changes = _baseSolution.GetChanges(newSolution); var changedDocumentIDs = changes.GetProjectChanges().SelectMany(c => c.GetChangedDocuments()).ToList(); if (!_renameInfo.TryOnBeforeGlobalSymbolRenamed(_workspace, changedDocumentIDs, this.ReplacementText)) - return EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid; + return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid); using var undoTransaction = _workspace.OpenGlobalUndoTransaction(EditorFeaturesResources.Inline_Rename); var finalSolution = newSolution.Workspace.CurrentSolution; @@ -845,25 +845,30 @@ private string TryApplyRename(Solution newSolution) } if (!_workspace.TryApplyChanges(finalSolution)) - return EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace; + return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace); - // Since rename can apply file changes as well, and those file - // changes can generate new document ids, include added documents - // as well as changed documents. This also ensures that any document - // that was removed is not included - var finalChanges = _workspace.CurrentSolution.GetChanges(_baseSolution); + try + { + // Since rename can apply file changes as well, and those file + // changes can generate new document ids, include added documents + // as well as changed documents. This also ensures that any document + // that was removed is not included + var finalChanges = _workspace.CurrentSolution.GetChanges(_baseSolution); - var finalChangedIds = finalChanges - .GetProjectChanges() - .SelectMany(c => c.GetChangedDocuments().Concat(c.GetAddedDocuments())) - .ToList(); + var finalChangedIds = finalChanges + .GetProjectChanges() + .SelectMany(c => c.GetChangedDocuments().Concat(c.GetAddedDocuments())) + .ToList(); - var result = !_renameInfo.TryOnAfterGlobalSymbolRenamed(_workspace, finalChangedIds, this.ReplacementText) - ? EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated - : null; + if (!_renameInfo.TryOnAfterGlobalSymbolRenamed(_workspace, finalChangedIds, this.ReplacementText)) + return (NotificationSeverity.Warning, EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated); - undoTransaction.Commit(); - return result; + return null; + } + finally + { + undoTransaction.Commit(); + } } internal bool TryGetContainingEditableSpan(SnapshotPoint point, out SnapshotSpan editableSpan) diff --git a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs index cf3295b8502ac..736d0c46a1d1b 100644 --- a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs +++ b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCodeAction.cs @@ -116,7 +116,7 @@ public override void Apply(Workspace workspace, CancellationToken cancellationTo { var notificationService = workspace.Services.GetService(); notificationService.SendNotification( - error, EditorFeaturesResources.Rename_Symbol, NotificationSeverity.Error); + error.Value.message, EditorFeaturesResources.Rename_Symbol, error.Value.severity); } } } diff --git a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs index db8adab9545a3..8ac2f1c1382d6 100644 --- a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs +++ b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs @@ -52,7 +52,7 @@ public RenameTrackingCommitter( /// /// Returns non-null error message if renaming fails. /// - public string TryCommit(CancellationToken cancellationToken) + public (NotificationSeverity severity, string message)? TryCommit(CancellationToken cancellationToken) { _stateMachine.ThreadingContext.ThrowIfNotOnUIThread(); @@ -93,7 +93,7 @@ private async Task RenameSymbolWorkerAsync(Cancellati /// /// Returns non-null error message if renaming fails. /// - private string TryApplyChangesToWorkspace(CancellationToken cancellationToken) + private (NotificationSeverity, string)? TryApplyChangesToWorkspace(CancellationToken cancellationToken) { _stateMachine.ThreadingContext.ThrowIfNotOnUIThread(); @@ -144,7 +144,7 @@ private string TryApplyChangesToWorkspace(CancellationToken cancellationToken) // Now that the solution is back in its original state, notify third parties about // the coming rename operation. if (!_refactorNotifyServices.TryOnBeforeGlobalSymbolRenamed(workspace, changedDocuments, renameTrackingSolutionSet.Symbol, newName, throwOnFailure: false)) - return EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid; + return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_was_cancelled_or_is_not_valid); // move all changes to final solution based on the workspace's current solution, since the current solution // got updated when we reset it above. @@ -215,7 +215,7 @@ private async Task TryGetSymbolAsync(Solution solutionWithOriginalName, /// /// Returns non-null error message if renaming fails. /// - private string TryUpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, Solution newSolution, int trackingSessionId) + private (NotificationSeverity, string)? TryUpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, Solution newSolution, int trackingSessionId) { _stateMachine.ThreadingContext.ThrowIfNotOnUIThread(); @@ -231,7 +231,7 @@ private string TryUpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, localUndoTransaction.AddUndo(undoPrimitiveBefore); if (!workspace.TryApplyChanges(newSolution)) - return EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace; + return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace); return null; } @@ -248,7 +248,7 @@ private string TryUpdateWorkspaceForResetOfTypedIdentifier(Workspace workspace, /// /// Returns non-null error message if renaming fails. /// - private string TryUpdateWorkspaceForGlobalIdentifierRename( + private (NotificationSeverity, string)? TryUpdateWorkspaceForGlobalIdentifierRename( Workspace workspace, Solution newSolution, string undoName, @@ -273,10 +273,10 @@ private string TryUpdateWorkspaceForGlobalIdentifierRename( try { if (!workspace.TryApplyChanges(newSolution)) - return EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace; + return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace); if (!_refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: false)) - return EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated; + return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated); return null; } From d8d517b242580f9189f3563cc3fb11324b257582 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 25 May 2022 12:38:50 -0700 Subject: [PATCH 4/8] Restore --- src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs index b190a65e2dd82..c72ab7f9ffa78 100644 --- a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs +++ b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs @@ -861,7 +861,7 @@ private void CommitCore(IUIThreadOperationContext operationContext, bool preview .ToList(); if (!_renameInfo.TryOnAfterGlobalSymbolRenamed(_workspace, finalChangedIds, this.ReplacementText)) - return (NotificationSeverity.Warning, EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated); + return (NotificationSeverity.Information, EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated); return null; } From fcf7da2eb59b3c90d57bdeec0ee4f411d92c8a6c Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 25 May 2022 12:46:09 -0700 Subject: [PATCH 5/8] Simplify --- .../Core/InlineRename/InlineRenameSession.cs | 2 + ...gTaggerProvider.RenameTrackingCommitter.cs | 39 ++++++++++--------- 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs index c72ab7f9ffa78..35d7acf2b2d85 100644 --- a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs +++ b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs @@ -867,6 +867,8 @@ private void CommitCore(IUIThreadOperationContext operationContext, bool preview } finally { + // If we successfully update the workspace then make sure the undo transaction is committed and is + // always able to undo anything any other external listener did. undoTransaction.Commit(); } } diff --git a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs index 8ac2f1c1382d6..bc518776bb175 100644 --- a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs +++ b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs @@ -225,24 +225,22 @@ private async Task TryGetSymbolAsync(Solution solutionWithOriginalName, var undoHistory = _undoHistoryRegistry.RegisterHistory(_stateMachine.Buffer); using var localUndoTransaction = undoHistory.CreateTransaction(EditorFeaturesResources.Text_Buffer_Change); - try - { - var undoPrimitiveBefore = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: true); - localUndoTransaction.AddUndo(undoPrimitiveBefore); + var undoPrimitiveBefore = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: true); + localUndoTransaction.AddUndo(undoPrimitiveBefore); - if (!workspace.TryApplyChanges(newSolution)) - return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace); + if (!workspace.TryApplyChanges(newSolution)) + return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace); - return null; - } - finally - { - // Never resume tracking session on redo - var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); - localUndoTransaction.AddUndo(undoPrimitiveAfter); + // If we successfully update the workspace then make sure the undo transaction is committed and is + // always able to undo anything any other external listener did. - localUndoTransaction.Complete(); - } + // Never resume tracking session on redo + var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); + localUndoTransaction.AddUndo(undoPrimitiveAfter); + + localUndoTransaction.Complete(); + + return null; } /// @@ -270,18 +268,21 @@ private async Task TryGetSymbolAsync(Solution solutionWithOriginalName, var undoPrimitiveBefore = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); localUndoTransaction.AddUndo(undoPrimitiveBefore); + if (!workspace.TryApplyChanges(newSolution)) + return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace); + try { - if (!workspace.TryApplyChanges(newSolution)) - return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace); - if (!_refactorNotifyServices.TryOnAfterGlobalSymbolRenamed(workspace, changedDocuments, symbol, newName, throwOnFailure: false)) - return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated); + return (NotificationSeverity.Information, EditorFeaturesResources.Rename_operation_was_not_properly_completed_Some_file_might_not_have_been_updated); return null; } finally { + // If we successfully update the workspace then make sure the undo transaction is committed and is + // always able to undo anything any other external listener did. + // Never resume tracking session on redo var undoPrimitiveAfter = new UndoPrimitive(_stateMachine.Buffer, trackingSessionId, shouldRestoreStateOnUndo: false); localUndoTransaction.AddUndo(undoPrimitiveAfter); From 1fd30922b96caafa30cf779f7eea3aea75ec8bad Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Wed, 25 May 2022 21:04:39 -0700 Subject: [PATCH 6/8] Update src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs --- src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs index 35d7acf2b2d85..1257bf4fe2f20 100644 --- a/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs +++ b/src/EditorFeatures/Core/InlineRename/InlineRenameSession.cs @@ -867,7 +867,7 @@ private void CommitCore(IUIThreadOperationContext operationContext, bool preview } finally { - // If we successfully update the workspace then make sure the undo transaction is committed and is + // If we successfully updated the workspace then make sure the undo transaction is committed and is // always able to undo anything any other external listener did. undoTransaction.Commit(); } From 752c9f2fb41e8f1a210ea2970401efca40d79796 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Wed, 25 May 2022 21:05:35 -0700 Subject: [PATCH 7/8] Update src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs --- .../RenameTrackingTaggerProvider.RenameTrackingCommitter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs index bc518776bb175..5acc9e8015f12 100644 --- a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs +++ b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs @@ -231,7 +231,7 @@ private async Task TryGetSymbolAsync(Solution solutionWithOriginalName, if (!workspace.TryApplyChanges(newSolution)) return (NotificationSeverity.Error, EditorFeaturesResources.Rename_operation_could_not_complete_due_to_external_change_to_workspace); - // If we successfully update the workspace then make sure the undo transaction is committed and is + // If we successfully updated the workspace then make sure the undo transaction is committed and is // always able to undo anything any other external listener did. // Never resume tracking session on redo From f0ee857bb41d7376c6ee68627b59143c988e6532 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Wed, 25 May 2022 21:05:57 -0700 Subject: [PATCH 8/8] Update src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs --- .../RenameTrackingTaggerProvider.RenameTrackingCommitter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs index 5acc9e8015f12..15871df1e7025 100644 --- a/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs +++ b/src/EditorFeatures/Core/RenameTracking/RenameTrackingTaggerProvider.RenameTrackingCommitter.cs @@ -280,7 +280,7 @@ private async Task TryGetSymbolAsync(Solution solutionWithOriginalName, } finally { - // If we successfully update the workspace then make sure the undo transaction is committed and is + // If we successfully updated the workspace then make sure the undo transaction is committed and is // always able to undo anything any other external listener did. // Never resume tracking session on redo