Skip to content

Conversation

@spoorcc
Copy link
Contributor

@spoorcc spoorcc commented Jan 12, 2026

fix #945

Summary by CodeRabbit

  • Bug Fixes

    • Fixed extra newlines in generated patches for newly created files, improving patch formatting and consistency for Git and SVN outputs.
  • Tests

    • Added unit tests validating patch generation for new files to ensure correct headers and content hunks.
  • Changelog

    • Added an unreleased changelog entry noting the patch formatting fix.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 12, 2026

Walkthrough

Renames unified_diff_new_file to a private _unified_diff_new_file, enforces lineterm="\n" when generating diffs, and updates create_svn_patch_for_new_file and create_git_patch_for_new_file to use it and to ensure patch header lines end with newline. Adds tests and a changelog entry.

Changes

Cohort / File(s) Summary
Changelog
CHANGELOG.rst
Added unreleased entry: "Fix extra newlines in patch for new files (#945)".
Patch generation
dfetch/vcs/patch.py
Renamed unified_diff_new_file -> _unified_diff_new_file (private). _unified_diff_new_file reads files with encoding="utf-8", errors="replace" and uses lineterm="\n". Updated create_git_patch_for_new_file and create_svn_patch_for_new_file to call the private helper and ensure all diff header lines end with newline (added explicit newline terminators, SVN patch gets Index: <path>\n).
Tests
tests/test_patch.py
Added two unit tests that create a temp file and assert exact output of create_git_patch_for_new_file and create_svn_patch_for_new_file against expected multi-line patch strings.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main fix: removing extra newlines in patches for new files, directly addressing issue #945.
Linked Issues check ✅ Passed The PR successfully implements the fix for #945 by refactoring patch generation to eliminate extra newlines in new file patches through lineterm specification and header formatting.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the extra newline issue in patches for new files; the changelog entry, private function refactoring, and new tests are all directly related to the #945 objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f128e81 and ec1b61d.

📒 Files selected for processing (3)
  • CHANGELOG.rst
  • dfetch/vcs/patch.py
  • tests/test_patch.py
✅ Files skipped from review due to trivial changes (1)
  • tests/test_patch.py
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-09T22:59:31.863Z
Learnt from: spoorcc
Repo: dfetch-org/dfetch PR: 942
File: dfetch/vcs/patch.py:62-81
Timestamp: 2026-01-09T22:59:31.863Z
Learning: In dfetch/vcs/patch.py, patch_ng.fromstring() expects bytes (Python 2-style binary string), not a str. The pattern patch_ng.decode_text(...).encode("utf-8") is correct for converting various encodings to UTF-8 bytes before parsing.

Applied to files:

  • dfetch/vcs/patch.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: Publish Python distribution 📦 to TestPyPI
  • GitHub Check: build / build (ubuntu-latest)
  • GitHub Check: build / build (macos-latest)
  • GitHub Check: build / build (windows-latest)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: run / run (windows-latest, 3.12)
  • GitHub Check: run / run (windows-latest, 3.11)
  • GitHub Check: run / run (windows-latest, 3.13)
  • GitHub Check: run / run (windows-latest, 3.10)
  • GitHub Check: run / run (macos-latest, 3.12)
  • GitHub Check: run / run (windows-latest, 3.9)
  • GitHub Check: test / test
  • GitHub Check: run / test-cygwin
  • GitHub Check: DevContainer Build & Test
🔇 Additional comments (4)
CHANGELOG.rst (1)

13-13: LGTM!

The changelog entry is correctly placed and follows the established format.

dfetch/vcs/patch.py (3)

83-88: LGTM!

The SVN patch header construction now explicitly includes \n terminators, ensuring consistent formatting. The use of "".join() correctly concatenates the header components with the diff lines.


91-107: LGTM!

The Git patch header lines now have explicit \n terminators for consistency. The structure correctly builds the complete patch with diff header, file mode, index, and diff content.


110-119: LGTM!

The function is correctly made private with the underscore prefix. The lineterm="\n" parameter ensures consistent line endings in the diff output. Since readlines() preserves trailing newlines and unified_diff prepends +/-/ to content lines without adding redundant terminators to lines that already end with \n, this produces correctly formatted diffs.

The fix addresses the Windows issue where inconsistent line terminator handling could introduce extra blank lines.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@spoorcc spoorcc merged commit 3840e82 into main Jan 13, 2026
38 checks passed
@spoorcc spoorcc deleted the spoorcc/issue945 branch January 13, 2026 06:31
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.

Diff: new files have extra new line

3 participants