Addition of Http Parser #3103
Merged
Addition of Http Parser #3103
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Includes some code clean up to fix up warnings (mainly around nullable annotations). Also moves reference parser implementation into a sub-folder.
Add support for parsing leading trivia.
The additions in this file make it possible to put in just a url as a valid request node
Closed
jonsequitur
reviewed
Jul 25, 2023
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/LexerTests.cs
Outdated
Show resolved
Hide resolved
jonsequitur
reviewed
Jul 25, 2023
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
jonsequitur
reviewed
Jul 25, 2023
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpRequestParser.cs
Outdated
Show resolved
Hide resolved
jonsequitur
reviewed
Jul 25, 2023
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpRequestParser.cs
Outdated
Show resolved
Hide resolved
...t.DotNet.Interactive.HttpRequest.Tests/Microsoft.DotNet.Interactive.HttpRequest.Tests.csproj
Outdated
Show resolved
Hide resolved
...t.DotNet.Interactive.HttpRequest.Tests/Microsoft.DotNet.Interactive.HttpRequest.Tests.csproj
Outdated
Show resolved
Hide resolved
jonsequitur
reviewed
Jul 25, 2023
src/Microsoft.DotNet.Interactive.HttpRequestParser/Reference/DynamicVariable.cs
Outdated
Show resolved
Hide resolved
jonsequitur
reviewed
Jul 25, 2023
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
jonsequitur
reviewed
Jul 25, 2023
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest/Microsoft.DotNet.Interactive.HttpRequest.csproj
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpTokenKind.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpLexer.cs
Outdated
Show resolved
Hide resolved
Also the addition of a lexer test
jonsequitur
reviewed
Jul 26, 2023
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
jonsequitur
reviewed
Jul 26, 2023
jonsequitur
reviewed
Jul 26, 2023
jonsequitur
reviewed
Jul 26, 2023
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpSyntaxNodeOrToken.cs
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpRequestParser.cs
Outdated
Show resolved
Hide resolved
| { | ||
| [Fact] | ||
|
|
||
| public void multiple_whitespaces_are_treated_as_a_single_token() |
There was a problem hiding this comment.
Please add similar tokenization tests for the other token kinds listed in the TODO comment above.
For example, punctuation should always be split into separate tokens, newline chars should always be split into separate tokens (except when it is \r followed by \n), and words should always be combined into a single token.
The tokenization test for newlines would also address Jon's comment above.
There was a problem hiding this comment.
Looks like the tests for whitespace, newline and punctuation are now in place. It would be great to add one for "words should always be combined into a single token" so that we round out all the kinds of tokens that the lexer produces.
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequest.Tests/ParserTests.cs
Outdated
Show resolved
Hide resolved
| Content-Type:application/json | ||
| #comment | ||
| Accept: gzip | ||
| """; |
There was a problem hiding this comment.
This test should work now, should it not? The comment will be added as leading trivia for the header.
| } | ||
|
|
||
| [Fact] | ||
| public async Task BodyAfterCommentAsync() |
There was a problem hiding this comment.
This test should work now, should it not? The comment should be added as leading trivia for the body.
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpLexer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpLexer.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Interactive.HttpRequestParser/HttpRequestParser.cs
Outdated
Show resolved
Hide resolved
shyamnamboodiripad
approved these changes
Jul 27, 2023
|
Merging |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the beginning of a package to tokenize and parse htttp requests.