Fails on empty comments without 4 -" #618
Conversation
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #618 +/- ##
==========================================
- Coverage 64.67% 64.66% -0.02%
==========================================
Files 36 36
Lines 16998 16998
==========================================
- Hits 10994 10991 -3
- Misses 6004 6007 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Actually, fix of this issue not so simple. |
Indeed. Thank you for spotting this! I have found a cleaner way to fix the bug that behaves properly in this case. |
|
Thank you! I have updated the tests following your review! |
| @@ -117,7 +117,7 @@ macro_rules! impl_buffered_source { | |||
| // somewhere sane rather than at the EOF | |||
| Ok(n) if n.is_empty() => return Err(bang_type.to_err()), | |||
| Ok(available) => { | |||
| if let Some((consumed, used)) = bang_type.parse(buf, available) { | |||
| if let Some((consumed, used)) = bang_type.parse(&buf[start..], available) { | |||
There was a problem hiding this comment.
I'm trying to understand, how the fix worked. I feel it will be worth if this line will have a comment, why we need to take bytes only from start. Maybe with an examples of buf content for better understanding. If you known, what happens here, could you add such a comment? If not, I'll investigate that myself. Because tests are passed I assume that the fix is correct, but I want to know why it is correct
There was a problem hiding this comment.
I think, I understood. We should parse only newly read data from the buf. This is correct, because we in the function that parses the whole comment, so any data that exists in the buffer before is not relevant here and should be skipped.
There was a problem hiding this comment.
Yes, exactly. I have added the comment. Not doing so was making some checks like the one on this line to be incorrect.
…not cleared before Closes tafia#604
|
Thanks! I slightly changed tests to more precisely test what an error we returns and changed Changelog description to be less technically specific and more user specific |
The parser was crashing because of bad slice bounds
Closes #604