Skip to content

Allow cohosting quick info to show html tag information even when on a taghelper or component tag. #12415

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.

Sign up for GitHub

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

Merged
ToddGrun merged 3 commits into from
Oct 30, 2025
Merged

Allow cohosting quick info to show html tag information even when on a taghelper or component tag. #12415

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2ec3f12
Allow cohosting quick info to show html tag information even when on …
ToddGrun Oct 30, 2025
07815eb
1) Support VS code html/razor hover merging
ToddGrun Oct 30, 2025
2c46c25
Fix VSCode hover test
ToddGrun Oct 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 68 additions & 9 deletions src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Hover/CohostHoverEndpoint.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.CodeAnalysis.ExternalAccess.Razor.Features;
using Microsoft.CodeAnalysis.Razor.Cohost;
using Microsoft.CodeAnalysis.Razor.Remote;
using Roslyn.Text.Adornments;

namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;

Expand Down Expand Up @@ -54,29 +55,87 @@ public ImmutableArray<Registration> GetRegistrations(VSInternalClientCapabilitie
{
var position = LspFactory.CreatePosition(request.Position.ToLinePosition());

var response = await _remoteServiceInvoker
var razorResponse = await _remoteServiceInvoker
.TryInvokeAsync<IRemoteHoverService, RemoteResponse<LspHover?>>(
razorDocument.Project.Solution,
(service, solutionInfo, cancellationToken) =>
service.GetHoverAsync(solutionInfo, razorDocument.Id, position, cancellationToken),
cancellationToken)
.ConfigureAwait(false);

if (response.Result is LspHover hover)
if (razorResponse.StopHandling)
{
return hover;
return razorResponse.Result;
}

if (response.StopHandling)
{
return null;
}

return await _requestInvoker.MakeHtmlLspRequestAsync<TextDocumentPositionParams, LspHover>(
var htmlHover = await _requestInvoker.MakeHtmlLspRequestAsync<TextDocumentPositionParams, LspHover>(
Copy link
Contributor Author

@ToddGrun ToddGrun Oct 30, 2025

Choose a reason for hiding this comment

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

MakeHtmlLspRequestAsync

I'd love some guidance on changing this to not make the html call if we aren't in an html context. It looks like this information isn't at my fingertips, and I noticed some other cohosting calls across to the remote service end up returning a flag indicating the language that is being operated in. Is that the general mechanism suggested to filter work done at this level by language?

Copy link
Member

@davidwengier davidwengier Oct 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the endpoints run in devenv, and we have no knowledge of the Razor syntax tree or anything that lets us determine what language is at a position there, so we have to jump to OOP to get that info. In this case, I would imagine the OOP call above would change to return a custom type that carries the hover info, and whether or not to call Html and merge the results.

Copy link
Contributor Author

@ToddGrun ToddGrun Oct 30, 2025

Choose a reason for hiding this comment

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

That's what I thought when I asked for guidance. I think the small change I made in commit 2 to the stopHandling determination in the remote process prevents most of the unnecessary html lsp requests.

razorDocument,
Methods.TextDocumentHoverName,
request,
cancellationToken).ConfigureAwait(false);

return MergeHtmlAndRazorHoverResponses(razorResponse.Result, htmlHover);
}

private static LspHover? MergeHtmlAndRazorHoverResponses(LspHover? razorHover, LspHover? htmlHover)
{
if (razorHover is null)
{
return htmlHover;
}

if (htmlHover is null
|| htmlHover.Range != razorHover.Range)
{
return razorHover;
}

var htmlStringResponse = htmlHover.Contents.Match(
static s => s,
static markedString => null,
static stringOrMarkedStringArray => null,
static markupContent => markupContent.Value
);

if (htmlStringResponse is not null)
{
// This logic is to prepend HTML hover content to the razor hover content if both exist.
// The razor content comes through as a ContainerElement, while the html content comes
// through as MarkupContent. We need to extract the html content and insert it at the
// start of the combined ContainerElement.
if (razorHover is VSInternalHover razorVsInternalHover
&& razorVsInternalHover.RawContent is ContainerElement razorContainerElement)
{
var htmlStringClassifiedTextElement = ClassifiedTextElement.CreatePlainText(htmlStringResponse);
var verticalSpacingTextElement = ClassifiedTextElement.CreatePlainText(string.Empty);
var htmlContainerElement = new ContainerElement(
ContainerElementStyle.Stacked,
[htmlStringClassifiedTextElement, verticalSpacingTextElement]);

// Modify the existing hover's RawContent to prepend the HTML content.
razorVsInternalHover.RawContent = new ContainerElement(razorContainerElement.Style, [htmlContainerElement, .. razorContainerElement.Elements]);
}
else
{
var razorStringResponse = razorHover.Contents.Match(
static s => s,
static markedString => null,
static stringOrMarkedStringArray => null,
static markupContent => markupContent.Value
);

if (razorStringResponse is not null)
{
razorHover.Contents = new MarkupContent()
{
Kind = MarkupKind.Markdown,
Value = htmlStringResponse + "\n\n---\n\n" + razorStringResponse
};
}
}
}

return razorHover;
}

internal TestAccessor GetTestAccessor() => new(this);
Expand Down
5 changes: 3 additions & 2 deletions src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Hover/RemoteHoverService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ protected override IRemoteHoverService CreateService(in ServiceArgs args)
}
}

