Keep the semantic colors of open documents on screen - #20450
Open
xperiandri wants to merge 2 commits into
Open
Conversation
3 tasks
Contributor
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
xperiandri
force-pushed
the
fix-20445-semantic-classification
branch
from
September 4, 2026 17:30
a44eabe to
a048f57
Compare
3 tasks
github-actions
Bot
added
the
AI-Tooling-Check-Scanned-Clean
xperiandri
changed the title
xperiandri
commented
Sep 4, 2026
Every entry point in WorkspaceExtensions signalled "the project has no options yet" by raising an OperationCanceledException, which the callers then had to tell apart from a real cancellation. They cannot, so they treat both as "cancelled" and return an empty result. For a service Roslyn asks repeatedly while a solution loads, that is a wrong answer, not a missing one. Add `Try` siblings that return ValueNone instead, and keep the raising members as thin wrappers so the call sites that have not moved over still get the same exception with the same message. The four-tuple those members hand out becomes a named record, which is also what ProjectCache stores, so a cache hit hands back the instance it holds rather than rebuilding a tuple. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Splitting the semantic classification cache in two (dotnet#15954) left the open-document branch reading the opened-documents cache and writing the unopened one, so the opened cache was never populated and every request for an open file re-ran the checker. What it wrote was also only the requested span, keyed by document and text version, so a second request for another span of the same version (scrolling, a split view, Roslyn asking around the viewport) would have hit that entry and sliced nothing out of it. Cache the classification of the whole file instead, and slice it per request, the way the unopened-documents branch already does. A check that cannot complete - the project is loading or reloading, or the check was superseded - must not answer with no classifications either: Roslyn replaces the tags of a span with whatever comes back, so an empty answer strips the colours the user is looking at, while a cancellation carrying Roslyn's own token leaves them alone. Keep the last classification per document and re-emit it for those requests, but only while the text it was computed from is still the current one, as its spans would otherwise land on the wrong characters. `ifCanceledThen` replaces `ifCanceledReturn ()` for the same reason: the checker relabels its internal cancellations with the caller's token, and those must produce the last known colours rather than none. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
xperiandri
force-pushed
the
fix-20445-semantic-classification
branch
from
September 4, 2026 22:10
a048f57 to
6e5d1f9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Semantic colours of an open F# file appear for a moment and then disappear. Roslyn replaces the semantic tags of a span with whatever
IFSharpClassificationService.AddSemanticClassificationsAsyncreturns, so an empty answer that completes successfully strips the colours the user is looking at. Only anOperationCanceledExceptioncarrying Roslyn's own token leaves the previous tags alone.Three things in the F# service produce such empty answers.
The open-document cache was never populated. Splitting the cache in two (#15954) left the open-document branch reading
openedDocumentsSemanticClassificationCacheand writingunopenedDocumentsSemanticClassificationCache, so every request for an open file re-ran the checker.What was cached could not answer the next request. The entry is keyed by document and text version, but it held the classification of the requested span only. A second request for another span of the same version — scrolling, a split view, Roslyn asking around the viewport — would have hit that entry and sliced nothing out of it. The classification of the whole file is cached now and sliced per request, the way the unopened-documents branch already does it.
"I cannot answer" was encoded as a cancellation.
WorkspaceExtensionsraisedOperationCanceledExceptionfor "this project has no options yet" (a project loading or reloading, a miscellaneous file, any failure in the options agent) and for an aborted check;ifCanceledReturn ()then turned those, and every checker-internal cancellation that theCancellableTaskadapter relabels with the caller's token, into an empty success. Those entry points now haveTrysiblings returningvoption, and the classification service re-emits the last classification it had for the document instead of nothing. It does so only while the text that classification was computed from is still the current one — its spans name positions, so against edited text they would colour the wrong characters. A cancellation carrying Roslyn's token still propagates untouched.The raising members stay as thin wrappers over the
Tryones, with the same messages, so the call sites that have not moved over behave exactly as before; migrating them is follow-up work. The four-tuple they hand out became a named record, which is also whatProjectCachestores.Fixes #20445, including the secondary observation about
ifCanceledReturnin that issue.Left for later, deliberately: concurrent requests for the same version each compute their own whole-file lookup (no in-flight coalescing), and the last-known-good entry of a document is kept until its project goes away.
Checklist