Add syntax highlighting of ignored directives #78458
Conversation
jjonescz
added
the
Feature - Run File
ghost
added
Area-IDE
untriaged
| AddClassification(node.ColonToken, ClassificationTypeNames.PreprocessorKeyword); | ||
|
|
||
| // The first part (separated by whitespace) of content is a "keyword", e.g., 'sdk' in '#:sdk Test'. | ||
| if (TryFindWhitespace(node.Content.ValueText, out var firstSpaceIndex)) |
There was a problem hiding this comment.
| if (TryFindWhitespace(node.Content.ValueText, out var firstSpaceIndex)) | |
| var firstSpaceIndex = node.Content.Text.IndexOf(" "); |
Don't use 'valueText'. it is interpreted characters, not the actual characters in the file.
There was a problem hiding this comment.
Thanks, there is so many of these - ValueText, Text, ToString, ToFullString, ...
There was a problem hiding this comment.
yup. never use 'Value' if actually trying to get real positions infiles :)
|
|
||
| static bool TryFindWhitespace(string text, out int index) | ||
| { | ||
| for (var i = 0; i < text.Length; i++) | ||
| { | ||
| if (SyntaxFacts.IsWhitespace(text[i])) | ||
| { | ||
| index = i; | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| index = -1; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
| static bool TryFindWhitespace(string text, out int index) | |
| { | |
| for (var i = 0; i < text.Length; i++) | |
| { | |
| if (SyntaxFacts.IsWhitespace(text[i])) | |
| { | |
| index = i; | |
| return true; | |
| } | |
| } | |
| index = -1; | |
| return false; | |
| } |
There was a problem hiding this comment.
Are you suggesting to search just for space? Then having #:sdk\tTest wouldn't be colorized properly.
There was a problem hiding this comment.
I'm totally ok with that.
There was a problem hiding this comment.
or do IndexOfAny([' ', '\t']) if you do want to support that. i'm fine with both.
| if (TryFindWhitespace(node.Content.ValueText, out var firstSpaceIndex)) | ||
| { | ||
| var keywordSpan = new TextSpan(node.Content.SpanStart, firstSpaceIndex); | ||
| var stringLiteralSpan = new TextSpan(node.Content.SpanStart + firstSpaceIndex + 1, node.Content.Span.Length - firstSpaceIndex - 1); |
There was a problem hiding this comment.
| var stringLiteralSpan = new TextSpan(node.Content.SpanStart + firstSpaceIndex + 1, node.Content.Span.Length - firstSpaceIndex - 1); | |
| var stringLiteralSpan = TextSpan.FromBound(node.Content.SpanStart + firstSpaceIndex + 1, node.Content.FullSpan.End); |
There was a problem hiding this comment.
adding the +1 doesn't seem necessary. you're doing this to avoid classifying the spce. but you could have #:sdk whatever, and it would still classify the spaces after the first one. So just split on the space index and make the math easier :)
* upstream/main: (415 commits) Use lazy initialization for members in CodeFixService (dotnet#78484) Targeted perf changes to CommandLineParser (dotnet#78446) Exclude VS.ExternalAPIs.Roslyn.Package from source-build Add syntax highlighting of ignored directives (dotnet#78458) Fix unexpected conditional state in nullable analysis of conditional access (dotnet#78439) Update RoslynAnalyzers to use Roslyns version Fix source-build by renaming Assets folder to assets Unlist M.CA.AnalyzerUtilities as an expected DevDivInsertion dependency Use a project reference for M.CA.AnalyzerUtilities Rectify status of dictionary expressions (dotnet#78497) Remove unused field Remove unused field Remove using Fix check Update eng/config/PublishData.json Make internal Remove now unused methods from IWorkspaceProjectContext (dotnet#78479) Reduce allocations during SourceGeneration (dotnet#78403) Update Roslyn.sln Merge EditorFeatures.Wpf entirely into EditorFeatures ...
| if (node.Content.Text.IndexOfAny([' ', '\t']) is > 0 and var firstSpaceIndex) | ||
| { | ||
| var keywordSpan = new TextSpan(node.Content.SpanStart, firstSpaceIndex); | ||
| var stringLiteralSpan = TextSpan.FromBounds(node.Content.SpanStart + firstSpaceIndex, node.Content.FullSpan.End); |
There was a problem hiding this comment.
Was there a reason we're classifying the whitespace character?
There was a problem hiding this comment.
Primarily simplicity. We didn't see any harm in it. The entire line is free form anyways. So there might be many spaces. Easiest to just classify these chunks.
Thoughts?
No description provided.