-
-
Notifications
You must be signed in to change notification settings - Fork 405
Account for "\ No newline at end of file" in baseline diff parser #2285
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
Account for "\ No newline at end of file" in baseline diff parser #2285
Conversation
Byron
left a comment
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.
Thanks a lot, happy to merge!
Just one suggestion, but I can also merge without it.
And let's try one more thing 😅, feel free to ignore that.
gix-diff/tests/diff/blob/slider.rs
Outdated
| if line == "\\ No newline at end of file".as_bytes() { | ||
| // Do nothing. | ||
| } else { | ||
| unreachable!( | ||
| "BUG: expecting unified diff format, found line: `{}`", | ||
| line.to_str_lossy() | ||
| ) | ||
| } |
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 think this should just be an assert! with message.
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.
Makes a lot of sense, thanks for the suggestion!
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.
Pull request overview
This PR enhances the baseline diff parser in the diff slider tests to handle the \ No newline at end of file marker that appears in unified diff format. Additionally, it includes two grammar corrections in comments.
- Adds handling for
\ No newline at end of filelines in the baseline diff parser - Improves error messages to show the actual problematic line when an unexpected diff format is encountered
- Fixes grammar errors in two comment sections
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| gix-diff/tests/diff/blob/slider.rs | Adds case for handling backslash-prefixed lines in unified diff format, specifically the "\ No newline at end of file" marker, and improves error messages |
| tests/it/src/args.rs | Corrects grammar by removing duplicate verb "are" from comment |
| gix-diff/src/blob/platform.rs | Fixes subject-verb agreement and double negative in comment ("were...cannot" → "was...can") |
This is a small PR that fixes an oversight in the baseline diff parser in the diff slider tests. The parser now also parses
\ No newline at end of filelines. This is something I found while using the script to run more tests. Off the top of my head, I don’t know whether there’s more cases the parser doesn’t handle yet.