return Results(csharpHover);
// As there is a C# hover, stop further handling.
return new RemoteResponse<Hover?>(StopHandling: true, Result: csharpHover);
Copy link
Member

@davidwengier davidwengier Oct 30, 2025

Choose a reason for hiding this comment

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

I might be missing it, but I was expecting StopHandling to be used when we product a Razor response at times too. eg, when hovering over <PageTitle> in a .razor file, we don't need to make a Html request. In fact, I can't think of any scenario where we'd want to call the Html server if we're in a .razor file and we were able to produce a hover response ourselves. It's really only MVC that has Razor concepts apply to regular Html elements or attributes.

That check could even be done in the endpoint, by checking the file type in the endpoint (there is a FileTypes.GetFromExtension method or something) and if its a .razor, then any response from OOP means don't call Html.

Copy link
Contributor Author

@ToddGrun ToddGrun Oct 30, 2025

Choose a reason for hiding this comment

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

I'm probably missing something, but I see the if condition on line 65 hit when hovering over PageTitle, due to the remapping code in GetPositionInfo. Or is there another case you are concerned about that isn't true for line 65?

Copy link
Member

@davidwengier davidwengier Oct 30, 2025

Choose a reason for hiding this comment

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

Ohh of course, because thats actually a C# hover. Okay, cool. You lost the fight over who missed something :P

}

if (positionInfo.LanguageKind is not (RazorLanguageKind.Html or RazorLanguageKind.Razor))
Expand Down Expand Up @@ -152,7 +153,7 @@ protected override IRemoteHoverService CreateService(in ServiceArgs args)
/// <remarks>
/// Once Razor moves wholly over to Roslyn.LanguageServer.Protocol, this method can be removed.
/// </remarks>
private Hover ConvertHover(Hover hover)
private static Hover ConvertHover(Hover hover)
{
// Note: Razor only ever produces a Hover with MarkupContent or a VSInternalHover with RawContents.
// Both variants return a Range.
Expand Down
61 changes: 58 additions & 3 deletions ...icrosoft.VisualStudioCode.RazorExtension.Test/Endpoints/Shared/CohostHoverEndpointTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Test.Common;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
Expand Down Expand Up @@ -73,6 +74,57 @@ public async Task Html()
await VerifyHoverAsync(code, htmlResponse, h => Assert.Same(htmlResponse, h));
}

[Fact]
public async Task Html_TagHelper()
{
TestCode code = """
<[|bo$$dy|]></body>
""";

// This verifies Hover calls into both razor and HTML, aggregating their results
const string BodyDescription = "body description";
var htmlResponse = new VSInternalHover
{
Range = new LspRange()
{
Start = new Position(0, 1),
End = new Position(0, "<body".Length),
},
Contents = new MarkupContent()
{
Kind = MarkupKind.Markdown,
Value = BodyDescription,
}
};

await VerifyHoverAsync(code, RazorFileKind.Legacy, htmlResponse, async (hover, document) =>
{
await VerifyRangeAsync(hover, code.Span, document);

hover.VerifyContents(
Container(
Container(
ClassifiedText(
Text(BodyDescription)),
ClassifiedText(
Text(string.Empty))),
Container(
Image,
ClassifiedText(
Text("Microsoft"),
Punctuation("."),
Text("AspNetCore"),
Punctuation("."),
Text("Mvc"),
Punctuation("."),
Text("Razor"),
Punctuation("."),
Text("TagHelpers"),
Punctuation("."),
Type("BodyTagHelper")))));
});
}

[Fact]
public async Task Html_EndTag()
{
Expand Down Expand Up @@ -310,10 +362,13 @@ await VerifyHoverAsync(code, async (hover, document) =>
});
}

private async Task VerifyHoverAsync(TestCode input, Func<Hover, TextDocument, Task> verifyHover)
private Task VerifyHoverAsync(TestCode input, Func<Hover, TextDocument, Task> verifyHover)
=> VerifyHoverAsync(input, fileKind: null, htmlResponse: null, verifyHover);

private async Task VerifyHoverAsync(TestCode input, RazorFileKind? fileKind, Hover? htmlResponse, Func<Hover, TextDocument, Task> verifyHover)
{
var document = CreateProjectAndRazorDocument(input.Text);
var result = await GetHoverResultAsync(document, input);
var document = CreateProjectAndRazorDocument(input.Text, fileKind);
var result = await GetHoverResultAsync(document, input, htmlResponse);

Assert.NotNull(result);
await verifyHover(result, document);
Expand Down
12 changes: 9 additions & 3 deletions src/Razor/test/Microsoft.VisualStudioCode.RazorExtension.Test/HoverAssertions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System.Collections.Immutable;
using Roslyn.Test.Utilities;
using Xunit;

namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;

Expand All @@ -12,8 +11,15 @@ internal static class HoverAssertions
public static void VerifyContents(this LspHover hover, object expected)
{
var markup = hover.Contents.Fourth;
Assert.Equal(MarkupKind.PlainText, markup.Kind);
AssertEx.EqualOrDiff(expected.ToString(), markup.Value.TrimEnd('\r', '\n'));

var actual = markup.Value.TrimEnd('\r', '\n');
if (markup.Kind == MarkupKind.Markdown)
{
// Remove any horizontal rules we may have added to separate HTML and Razor content
actual = actual.Replace("\n\n---\n\n", string.Empty);
}

AssertEx.EqualOrDiff(expected.ToString(), actual);
}

// Our VS Code test only produce plain text hover content, so these methods are complete overkill,
Expand Down