Skip to content

Support Task List in cohosting#12181

Merged
davidwengier merged 10 commits intodotnet:mainfrom
davidwengier:CohostTaskList
Sep 5, 2025
Merged

Support Task List in cohosting#12181
davidwengier merged 10 commits intodotnet:mainfrom
davidwengier:CohostTaskList

Conversation

@davidwengier
Copy link
Member

Fixes #12142 for cohosting

I just completely missed this when I did diagnostics originally, because we get them for free in the LSP editor.

@davidwengier davidwengier requested a review from a team as a code owner September 4, 2025 08:15
<MicrosoftCommonLanguageServerProtocolFrameworkPackageVersion>5.0.0-2.25421.11</MicrosoftCommonLanguageServerProtocolFrameworkPackageVersion>
<MicrosoftNetCompilersToolsetPackageVersion>5.0.0-2.25421.11</MicrosoftNetCompilersToolsetPackageVersion>
<MicrosoftVisualStudioLanguageServicesPackageVersion>5.0.0-2.25421.11</MicrosoftVisualStudioLanguageServicesPackageVersion>
<MicrosoftCodeAnalysisAnalyzersPackageVersion>5.0.0-2.25452.2</MicrosoftCodeAnalysisAnalyzersPackageVersion>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5.0.0-2.25452.2

Out of curiosity, why not have this defined in version.details.props?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you meant "why not in Version.props", and the answer is this split came from arcade, so there was one version file for people, and one for automation. See dotnet/arcade-services#4998

The irony is of course, I've only ever manually updated this file, and its xml file twin.

JsonSerializableRazorPinnedSolutionInfoWrapper solutionInfo,
JsonSerializableDocumentId documentId,
ImmutableArray<string> taskListDescriptors,
LspDiagnostic[] csharpTaskItems,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LspDiagnostic[]

nit: Why not an ImmutableArray?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No reason, thats just what the diagnostic translate service takes, presumably because it works directly on a deserialized LSP response, and I carried it through. Is it worth changing, other than for consistency?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not worth changing, just consistency and a general appreciation for using ImmutableArrays when possible

if (generatedDocument is null)
{
return [];
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice if this code didn't need to know about hintNames. Would it be possible to add an extension to TextDocument so we could just call something like:

var generatedDocument = await razorDocument.TryGetSourceGeneratedDocumentAsync(cancellationToken).ConfigureAwait(false);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I wrote diagnostics originally, I was on the fence about making this too easy, because accessing the source generated document in the devenv side of things (where this is) is generally undesirable. In fact, diagnostics and code actions are the only things that should, everything else should be on the remote side.

Having said that, an extension method that can't be called in OOP shouldn't be a problem, especially now that all of the code is written.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, its only diagnostics, so just a helper method in the base class would work here

Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@davidwengier
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@davidwengier davidwengier merged commit b6c861e into dotnet:main Sep 5, 2025
11 checks passed
@davidwengier davidwengier deleted the CohostTaskList branch September 5, 2025 07:37
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Sep 5, 2025
@akhera99 akhera99 modified the milestones: Next, 18.0 P1, 18.0 P2 Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Task list items from Razor aren't working

4 participants