Skip to content

fix(repo): preserve trailing whitespace in blamed line content - #2339

Open
Keerthana-64 wants to merge 2 commits into
gitpython-developers:mainfrom
Keerthana-64:blame-preserve-line-content
Open

Keerthana-64 wants to merge 2 commits into
gitpython-developers:mainfrom
Keerthana-64:blame-preserve-line-content

Conversation

@Keerthana-64

Copy link
Copy Markdown
Contributor

repo.blame mein line_bytes.rstrip().decode() use hone ki wajah se line ke end ke spaces aur tabs remove ho rahe the. Whitespace-only lines empty ho jaati thi aur CRLF line ka carriage return bhi remove ho raha tha.

Is wajah se repo.blame() original file ka exact content return nahi kar raha tha, jisse exact line matching aur commit identify karne mein problem aa sakti thi.

Fix: Ab sirf starting tab aur ending line feed remove hoti hai. Baaki original content preserve rehta hai.

Testing: blame-content.sh aur test_blame_preserves_line_content dono backends par successfully pass ho rahe hain.

`Repo.blame` built each blamed line with `line_bytes.rstrip().decode(defenc)`
and then stripped the leading tab, but a `git blame -p` content line is a
leading tab, the blamed line's exact bytes, and git's own terminating line
feed. `rstrip` therefore dropped a line's trailing spaces and tabs, emptied a
whitespace-only line, and removed the carriage return of a CRLF line, so the
content returned by `repo.blame()` differed from both the file and
`git blame`. A caller that reconstructs or compares file content from blame,
for example to find the commit that introduced an exact line, was handed
altered text.

Recover the exact bytes instead: drop the leading tab and only git's trailing
line feed, then decode. Binary lines and the hexsha/header parsing are
unchanged, so the only behavior change is that blamed lines keep their
original trailing whitespace and carriage returns.

Adds `tests/fixtures/blame-content.sh` and `test_blame_preserves_line_content`
in `test/test_repo.py`, which fail before the change
(`['alpha', 'beta', '', 'crlf', 'gamma']`) and pass after
(`['alpha   ', 'beta', '\t', 'crlf\r', 'gamma']`) on both the `cli` and `gix`
backends. `ruff check`, `ruff format --check` and `mypy` pass on the touched
files. The repository's pre-existing shallow-clone failures occur identically
with and without this change.

Agent-Model: claude-opus-4-8

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the change to preserve trailing whitespace and carriage returns in blame output.

No open issues 🎉

What changed in this PR

This PR fixes Repo.blame() text-line parsing so returned content preserves trailing whitespace and carriage returns from CRLF lines.

Changes:

  • Remove only the porcelain content prefix and terminating line feed before decoding.
  • Add a byte-preserving fixture and regression test covering trailing spaces, a tab-only line, and CRLF content.
File Description
tests/​fixtures/​blame-content.sh Creates committed content with exact whitespace bytes.
test/​test_repo.py Verifies blame preserves the expected line content.
git/​repo/​base.py Decodes content without stripping its trailing whitespace.

Review effort: Balanced

`test_revision_arguments_preserve_pathlike_paths` fails in the Windows
`Python package` jobs because `Path("refs/heads/main")` emits
`refs\heads\main`. Git rejects backslashes in reference names, so the
test raises `ValueError: Invalid reference` before exercising revision
argument handling. All four combinations of SHA-1/SHA-256 and
files/reftable fail. The same failures are present in the final Windows
Python 3.14t job for the introducing PR gitpython-developers#2336, so this is inherited by
PR gitpython-developers#2339 rather than caused by its blame-content change.

Use `PurePosixPath` for the reference-name test parameter. Reference
names use forward slashes on every platform; filesystem paths elsewhere
in the tests continue to use `Path`. Keep `PathLikeMock` coverage and
the existing reference validation unchanged.

Validation: 28 focused reference/revision tests pass on each of the
`cli` and `gix` backends, including all eight PathLike cases, nested
merge-base operands, reflog walks, fork-point lookup, and remote archives.
Both blame-content regressions also pass on each backend. Ruff 0.17.2
lint and formatting checks pass for `test/test_refs.py`; `git diff --check`
passes. Windows validation is left to the PR's CI matrix.

Agent-Model: unknown
@Byron

Byron commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Codex on behalf of Byron: pushed ff48765 (test(refs): keep PathLike reference names portable on Windows) to fix the inherited Windows failures.

Path("refs/heads/main") produces refs\heads\main on Windows, which Git correctly rejects. The test now uses PurePosixPath so reference names retain Git's forward-slash syntax on every platform. Library behavior and reference validation are unchanged.

The earlier revision of #2336, d03d3d7, passed CI. Its follow-up 6de90f1 added this test and failed with the same four Windows cases. This explains why the introducing PR appeared to have passed previously.

Locally, 28 focused reference tests and both blame-content regressions pass on each of the CLI and Gix backends. Ruff lint and formatting checks also pass. All 64 CI checks passed for ff48765, including every Windows CLI and Gix job. There are no unresolved review threads. The PR workflow is complete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants