-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Implement equality semantics for async tagger tags. #64802
Changes from all commits
9deff2c
7148d87
7fac8ca
48becee
5bf93ed
5f70bc4
edc4588
67489aa
9d2e026
4bb2764
901959a
49a0641
727c00c
3edfeee
a71492e
307060f
4a33241
ea9a7e4
73d4c8b
e14f0d0
23c5089
9ad9b5d
eff6de0
a497bcd
ff54861
343d0af
5d5bdec
71d07eb
6c8c140
0e3a38c
c34bfc4
1cf970f
7e959bc
2b61433
b2d3dee
da1bef8
2f451de
0128f0e
deb93fa
53d28dc
1647dce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -114,5 +114,12 @@ protected override async Task ProduceTagsAsync( | |
context.AddTag(new TagSpan<LineSeparatorTag>(span.ToSnapshotSpan(snapshotSpan.Snapshot), tag)); | ||
} | ||
} | ||
|
||
/// <summary> | ||
/// We create and cache a separator tag to use (unless the format mapping changes). So we can just use identity | ||
/// comparisons here. | ||
/// </summary> | ||
protected override bool TagEquals(LineSeparatorTag tag1, LineSeparatorTag tag2) | ||
=> tag1 == tag2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. many simple tags just need something like this. |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// See the LICENSE file in the project root for more information. | ||
|
||
using System; | ||
using Microsoft.CodeAnalysis.Classification; | ||
using Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.QuickInfo; | ||
using Microsoft.VisualStudio.Text.Adornments; | ||
using Microsoft.VisualStudio.Text.Tagging; | ||
using Roslyn.Utilities; | ||
|
||
namespace Microsoft.CodeAnalysis.Diagnostics | ||
{ | ||
internal partial class AbstractDiagnosticsAdornmentTaggerProvider<TTag> | ||
{ | ||
protected sealed class RoslynErrorTag : ErrorTag, IEquatable<RoslynErrorTag> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. by creating our own subclass, we can hold onto the data we used to create the actual ErrorTag. we can then use that data to compare old/new tags. |
||
{ | ||
private readonly DiagnosticData _data; | ||
|
||
public RoslynErrorTag(string errorType, Workspace workspace, DiagnosticData data) | ||
: base(errorType, CreateToolTipContent(workspace, data)) | ||
{ | ||
_data = data; | ||
} | ||
|
||
private static object CreateToolTipContent(Workspace workspace, DiagnosticData diagnostic) | ||
{ | ||
Action? navigationAction = null; | ||
string? tooltip = null; | ||
if (workspace != null) | ||
{ | ||
var helpLinkUri = diagnostic.GetValidHelpLinkUri(); | ||
if (helpLinkUri != null) | ||
{ | ||
navigationAction = new QuickInfoHyperLink(workspace, helpLinkUri).NavigationAction; | ||
tooltip = diagnostic.HelpLink; | ||
} | ||
} | ||
|
||
var diagnosticIdTextRun = navigationAction is null | ||
? new ClassifiedTextRun(ClassificationTypeNames.Text, diagnostic.Id) | ||
: new ClassifiedTextRun(ClassificationTypeNames.Text, diagnostic.Id, navigationAction, tooltip); | ||
|
||
return new ContainerElement( | ||
ContainerElementStyle.Wrapped, | ||
new ClassifiedTextElement( | ||
diagnosticIdTextRun, | ||
new ClassifiedTextRun(ClassificationTypeNames.Punctuation, ":"), | ||
new ClassifiedTextRun(ClassificationTypeNames.WhiteSpace, " "), | ||
new ClassifiedTextRun(ClassificationTypeNames.Text, diagnostic.Message))); | ||
} | ||
|
||
public override bool Equals(object? obj) | ||
=> Equals(obj as RoslynErrorTag); | ||
|
||
public bool Equals(RoslynErrorTag? other) | ||
{ | ||
return other != null && | ||
this.ErrorType == other.ErrorType && | ||
this._data.GetValidHelpLinkUri() == other._data.GetValidHelpLinkUri() && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this expensive? I see there's some URI parsing but if the only input is looks to be the descriptor's HelpLinkUri can we just compare that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. my goal was to have this match whatever we do when creating the actual final tag content. but i can def look into seeing about making all of them cheaper/conssitent |
||
this._data.Id == other._data.Id && | ||
this._data.Message == other._data.Message; | ||
} | ||
|
||
public override int GetHashCode() | ||
=> throw ExceptionUtilities.Unreachable(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. while we implement Equals, we have no expectation that we will hash tags themselves. i explicitly made our tags throw so we catch if anything tries to do that. |
||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@akhera99?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what the comment says makes sense, separating the data tagging and creating UI tags from the data tags. I would be happy to change that in a follow-up PR.