From 03d790a3d4fba4087057e9d6142f4591d8032b0d Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Wed, 3 Jan 2024 13:22:54 -0800 Subject: [PATCH] There have been numerous complaints and feedback tickets around document munging in VB aspx scenarios during line commit for several years. The underlying issue is that the aspx editor resets all projection spans upon an impactful edit (eg, changing an <%= to an <%#), thus causing large diffs to be detected and seen as dirty ranges to incorporate into the commit. These large dirty ranges end up causing the line commit code behave poorly, throwing exceptions and modifying document contents in undesirable ways. (#71471) I believe the simplest and most effective way to address this is to simply remove all dirty state information upon projection spans reset. At the very worst, the user won't get line commit information when these spans weren't reset in an impactful way. I've verified that normal editing within the VB spans doesn't cause projection resets and thus continues to perform line commits as expected. Addresses https://developercommunity.visualstudio.com/t/aspx-code-corruption-with-ExternalSourc/10123117#T-ND10552491 --- .../LineCommit/CommitBufferManager.vb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/EditorFeatures/VisualBasic/LineCommit/CommitBufferManager.vb b/src/EditorFeatures/VisualBasic/LineCommit/CommitBufferManager.vb index 1665f2a0db09d..d598f19fe2842 100644 --- a/src/EditorFeatures/VisualBasic/LineCommit/CommitBufferManager.vb +++ b/src/EditorFeatures/VisualBasic/LineCommit/CommitBufferManager.vb @@ -7,6 +7,7 @@ Imports System.Threading.Tasks Imports Microsoft.CodeAnalysis.Editor.Shared.Utilities Imports Microsoft.CodeAnalysis.Text Imports Microsoft.VisualStudio.Text +Imports Microsoft.VisualStudio.Text.Projection Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.LineCommit ''' @@ -63,6 +64,11 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.LineCommit If _referencingViews = 1 Then AddHandler _buffer.Changing, AddressOf OnTextBufferChanging AddHandler _buffer.Changed, AddressOf OnTextBufferChanged + + Dim projectionBuffer = TryCast(_buffer, IProjectionBuffer) + If projectionBuffer IsNot Nothing Then + AddHandler projectionBuffer.SourceSpansChanged, AddressOf OnSourceSpansChanged + End If End If End SyncLock End Sub @@ -79,6 +85,11 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.LineCommit If _referencingViews = 0 Then RemoveHandler _buffer.Changed, AddressOf OnTextBufferChanged RemoveHandler _buffer.Changing, AddressOf OnTextBufferChanging + + Dim projectionBuffer = TryCast(_buffer, IProjectionBuffer) + If projectionBuffer IsNot Nothing Then + RemoveHandler projectionBuffer.SourceSpansChanged, AddressOf OnSourceSpansChanged + End If End If End If End SyncLock @@ -292,6 +303,14 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.LineCommit End If End Sub + Private Sub OnSourceSpansChanged(sender As Object, e As ProjectionSourceSpansChangedEventArgs) + ' DirtyState information should be purged when source spans change, as the new buffer content + ' may be unrelated to prior content. Aspx and legacy razor may generate significant differences + ' during generations (eg, converting a <%= to a <%# in aspx), causing unnecessary (and troublesome) large + ' dirty state ranges. + _dirtyState = Nothing + End Sub + Private Shared Async Function ForceComputeInternalsVisibleToAsync(document As Document, cancellationToken As CancellationToken) As Task Dim project = document.Project Dim compilation = Await project.GetCompilationAsync(cancellationToken).ConfigureAwait(False)