Skip to content

fix: split base file content on newline only when applying hunks - #69

Merged
vitali87 merged 5 commits into
mainfrom
fix/apply-hunks-line-splitting
Aug 31, 2026
Merged

fix: split base file content on newline only when applying hunks#69
vitali87 merged 5 commits into
mainfrom
fix/apply-hunks-line-splitting

Conversation

@vitali87

Copy link
Copy Markdown
Owner

Problem

apply_hunks split the base file with str.splitlines(keepends=True), which treats \x0c (form feed), \x0b, \x1c\x1e, \x85, and as line breaks. git does not, so hunk source_start offsets refer to \n-counted lines while the slice was applied to a differently-numbered list. Any file containing one of those characters before a hunk (form feeds are common in older Python/C sources and Emacs page breaks) is silently corrupted in every sub-PR:

base: "a\x0cb\nc\nd\n…\nk\nl\n", dev changes k→K
materialized: 'a\x0cb\nc\nd\ne\nf\nh\ni\nj\nK\nl\nl\n'   # 'g' lost, 'l' duplicated

Fix

split_git_lines(content) splits on \n only (keeping the terminator, handling a missing trailing newline and empty content), and apply_hunks uses it.

Tests

  • split_git_lines parametrised over empty / trailing-newline-less / form feed / vertical tab / U+2028 / NEL / CRLF / blank lines
  • apply_hunks regression with a form feed on line 1 and a hunk at line 7 reconstructs the dev content exactly

458 tests pass, ruff clean.

Stacked on #50 (fix/preserve-missing-trailing-newline) because both edit apply_hunks; retargets main when #50 merges.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 58 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d507d976-b233-4b8c-bb25-a1410ef6add1

📥 Commits

Reviewing files that changed from the base of the PR and between 2e946a3 and 6b3be16.

📒 Files selected for processing (2)
  • pr_split/diff_ops/reconstructor.py
  • tests/test_reconstructor.py

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.

@vitali87
vitali87 force-pushed the fix/apply-hunks-line-splitting branch from 7762aad to f77512b Compare August 30, 2026 13:06
@vitali87
vitali87 force-pushed the fix/apply-hunks-line-splitting branch from f77512b to 6dbe1ed Compare August 30, 2026 13:15
@vitali87
vitali87 force-pushed the fix/apply-hunks-line-splitting branch from 6dbe1ed to 3502459 Compare August 30, 2026 19:15
@vitali87
vitali87 force-pushed the fix/apply-hunks-line-splitting branch from 3502459 to 2b24dae Compare August 30, 2026 19:16
Base automatically changed from fix/preserve-missing-trailing-newline to main August 30, 2026 19:17
apply_hunks used str.splitlines, which also breaks on form feed,
vertical tab, \x1c-\x1e, \x85, \u2028 and \u2029. git counts none of
those as line breaks, so a file containing one had every later hunk
applied at the wrong offset: a line was lost and another duplicated,
with no error. split_git_lines splits on \n only, matching git's line
numbering.
@vitali87
vitali87 force-pushed the fix/apply-hunks-line-splitting branch from 2b24dae to ab692d4 Compare August 30, 2026 19:17
@vitali87
vitali87 merged commit e128b2d into main Aug 31, 2026
4 checks passed
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.

1 participant