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

Fix issue classifying while performing a layout #75791

Merged
merged 15 commits into from
Nov 8, 2024
Merged
6 changes: 5 additions & 1 deletion src/EditorFeatures/Core/Copilot/CopilotTaggerProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,18 @@ protected override ITaggerEventSource CreateEventSource(ITextView textView, ITex
TaggerEventSources.OnTextChanged(subjectBuffer));
}

protected override void AddSpansToTag(ITextView? textView, ITextBuffer subjectBuffer, ref TemporaryArray<SnapshotSpan> result)
protected override bool TryAddSpansToTag(ITextView? textView, ITextBuffer subjectBuffer, ref TemporaryArray<SnapshotSpan> result)
{
this.ThreadingContext.ThrowIfNotOnUIThread();
Contract.ThrowIfNull(textView);

// We only care about the cases where we have caret.
if (textView.GetCaretPoint(subjectBuffer) is { } caret)
result.Add(new SnapshotSpan(caret, 0));

// Note: we report 'true' unconditionally here. This is because we want to ensure that we still compute 'no tags'
// in the case that we don't have a caret point, as opposed to bailing out and keeping the current set of tags.
return true;
}

protected override async Task ProduceTagsAsync(TaggerContext<ITextMarkerTag> context, DocumentSnapshotSpan spanToTag, int? caretPosition, CancellationToken cancellationToken)
Expand Down
5 changes: 3 additions & 2 deletions src/EditorFeatures/Core/InlineHints/InlineHintDataTag.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System;
using System.Linq;
using Microsoft.CodeAnalysis.InlineHints;
using Microsoft.CodeAnalysis.Text.Shared.Extensions;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Tagging;
using Roslyn.Utilities;
Expand Down Expand Up @@ -48,12 +49,12 @@ public bool Equals(InlineHintDataTag? other)
return false;

// Ensure both hints are talking about the same snapshot.
if (!_provider.SpanEquals(_snapshot, this.Hint.Span, other._snapshot, other.Hint.Span))
if (!_provider.SpanEquals(this.Hint.Span.ToSnapshotSpan(_snapshot), other.Hint.Span.ToSnapshotSpan(other._snapshot)))
return false;

if (this.Hint.ReplacementTextChange != null &&
other.Hint.ReplacementTextChange != null &&
!_provider.SpanEquals(_snapshot, this.Hint.ReplacementTextChange.Value.Span, other._snapshot, other.Hint.ReplacementTextChange.Value.Span))
!_provider.SpanEquals(this.Hint.ReplacementTextChange.Value.Span.ToSnapshotSpan(_snapshot), other.Hint.ReplacementTextChange.Value.Span.ToSnapshotSpan(other._snapshot)))
{
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,12 @@
using Microsoft.CodeAnalysis.Editor.Tagging;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.CodeAnalysis.InlineHints;
using Microsoft.CodeAnalysis.Shared.Collections;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Text.Shared.Extensions;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Microsoft.VisualStudio.Text.Tagging;
using Roslyn.Utilities;
using VSUtilities = Microsoft.VisualStudio.Utilities;

namespace Microsoft.CodeAnalysis.Editor.InlineHints;
Expand All @@ -33,10 +31,10 @@ namespace Microsoft.CodeAnalysis.Editor.InlineHints;
[VSUtilities.Name(nameof(InlineHintsDataTaggerProvider))]
[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
[method: ImportingConstructor]
internal partial class InlineHintsDataTaggerProvider(
internal sealed partial class InlineHintsDataTaggerProvider(
TaggerHost taggerHost,
[Import(AllowDefault = true)] IInlineHintKeyProcessor inlineHintKeyProcessor)
: AsynchronousViewTaggerProvider<InlineHintDataTag>(taggerHost, FeatureAttribute.InlineHints)
: AsynchronousViewportTaggerProvider<InlineHintDataTag>(taggerHost, FeatureAttribute.InlineHints)
{
private readonly IInlineHintKeyProcessor _inlineHintKeyProcessor = inlineHintKeyProcessor;

Expand Down Expand Up @@ -74,28 +72,12 @@ protected override ITaggerEventSource CreateEventSource(ITextView textView, ITex
option.Equals(InlineHintsOptionsStorage.ForCollectionExpressions)));
}

protected override void AddSpansToTag(ITextView? textView, ITextBuffer subjectBuffer, ref TemporaryArray<SnapshotSpan> result)
{
this.ThreadingContext.ThrowIfNotOnUIThread();
Contract.ThrowIfNull(textView);

// Find the visible span some 100 lines +/- what's actually in view. This way
// if the user scrolls up/down, we'll already have the results.
var visibleSpanOpt = textView.GetVisibleLinesSpan(subjectBuffer, extraLines: 100);
if (visibleSpanOpt == null)
{
// Couldn't find anything visible, just fall back to tagging all hint locations
base.AddSpansToTag(textView, subjectBuffer, ref result);
return;
}

result.Add(visibleSpanOpt.Value);
}

protected override async Task ProduceTagsAsync(
TaggerContext<InlineHintDataTag> context, DocumentSnapshotSpan documentSnapshotSpan, int? caretPosition, CancellationToken cancellationToken)
TaggerContext<InlineHintDataTag> context,
DocumentSnapshotSpan spanToTag,
CancellationToken cancellationToken)
{
var document = documentSnapshotSpan.Document;
var document = spanToTag.Document;
if (document == null)
return;

Expand All @@ -111,7 +93,7 @@ protected override async Task ProduceTagsAsync(

var options = GlobalOptions.GetInlineHintsOptions(document.Project.Language);

var snapshotSpan = documentSnapshotSpan.SnapshotSpan;
var snapshotSpan = spanToTag.SnapshotSpan;
var hints = await service.GetInlineHintsAsync(
document, snapshotSpan.Span.ToTextSpan(), options,
displayAllOverride: _inlineHintKeyProcessor?.State is true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,16 @@ protected override ITaggerEventSource CreateEventSource(ITextView textView, ITex
return textViewOpt.BufferGraph.MapDownToFirstMatch(textViewOpt.Selection.Start.Position, PointTrackingMode.Positive, b => IsSupportedContentType(b.ContentType), PositionAffinity.Successor);
}

protected override void AddSpansToTag(ITextView textViewOpt, ITextBuffer subjectBuffer, ref TemporaryArray<SnapshotSpan> result)
protected override bool TryAddSpansToTag(ITextView textViewOpt, ITextBuffer subjectBuffer, ref TemporaryArray<SnapshotSpan> result)
{
// Note: this may return no snapshot spans. We have to be resilient to that
// when processing the TaggerContext<>.SpansToTag below.
foreach (var buffer in textViewOpt.BufferGraph.GetTextBuffers(b => IsSupportedContentType(b.ContentType)))
result.Add(buffer.CurrentSnapshot.GetFullSpan());

// Fine to always return 'true' here. If we get no spans to tag, we can move ourselves to the no-tags state and
// update the editor.
return true;
}

protected override Task ProduceTagsAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,61 +320,6 @@ public static bool TryGetSurfaceBufferSpan(
return false;
}

/// <summary>
/// Returns the span of the lines in subjectBuffer that is currently visible in the provided
/// view. "extraLines" can be provided to get a span that encompasses some number of lines
/// before and after the actual visible lines.
/// </summary>
public static SnapshotSpan? GetVisibleLinesSpan(this ITextView textView, ITextBuffer subjectBuffer, int extraLines = 0)
{
// No point in continuing if the text view has been closed.
if (textView.IsClosed)
{
return null;
}

// If we're being called while the textview is actually in the middle of a layout, then
// we can't proceed. Much of the text view state is unsafe to access (and will throw).
if (textView.InLayout)
{
return null;
}

// During text view initialization the TextViewLines may be null. In that case we can't
// get an appropriate visisble span.
if (textView.TextViewLines == null)
{
return null;
}

// Determine the range of text that is visible in the view. Then map this down to the
// bufffer passed in. From that, determine the start/end line for the buffer that is in
// view.
var visibleSpan = textView.TextViewLines.FormattedSpan;
var visibleSpansInBuffer = textView.BufferGraph.MapDownToBuffer(visibleSpan, SpanTrackingMode.EdgeInclusive, subjectBuffer);
if (visibleSpansInBuffer.Count == 0)
{
return null;
}

var visibleStart = visibleSpansInBuffer.First().Start;
var visibleEnd = visibleSpansInBuffer.Last().End;

var snapshot = subjectBuffer.CurrentSnapshot;
var startLine = visibleStart.GetContainingLineNumber();
var endLine = visibleEnd.GetContainingLineNumber();

startLine = Math.Max(startLine - extraLines, 0);
endLine = Math.Min(endLine + extraLines, snapshot.LineCount - 1);

var start = snapshot.GetLineFromLineNumber(startLine).Start;
var end = snapshot.GetLineFromLineNumber(endLine).EndIncludingLineBreak;

var span = new SnapshotSpan(snapshot, Span.FromBounds(start, end));

return span;
}

/// <summary>
/// Determines if the textbuffer passed in matches the buffer for the textview.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,19 @@ private async ValueTask<VoidResult> ProcessEventChangeAsync(
// us avoid hammering the dispatcher queue with lots of work that causes contention. Additionally, use
// a no-throw awaitable so that in the common case where we cancel before, we don't throw an exception
// that can exacerbate cross process debugging scenarios.
var (isVisible, caretPosition, snapshotSpansToTag) = await _dataSource.MainThreadManager.PerformWorkOnMainThreadAsync(
var valueOpt = await _dataSource.MainThreadManager.PerformWorkOnMainThreadAsync(
GetTaggerUIData, cancellationToken).ConfigureAwait(true);

if (valueOpt is null)
{
// We failed to get the UI data we need. This can happen in cases like trying to get that during a layout pass.
// In that case, we just bail out and try again later.
this.EnqueueWork(highPriority);
return null;
}

var (isVisible, caretPosition, snapshotSpansToTag) = valueOpt.Value;

// Since we don't ever throw above, check and see if the await completed due to cancellation and do not
// proceed.
if (cancellationToken.IsCancellationRequested)
Expand Down Expand Up @@ -379,7 +389,9 @@ static async (oldTagTrees, args, cancellationToken) =>
return newTagTrees;
}

(bool isVisible, SnapshotPoint? caretPosition, OneOrMany<SnapshotSpan> spansToTag) GetTaggerUIData()
// Returns null if we we're currently in a state where we can't get the tags to span and we should bail out
// from producing tags on this turn of the crank.
(bool isVisible, SnapshotPoint? caretPosition, OneOrMany<SnapshotSpan> spansToTag)? GetTaggerUIData()
{
_dataSource.ThreadingContext.ThrowIfNotOnUIThread();

Expand All @@ -393,7 +405,8 @@ static async (oldTagTrees, args, cancellationToken) =>
var caretPosition = _dataSource.GetCaretPoint(_textView, _subjectBuffer);

using var spansToTag = TemporaryArray<SnapshotSpan>.Empty;
_dataSource.AddSpansToTag(_textView, _subjectBuffer, ref spansToTag.AsRef());
if (!_dataSource.TryAddSpansToTag(_textView, _subjectBuffer, ref spansToTag.AsRef()))
return null;

#if DEBUG
foreach (var snapshotSpan in spansToTag)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,19 +207,29 @@ private void StoreTagSource(ITextView? textView, ITextBuffer subjectBuffer, TagS
=> textView?.GetCaretPoint(subjectBuffer);

/// <summary>
/// Called by the <see cref="AbstractAsynchronousTaggerProvider{TTag}"/> infrastructure to determine
/// the set of spans that it should asynchronously tag. This will be called in response to
/// notifications from the <see cref="ITaggerEventSource"/> that something has changed, and
/// will only be called from the UI thread. The tagger infrastructure will then determine
/// the <see cref="DocumentSnapshotSpan"/>s associated with these <see cref="SnapshotSpan"/>s
/// and will asynchronously call into <see cref="ProduceTagsAsync(TaggerContext{TTag}, CancellationToken)"/> at some point in
/// the future to produce tags for these spans.
/// Called by the <see cref="AbstractAsynchronousTaggerProvider{TTag}"/> infrastructure to determine the set of
/// spans that it should asynchronously tag. This will be called in response to notifications from the <see
/// cref="ITaggerEventSource"/> that something has changed, and will only be called from the UI thread. The tagger
/// infrastructure will then determine the <see cref="DocumentSnapshotSpan"/>s associated with these <see
/// cref="SnapshotSpan"/>s and will asynchronously call into <see cref="ProduceTagsAsync(TaggerContext{TTag},
/// CancellationToken)"/> at some point in the future to produce tags for these spans.
/// </summary>
protected virtual void AddSpansToTag(
/// <returns><see langword="false"/> if spans could not be added and if tagging should abort and re-run at a later
/// point. Note: <see langword="false"/> is not equivalent to <see langword="true"/> along with no spans returned.
/// The latter means we can proceed to actual tag computation, just that since there are no applicable spans, we
/// should produce no tags whatsoever. The former means 'we cannot even proceed to tagging', 'we should preserve
/// whatever tags we current have', and 'we should rerun tagging in the future to see if we can proceed'. Examples
/// of where <see langword="false"/> should be used include if the <paramref name="textView"/> needs to be queried
/// for data, but it is in a state where it cannot respond to queries (for example, it is in the process of a
/// layout). In that case, we cannot proceed, and we do not want to clear existing tags. Instead, we want to wait
/// a short while till the next applicable time to tag.
/// </returns>
protected virtual bool TryAddSpansToTag(
ITextView? textView, ITextBuffer subjectBuffer, ref TemporaryArray<SnapshotSpan> result)
{
// For a standard tagger, the spans to tag is the span of the entire snapshot.
result.Add(subjectBuffer.CurrentSnapshot.GetFullSpan());
return true;
}

/// <summary>
Expand Down
Loading
Loading