Brace matcher for preprocessor directives #7327
Conversation
C# Implementation of a DirectiveTrivia Matcher as an IBraceMatcher which enables Ctrl + ] navigation on preprocessor directives like #If , #region etc.
Tests for C# brace matcher on directive trivia syntax
Visual Basic implementation of a DirectiveTrivia Matcher which enables Ctrl + ] nagivation on preprocessor directives like #if, #region etc.
VB tests for brace matching on directive trivia syntax.
|
@dotnet/roslyn-ide |
| }"; | ||
|
|
||
| await TestAsync(code, expected); | ||
| } |
There was a problem hiding this comment.
Consider: Tests for interleaved directives.
|
👍 |
Add tests for interleaved directives
| var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); | ||
| var token = root.FindToken(position, findInsideTrivia: true); | ||
|
|
||
| var directive = token.Parent as DirectiveTriviaSyntax; |
There was a problem hiding this comment.
Will the parent always be DirectiveTrivia? What if i'm in something like #if Some || Thing && Else
I think you want to know if an ancestor is DirectiveTrivia instead.
There was a problem hiding this comment.
It works only when the caret is on the hashtoken or the keyword (#if$$ $$#else) and not on identifiers ($$CHK) or related trailing trivia ($$MyRegion).
In that it is consistent with how Highlight references performs navigation on these and with brace matching itself in VS2013. I'm open to tweaking this if you feel we should.
There was a problem hiding this comment.
agreed offline to keep the existing design as such.
Moving common code to AbstractDirectiveTriviaBraceMatcher
Rename files.
|
@CyrusNajmabadi - incorporated all the feedback, please take a look. |
| { | ||
| internal abstract List<TDirectiveTriviaSyntax> GetMatchingConditionalDirectives(TDirectiveTriviaSyntax directive, CancellationToken cancellationToken); | ||
| internal abstract TDirectiveTriviaSyntax GetMatchingDirective(TDirectiveTriviaSyntax directive, CancellationToken cancellationToken); | ||
| internal abstract TextSpan GetSpansForTagging(TDirectiveTriviaSyntax directive); |
|
👍 when you make the name change and you verify that the left/right thing isn't an issue. Also, make sure you have a test where you're on the end directive of a conditional. |
|
Mac is back up |
Rename method per PR feedback
|
@CyrusNajmabadi : thanks, made the name change, left/right thing isn't an issue and already have a test for caret at end if, it wraps over to if directive as expected. Merging. |
|
The test failure at prtest/win/dbg/unit32 is unrelated to this change. It is a known issue #6358 . retrying the job. |
|
@dotnet-bot retest prtest/win/dbg/unit32 please |
Brace matcher for preprocessor directives
Fixes #7120
Implements an IBraceMatcher that matches directive trivia syntax for C# and VB.