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

Fix diff generation for lines without newline #550

Merged
merged 1 commit into from
Aug 12, 2020

Conversation

ad1217
Copy link
Contributor

@ad1217 ad1217 commented Aug 12, 2020

Rather than keeping the line endings before passing to diff, then joining the resulting lines with '', strip the line endings and join
with '\n'. This ensures a that each output line has exactly one trailing newline.

Possible issues:

  • This will prevent detection of a difference in line endings.
  • Joining with os.linesep might be better for Windows, but only if it is going to a text file, and I think most Windows stuff handles lines terminated with just LF fine now.

Rather than keeping the line endings before passing to diff, then
joining the resulting lines with '', strip the line endings and join
with '\n'. This ensures a that each output line has exactly one
trailing newline.
@polyzen
Copy link
Contributor

polyzen commented Aug 12, 2020

Before:

@@ -1,3 +1,3 @@
-1.5.8
-2020-07-01T04:51:11
-707ed1bdffd6b8bec2d74dca36cc2665081c8db8590241bf6a72a3378928dd65+1.5.9
+2020-07-23T04:58:02
+6f3e18befd0f26118c6c6ddf39e2ef18366ff9501cbcbf664ffde669e3c2d132

After:

@@ -1,3 +1,3 @@
-1.5.8
-2020-07-01T04:51:11
-707ed1bdffd6b8bec2d74dca36cc2665081c8db8590241bf6a72a3378928dd65
+1.5.9
+2020-07-23T04:58:02
+6f3e18befd0f26118c6c6ddf39e2ef18366ff9501cbcbf664ffde669e3c2d132

With this filter:
https://github.com/polyzen/dotfiles/blob/3b83c549d2e497098564ad7fdbb44c5050379231/misc/.config/urlwatch/hooks.py#L52

@thp thp merged commit 6905d1a into thp:master Aug 12, 2020
@thp
Copy link
Owner

thp commented Aug 12, 2020

This is a good change, I've ran into the issue as described by @polyzen already a few times, good that this is now fixed. I think most people don't care about newline changes, and if they do, they can always use an external diff_tool or the hexdump filter.

Change documented here: c3b5c24

@ad1217 ad1217 deleted the fix-unified-diff branch August 12, 2020 20:05
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

Successfully merging this pull request may close these issues.

3 participants