-
Notifications
You must be signed in to change notification settings - Fork 22
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
Failed to parse patch #5
Comments
Thanks for reporting this. I was expecting there to be some issues with that parsing logic given that there doesn't really seem to be a good specification for the actual format of a unified diff, specifically for the format that is accepted by I'll try and get a fix out soon for this, making the existence of filename headers optional. |
Just for reference,
It also seems to be able to handle parsing filename headers which are in the reverse order ('+++' line before '---' line) as well as when one of them is missing. |
@weiznich One other interesting thing about that test patch is that the hunk header doesn't actually match the hunk lines. The header indicates that this hunk corresponds to a chunk of 12 lines in the old file and 13 lines in the new file yet there's actually only 11 pre-image lines (context and '-' lines) and 12 post-image lines (context and '+' lines). I think this is probably due to missing a trailing empty line based on looking at the expected result. I'm not exactly sure what the best thing to do for this is though since the header is incorrect but |
It's interesting that |
Given that here's what I'm thinking, there's probably some benefit to wanting to parse a patch that's missing the headers so I can go ahead and finish tweaking the parsing logic to handle that (and eventually change the API so that those filenames are wrapped in Option types) and you could slightly tweak that test case so that the hunk header matches the hunk lines (making it |
That sounds great 👍 Thanks for your support |
This should be fixed with 39f787f and I'll do another release shortly. |
I've tried to integrate
diffy
intodiesel_cli
as suggested on reddit by @sgrif. While this is conceptually working it fails becausediffy
expects patches to include a file name header. As far as I understand the code this is caused by those linesIn detail the following patch file from our test suite fails to parse:
The text was updated successfully, but these errors were encountered: