-
Notifications
You must be signed in to change notification settings - Fork 49
Add a lineReader to provide two-line lookahead #56
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
Add a lineReader to provide two-line lookahead #56
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, makes sense to me to look two lines ahead.
Codecov Report
@@ Coverage Diff @@
## master #56 +/- ##
==========================================
+ Coverage 74.10% 74.59% +0.49%
==========================================
Files 4 4
Lines 421 437 +16
==========================================
+ Hits 312 326 +14
- Misses 62 63 +1
- Partials 47 48 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still feel a bit iffy about error conditions here, but its a clear improvement over previous code. Great thing is we have a great test case that is sourcegraph.com, so in practice this is fine :D
That's the spirit! I want to add more tests to this PR to check for more conditions. I also think it makes sense to add a more integration-level test here that makes sure that a huge diff can be parsed correctly. We have unit tests for the subparts, but I think a test that checks for multi-file diffs, including hunk contents, is missing. |
No description provided.