Skip to content
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

Eliminate linedelimiters from structured diff hunks #434

Open
ExplodingCabbage opened this issue Dec 20, 2023 · 1 comment
Open

Eliminate linedelimiters from structured diff hunks #434

ExplodingCabbage opened this issue Dec 20, 2023 · 1 comment
Assignees

Comments

@ExplodingCabbage
Copy link
Collaborator

It's confusing and shouldn't be necessary.

Steps to this:

  1. Figure out precisely what the scenario was in Cannot apply patch because my file delimiter is "/r/n" instead of "/n" #141 that was tripping up @soulbeing. (Did the diff file use \r\n endings, or just the file being patched? Was this a case of a diff generated from a file with \n endings being applied to a file with \r\n endings? How do Unix diff/patch tools handle this case?)
  2. Somehow make applyPatch magically handle that scenario, without needing the linedelimiters property on the patch

I confess I'm a bit confused about why that property would ever be necessary, since if you're working purely with files with \r\n endings, creating and applying diffs using tools that assume lines end with \n should work fine - the tools will just treat the \r at the end of every line as an ordinary character, but that doesn't prevent them from working any more than if the files you were diffing had a letter x or something at the end of every line.

So I think the scenario that motivated that patch must in some way have involved working with a mixture of \n endings and \r\n endings? Like, it lets us apply a patch generated on a file with \r\n endings to one with \n endings, maybe?

I should confirm that that is so, and if it is, add tests for it!

@ExplodingCabbage ExplodingCabbage self-assigned this Dec 20, 2023
@ExplodingCabbage
Copy link
Collaborator Author

(#435 is a start towards this and fixes an outright bug, but I think we can probably go further.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant