Skip to content

Conversation

@MirrorDNA-Reflection-Protocol

Summary

Fixes #2013

When diff.mnemonicPrefix=true is set in git config, git uses different prefixes for diff paths instead of the standard a/ and b/:

  • c/ for commit
  • w/ for worktree
  • i/ for index
  • o/ for object
  • h/ for HEAD

Previously, the diff regex and decode_path() function only accepted a/ and b/ prefixes, causing create_patch=True diffs to fail parsing when users had this config enabled.

Reproduction

As described in #2013:

# With ~/.gitconfig containing:
# [diff]
#     mnemonicPrefix = true

repo = git.Repo('.')
ancestor_ref = repo.merge_base(repo.head, repo.refs['main'])
diff = ancestor_ref[0].diff(None, create_patch=True)  # Would fail to parse

Changes

  • Update re_header regex to accept [abciwoh]/ prefixes instead of just [ab]/
  • Update decode_path() assertion to accept all valid mnemonic prefixes
  • Add test case for mnemonicPrefix-style diffs

Testing

  • New test test_diff_mnemonic_prefix verifies parsing works with c/ and w/ prefixes
  • All existing diff tests pass (except one pre-existing failure on main unrelated to this change)

References

Fixes gitpython-developers#2013

When diff.mnemonicPrefix=true is set in git config, git uses different
prefixes for diff paths instead of the standard a/ and b/:
- c/ for commit
- w/ for worktree
- i/ for index
- o/ for object
- h/ for HEAD

Previously, the diff regex and decode_path() function only accepted
a/ and b/ prefixes, causing create_patch=True diffs to fail parsing.

Changes:
- Update re_header regex to accept [abciwoh]/ prefixes
- Update decode_path() assertion to accept all valid mnemonic prefixes
- Add test case for mnemonicPrefix-style diffs
- Remove whitespace from blank lines in docstrings (ruff W293)
- Use !r format specifier for bytes in f-string (mypy str-bytes-safe)
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes diff parsing to support Git's diff.mnemonicPrefix configuration option, which was causing parse failures when users enabled this feature. When diff.mnemonicPrefix=true is set, Git uses alternative path prefixes (c/, w/, i/, o/, h/) instead of the standard a/ and b/ prefixes in diff output.

Key changes:

  • Updated the re_header regex pattern to accept all valid mnemonic prefixes in addition to standard a/b prefixes
  • Modified decode_path() function to validate and strip all supported prefix types
  • Added test coverage for mnemonicPrefix-style diffs

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
git/diff.py Updated regex pattern and decode_path() function to support c/, w/, i/, o/, and h/ prefixes alongside standard a/ and b/ prefixes
test/test_diff.py Added test case verifying diff parsing works correctly with mnemonicPrefix-style c/ and w/ prefixes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

git/diff.py Outdated

# Precompiled regex.
# Note: The path prefixes support both default (a/b) and mnemonicPrefix mode
# which can use prefixes like c/ (commit), w/ (worktree), i/ (index), o/ (object)
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

The comment is missing the h/ prefix for HEAD. According to the PR description and the Git documentation, h/ is also a valid mnemonicPrefix. The comment should list all supported prefixes: c/ (commit), w/ (worktree), i/ (index), o/ (object), and h/ (HEAD).

Suggested change
# which can use prefixes like c/ (commit), w/ (worktree), i/ (index), o/ (object)
# which can use prefixes like c/ (commit), w/ (worktree), i/ (index), o/ (object), and h/ (HEAD)

Copilot uses AI. Check for mistakes.
- c/ for commit
- w/ for worktree
- i/ for index
- o/ for object
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

The docstring is missing the h/ prefix for HEAD. According to the PR description and Git documentation, h/ is also a valid mnemonicPrefix. The comment should list all supported prefixes: c/ (commit), w/ (worktree), i/ (index), o/ (object), and h/ (HEAD).

Suggested change
- o/ for object
- o/ for object
- h/ for HEAD

Copilot uses AI. Check for mistakes.
Comment on lines 284 to 314
def test_diff_mnemonic_prefix(self):
"""Test that diff parsing works with mnemonicPrefix enabled.
When diff.mnemonicPrefix=true is set in git config, git uses different
prefixes for diff paths:
- c/ for commit
- w/ for worktree
- i/ for index
- o/ for object
This addresses issue #2013 where the regex only matched [ab]/ prefixes.
"""
# Create a diff with mnemonicPrefix-style c/ and w/ prefixes
# Using valid 40-char hex SHAs
diff_mnemonic = b"""diff --git c/.vscode/launch.json w/.vscode/launch.json
index 1234567890abcdef1234567890abcdef12345678..abcdef1234567890abcdef1234567890abcdef12 100644
--- c/.vscode/launch.json
+++ w/.vscode/launch.json
@@ -1,3 +1,3 @@
-old content
+new content
"""
diff_proc = StringProcessAdapter(diff_mnemonic)
diffs = Diff._index_from_patch_format(self.rorepo, diff_proc)

# Should parse successfully (previously would fail or return empty)
self.assertEqual(len(diffs), 1)
diff = diffs[0]
# The path should be extracted correctly (without the c/ or w/ prefix)
self.assertEqual(diff.a_path, ".vscode/launch.json")
self.assertEqual(diff.b_path, ".vscode/launch.json")
Copy link

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

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

The test only covers c/ and w/ prefixes but doesn't test the other valid mnemonicPrefix options (i/, o/, h/). Consider adding test cases for these prefixes as well to ensure the regex pattern [abciwoh] and the decode_path() function work correctly with all supported prefixes. This could be done either by adding more test data in this test or creating a parameterized test.

Copilot uses AI. Check for mistakes.
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for contributing! I particularly like the exhaustive documentation around the matter, and what I see looks good!

The test is also something I'd love to see improved, and I side with Copilot here.

… values

Changes per reviewer (Byron) and Copilot suggestions:
- Add h/ (HEAD) to the comment listing supported prefixes
- Update test docstring to include h/ prefix
- Expand test to cover all prefix combinations using subTest:
  - c/ (commit) vs w/ (worktree)
  - c/ (commit) vs i/ (index)
  - i/ (index) vs w/ (worktree)
  - o/ (object) vs w/ (worktree)
  - h/ (HEAD) vs i/ (index)
  - h/ (HEAD) vs w/ (worktree)

This ensures the regex pattern [abciwoh] and decode_path() work correctly
with all supported mnemonicPrefix values.
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.

gitconfig: mnemonicPrefix conflict with create_patch

2 participants