Switch our copy/paste tagger over to using the same layering algorithm we use for normal classification. #72739
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| AddSyntacticSpansAsync, | ||
| AddSemanticSpansAsync, | ||
| AddEmbeddedSpansAsync, | ||
| /*unused*/ false).ConfigureAwait(false); |
There was a problem hiding this comment.
major switch is to call into this new helper to unify on this logic.
| @@ -68,166 +69,198 @@ internal sealed class TotalClassificationTaggerProvider( | |||
|
|
|||
| return new TotalClassificationAggregateTagger(syntacticTagger, semanticTagger, embeddedTagger); | |||
| } | |||
| } | |||
|
|
|||
| internal sealed class TotalClassificationAggregateTagger( | |||
There was a problem hiding this comment.
i didn't move this into its own file because i wanted the diff to be clear.
...eatures/Core/Classification/CopyPasteAndPrintingClassificationBufferTaggerProvider.Tagger.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core/Classification/TotalClassificationTaggerProvider.cs
Outdated
Show resolved
Hide resolved
...eatures/Core/Classification/CopyPasteAndPrintingClassificationBufferTaggerProvider.Tagger.cs
Outdated
Show resolved
Hide resolved
| span => classificationService.AddSemanticClassificationsAsync(document, span, options, tempClassifiedSpans, cancellationToken)).ConfigureAwait(false); | ||
| } | ||
|
|
||
| async ValueTask AddEmbeddedSpansAsync(NormalizedSnapshotSpanCollection stringLiteralSpans, SegmentedList<ITagSpan<IClassificationTag>> result, bool _) |
There was a problem hiding this comment.
💡 It seems strange to have the TArg parameter here but not actually use it to make the method static
There was a problem hiding this comment.
enh. the amount of data needed to pass in in all these cases was a lot (something like 6 parameters). My pref is to keep things simple. The mainline classification case uses statics for efficiency. But this is for copy/paste, which is a user invoked operation, so 3 extra allocs is fine :)
...eatures/Core/Classification/CopyPasteAndPrintingClassificationBufferTaggerProvider.Tagger.cs
Outdated
Show resolved
Hide resolved
| String("\""), | ||
| Punctuation.CloseParen, | ||
| Punctuation.Semicolon, | ||
| Keyword("var"), |
There was a problem hiding this comment.
so currently we do not dedupe between tags created by both the underlying syntactic tagger (innacurate, but fast) and the semantic tagger (accurate, but slow). I will do a followup to do that on top of this PR.
|
What is the expected VS version this merge will land on? Checked all previews of the 17.11 and didn't see this one |
|
I'm running 17.11.4 stable and the CommentsPlus VS 2022 extension which depends on this fix still isn't working. Do we know which version will have the fix? And if so, do we need to do anything special for it to work with older versions of .NET (like installing |
|
The fix went into 17.11 preview 1 |
|
@CyrusNajmabadi im on 17.12.0 Preview 2, and can confirm this hasnt fixed the issue with CommentsPlus |
|
I don't know what CommentsPlus is. They may need to do something on their end. Our impl should be following the contract of both IViewTaggerProvider and the IAccurateTagger |
|
@CyrusNajmabadi Ok thanks. The issue was raised a while ago and a Microsoft employee response was that this PR would fix the issue - |
This should work now. Do you own the extension that is not working? If so, I can try to work with you to figure out what is going wrong. |
|
@CyrusNajmabadi The owner of CommentsPlus is @mhoumann. It's not an urgent issue by any means, but it's a real "nice to have". Any help would be appreciated. |
|
There are more plugins affected by this issue, not just ComentPlus. I am using Supercharger: And it stopped working at the same time as all the others. They have horizontal line, colorful comments and many many more things I rely on and it all stopped with this change |
It would be useful to know why these plugins are but working. The core issue reported on vscomunity (mentioned above) was resolved. Note: using IBufferTaggerProvider and IAccurateTagger is slow and not recommended. These plugins should use IViewTaggerProvide and a normal ITagger. This is blazing fast and is the recommended way for doing anything special with the code the user is looking at. |
1 similar comment
It would be useful to know why these plugins are but working. The core issue reported on vscomunity (mentioned above) was resolved. Note: using IBufferTaggerProvider and IAccurateTagger is slow and not recommended. These plugins should use IViewTaggerProvide and a normal ITagger. This is blazing fast and is the recommended way for doing anything special with the code the user is looking at. |
No description provided.