Bleaphar/comment node investigation #3492
Conversation
|
|
||
| {name=value} | ||
| """ | ||
| ); |
There was a problem hiding this comment.
I think it may be better to create a more targeted test for the scenario being fixed.
There was a problem hiding this comment.
Namely less requests and intricacies?
There was a problem hiding this comment.
Yes, a tight test that tests exactly the single case that was not working before and that is fixed now.
| @@ -462,6 +474,11 @@ private void ConsumeCurrentTokenInto(HttpSyntaxNode node) | |||
| } | |||
| } | |||
|
|
|||
| if (!((CurrentToken?.Kind is HttpTokenKind.Word or HttpTokenKind.Punctuation) || (IsAtStartOfEmbeddedExpression()))) | |||
There was a problem hiding this comment.
Was this another bug in the url parsing? Could you please add a separate test for this?
| """ | ||
| ); | ||
|
|
||
| var requestNodes = result.SyntaxTree.RootNode.ChildNodes |
There was a problem hiding this comment.
Curious about how this is parsed. Is a child comment node added to the request separator node?
There was a problem hiding this comment.
It's parsed as tokens into the http request separator node
|
|
||
| result.GetDiagnostics().Should().BeEmpty(); | ||
|
|
||
| result.SyntaxTree.RootNode.ChildNodes.Count().Should().Be(2); |
There was a problem hiding this comment.
Can we make a more specific assertion here?
This pr treats text that is on the same line as a request separator as a part of the node and consumes those tokens into the request separator. Also, it creates an early out for the parsing of url nodes in the event that we parse an embedded expression that is the entirety of the url node. Lastly, added a test based on several requests with request separator nodes and embedded expressions.