-
Notifications
You must be signed in to change notification settings - Fork 229
Fix perf of multiline semantic tokens, and LF handling of same #8828
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.
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
Changes from 3 commits
69207e6
0aadee7
95ac91e
c5ae034
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,6 @@ | |||||
| using Microsoft.AspNetCore.Razor.Language.Components; | ||||||
| using Microsoft.AspNetCore.Razor.Language.Syntax; | ||||||
| using Microsoft.AspNetCore.Razor.LanguageServer.Extensions; | ||||||
| using Microsoft.AspNetCore.Razor.LanguageServer.Semantic.Models; | ||||||
| using Microsoft.CodeAnalysis.Razor.Workspaces.Extensions; | ||||||
| using Microsoft.VisualStudio.LanguageServer.Protocol; | ||||||
|
|
||||||
|
|
@@ -485,21 +484,42 @@ private void AddSemanticRange(SyntaxNode node, int semanticKind) | |||||
| var childNodes = node.ChildNodes(); | ||||||
| if (childNodes.Count == 0) | ||||||
| { | ||||||
| var content = node.GetContent(); | ||||||
| var lines = content.Split(new string[] { Environment.NewLine }, StringSplitOptions.None); | ||||||
| var charPosition = range.Start.Character; | ||||||
| for (var i = 0; i < lines.Length; i++) | ||||||
| var lineStartAbsoluteIndex = node.SpanStart - charPosition; | ||||||
| for (var i = range.Start.Line; i <= range.End.Line; i++) | ||||||
davidwengier marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| { | ||||||
| var startPosition = new Position(range.Start.Line + i, charPosition); | ||||||
| var endPosition = new Position(range.Start.Line + i, charPosition + lines[i].Length); | ||||||
| var startPosition = new Position(i, charPosition); | ||||||
|
|
||||||
| // NOTE: GetLineLength includes newlines but we don't report tokens for newlines so | ||||||
| // need to account for them. | ||||||
| var lineLength = source.Lines.GetLineLength(i); | ||||||
|
|
||||||
| // For the last line, we end where the syntax tree tells us to. For all other lines, we end at the | ||||||
| // last non-newline character | ||||||
| var endChar = i == range.End.Line | ||||||
| ? range.End.Character | ||||||
| : GetLastNonWhitespaceCharacterOffset(source, lineStartAbsoluteIndex, lineLength); | ||||||
|
|
||||||
| // Make sure we move our line start index pointer on, before potentially skipping out on the loop | ||||||
| lineStartAbsoluteIndex += lineLength; | ||||||
| charPosition = 0; | ||||||
|
|
||||||
| // Blank line. The less than check is important because we could have seen two newline characters, but | ||||||
| // be in a situation where we have an LF file with multiple blank lines, so endChar would be -1. Still | ||||||
| // a blank line though :) | ||||||
| if (endChar <= 0) | ||||||
|
||||||
| { | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| var endPosition = new Position(i, endChar); | ||||||
| var lineRange = new Range | ||||||
| { | ||||||
| Start = startPosition, | ||||||
| End = endPosition | ||||||
| }; | ||||||
| var semantic = new SemanticRange(semanticKind, lineRange, modifier: 0); | ||||||
| AddRange(semantic); | ||||||
| charPosition = 0; | ||||||
| } | ||||||
| } | ||||||
| else | ||||||
|
|
@@ -533,5 +553,21 @@ void AddRange(SemanticRange semanticRange) | |||||
| _semanticRanges.Add(semanticRange); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| static int GetLastNonWhitespaceCharacterOffset(RazorSourceDocument source, int lineStartAbsoluteIndex, int lineLength) | ||||||
| { | ||||||
| // lineStartAbsoluteIndex + lineLength is the first character of the next line, so move back one to get to the end of the line | ||||||
| lineLength--; | ||||||
|
|
||||||
| var lineEndAbsoluteIndex = lineStartAbsoluteIndex + lineLength; | ||||||
| if (lineEndAbsoluteIndex == 0) | ||||||
|
||||||
| if (lineEndAbsoluteIndex == 0) | |
| if (lineEndAbsoluteIndex == 0 || lineLength == 0) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| //line,characterPos,length,tokenType,modifier | ||
| 0 0 1 razorCommentTransition 0 | ||
| 0 1 1 razorCommentStar 0 | ||
| 0 1 4 razorComment 0 | ||
| 2 0 3 razorComment 0 | ||
| 1 0 4 razorComment 0 | ||
| 1 0 19 razorComment 0 | ||
| 1 0 3 razorComment 0 | ||
| 0 3 1 razorCommentStar 0 | ||
| 0 1 1 razorCommentTransition 0 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| //line,characterPos,length,tokenType,modifier | ||
| 0 0 1 razorCommentTransition 0 | ||
| 0 1 1 razorCommentStar 0 | ||
| 0 1 4 razorComment 0 | ||
| 2 0 3 razorComment 0 | ||
| 1 0 4 razorComment 0 | ||
| 1 0 19 razorComment 0 | ||
| 1 0 3 razorComment 0 | ||
| 0 3 1 razorCommentStar 0 | ||
| 0 1 1 razorCommentTransition 0 | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Importantly, with the previous code, this test snapshot file and the above test snapshot file were different, which is the bug this is fixing. All other test snapshots are unchanged because the rest is just perf work. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| @page "/semantic" | ||
|
|
||
| <p>Words!</p> | ||
|
|
||
| <p>@* this is | ||
| a multi | ||
| line comment *@</p> | ||
|
|
||
| <NavMenu | ||
| Collapse="true" | ||
| /> | ||
|
|
||
| <NavMenu /> | ||
|
|
||
| @*<div class="top-row pl-4 navbar navbar-dark"> | ||
| <a class="navbar-brand" href="">ComponentApp</a> | ||
| <button class="navbar-toggler" @onclick="@ToggleNavMenu"> | ||
| <span class="navbar-toggler-icon"></span> | ||
| </button> | ||
| </div> | ||
|
|
||
| <div class="@NavMenuCssClass" @onclick="@ToggleNavMenu"> | ||
| <ul class="nav flex-column"> | ||
| <li class="nav-item px-3"> | ||
| <NavLink class="nav-link" href="" Match="NavLinkMatch.All"> | ||
| <span class="oi oi-home" aria-hidden="true"></span> Home | ||
| </NavLink> | ||
| </li> | ||
| <li class="nav-item px-3"> | ||
| <NavLink class="nav-link" href="counter"> | ||
| <span class="oi oi-plus" aria-hidden="true"></span> Counter | ||
| </NavLink> | ||
| </li> | ||
| <li class="nav-item px-3"> | ||
| <NavLink class="nav-link" href="fetchdata"> | ||
| <span class="oi oi-list-rich" aria-hidden="true"></span> Fetch data | ||
| </NavLink> | ||
| </li> | ||
| </ul> | ||
| </div> | ||
|
|
||
| @functions { | ||
| bool collapseNavMenu = true; | ||
|
|
||
| string NavMenuCssClass => collapseNavMenu ? "collapse" : null; | ||
|
|
||
| void ToggleNavMenu() | ||
| { | ||
| collapseNavMenu = !collapseNavMenu; | ||
| } | ||
| }*@ |
Uh oh!
There was an error while loading. Please reload this page.