Allow cohosting quick info to show html tag information even when on a taghelper or component tag. by ToddGrun · Pull Request #12415 · dotnet/razor

ToddGrun · 2025-10-30T02:59:58Z

Fixes https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/2556384

This change allows cohosting quick info to call into the html LSP after getting a result from the razor GetHoverAsync call. If both razor and html returned items, then the code merges the results with html items shown above the razor items (as the legacy editor does).

…a taghelper or component tag. Fixes https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/2556384 This change allows cohosting quick info to call into the html LSP after getting a result from the razor GetHoverAsync call. If both razor and html returned items, then the code merges the results with html items shown above the razor items (as the legacy editor does).

ToddGrun · 2025-10-30T03:01:58Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Hover/CohostHoverEndpoint.cs

-        }
-
-        return await _requestInvoker.MakeHtmlLspRequestAsync<TextDocumentPositionParams, LspHover>(
+        var htmlHover = await _requestInvoker.MakeHtmlLspRequestAsync<TextDocumentPositionParams, LspHover>(


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?

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.

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.

davidwengier · 2025-10-30T03:14:51Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Hover/CohostHoverEndpoint.cs

+        if (htmlHover != null
+            && htmlHover.Range == razorHover.Range
+            && razorHover is VSInternalHover razorVsInternalHover
+            && razorVsInternalHover.RawContent is ContainerElement razorContainerElement)


This will only be true in VS. In VS Code, We'd have Content that contains markdown. Though since it seems the Html server is returning markdown too, it should be easy enough to combine.

Modified VS code path to merge html/razor results with horizontal rule between:

What does the full markdown content of this look like? Or what did this look like before your change? That first tag helper line is quite chonky.

Looks like this for me right now:

Could just be a bug when we have two tag helpers (in which case, can just log a follow up issue), or perhaps VS Code's markdown is poisoning ours with an unclosed or trailing symbol. Might need to log a bug there.

It's a chunker for me without my changes too:

davidwengier · 2025-10-30T03:18:25Z

...t/Microsoft.VisualStudioCode.RazorExtension.Test/Endpoints/Shared/CohostHoverEndpointTest.cs


-        await VerifyHoverAsync(code, async (hover, document) =>
+        // This verifies Hover calls into both csharp and HTML and aggregates their results
+        const string htmlDescription = "html description";


I think this is worth reviewing after you're made the changes to the OOP service. I would have thought we'd only query html when its likely to be a real html tag, and given all Razor components must start with a capital letter, and no html elements do, I would think that's only in .cshtml files (FileKind.Legacy). So this verification code looks right, but should be in a different test. Realistically the <body> example in your screenshot is the perfect case to validate.

Thank you for this comment. I could not figure out why my attempts to use taghelpers were not working and the FileKind.Legacy was just the trick I was missing.

davidwengier · 2025-10-30T03:20:22Z

Sorry, I guess I broke this when I added mapping for tag helper start tags

2) Have remote service indicate stop handling in C# hovers 3) Make separate test for verifying merged html/razor hovers

davidwengier

My feedback is mostly nits, but I do think this probably makes a few unnecessary LSP requests to Html, in .razor files, which would be nice to remove if you agree.

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Hover/CohostHoverEndpoint.cs

src/Razor/test/Microsoft.VisualStudioCode.RazorExtension.Test/HoverAssertions.cs

davidwengier · 2025-10-30T20:49:10Z

src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Hover/RemoteHoverService.cs


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


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.

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?

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

davidwengier · 2025-10-30T20:53:54Z

src/Razor/src/Microsoft.CodeAnalysis.Razor.CohostingShared/Hover/CohostHoverEndpoint.cs

+        if (htmlHover != null
+            && htmlHover.Range == razorHover.Range
+            && razorHover is VSInternalHover razorVsInternalHover
+            && razorVsInternalHover.RawContent is ContainerElement razorContainerElement)


What does the full markdown content of this look like? Or what did this look like before your change? That first tag helper line is quite chonky.

Looks like this for me right now:

Could just be a bug when we have two tag helpers (in which case, can just log a follow up issue), or perhaps VS Code's markdown is poisoning ours with an unclosed or trailing symbol. Might need to log a bug there.

ToddGrun · 2025-10-30T22:18:39Z

@davidwengier -- I think I did most of your suggestions, let me know if I missed something or there is something else that could be changed.

davidwengier · 2025-10-30T22:23:34Z

@ToddGrun Merge at will

ToddGrun requested a review from a team as a code owner October 30, 2025 02:59

ToddGrun commented Oct 30, 2025

View reviewed changes

davidwengier reviewed Oct 30, 2025

View reviewed changes

ToddGrun added 2 commits October 30, 2025 10:00

1) Support VS code html/razor hover merging

07815eb

2) Have remote service indicate stop handling in C# hovers 3) Make separate test for verifying merged html/razor hovers

Fix VSCode hover test

2c46c25

davidwengier approved these changes Oct 30, 2025

View reviewed changes

ToddGrun merged commit ef490e4 into dotnet:main Oct 30, 2025
11 checks passed

dotnet-policy-service bot added this to the Next milestone Oct 30, 2025

ToddGrun mentioned this pull request Oct 30, 2025

Hover the mouse pointer over a tag, quick info information doesn't contain description info #12133

Closed

dotnet-maestro bot mentioned this pull request Oct 31, 2025

[release/10.0.2xx] Source code updates from dotnet/razor dotnet/dotnet#3169

Merged

ToddGrun added a commit to ToddGrun/razor that referenced this pull request Oct 31, 2025

PR feedback from dotnet#12415 which accidentally didn't get pushed

7f446f7

ToddGrun added a commit that referenced this pull request Oct 31, 2025

PR feedback from #12415 which accidentally didn't get pushed (#12424)

76dd37a

dotnet-bot mentioned this pull request Nov 1, 2025

[Automated] PRs inserted in VS build main-11131.181 #12427

Closed

dotnet-maestro bot mentioned this pull request Nov 1, 2025

[main] Source code updates from dotnet/razor dotnet/dotnet#3189

Merged

dotnet-bot mentioned this pull request Nov 1, 2025

[Automated] PRs inserted in VS build feature.debugger.main-11131.247 #12428

Closed

davidwengier mentioned this pull request Nov 3, 2025

Bump Razor to 10.0.0-preview.25552.2 dotnet/vscode-csharp#8734

Merged

davidwengier modified the milestones: Next, 18.3 Jan 6, 2026

Conversation

ToddGrun commented Oct 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwengier Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwengier commented Oct 30, 2025

Uh oh!

davidwengier left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ToddGrun commented Oct 30, 2025

Uh oh!

davidwengier commented Oct 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

davidwengier Oct 30, 2025 •

edited

Loading