Skip to content

Commit

Permalink
There have been numerous complaints and feedback tickets around docum…
Browse files Browse the repository at this point in the history
…ent 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
  • Loading branch information
ToddGrun committed Jan 3, 2024
1 parent 80310d6 commit 03d790a
Showing 1 changed file with 19 additions and 0 deletions.
19 changes: 19 additions & 0 deletions src/EditorFeatures/VisualBasic/LineCommit/CommitBufferManager.vb
Original file line number Diff line number Diff line change
Expand Up @@ -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
''' <summary>
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 03d790a

Please sign in to comment.