From 656a58a7f1a7ea59a88677f88584c5cc948698d5 Mon Sep 17 00:00:00 2001 From: CLCK0622 Date: Sun, 25 Oct 2026 13:16:19 -0500 Subject: [PATCH] Fix rename detection for diffs without filename prefixes With `git diff --no-prefix`, `is_rename` unconditionally discards the first two filename characters, so a rename such as `a.py` to `b.py` goes undetected and `path` returns the source filename. Remove surrounding quotes for comparison and discard two characters only when both resulting paths start with a character followed by `/`; this retains custom prefixes such as `x/` and `y/`. For equally quoted inputs, comparisons with two such prefixes remain equivalent to the original behavior; otherwise, results change only when the original strings differ but become equal after discarding their first two characters. This follows the target-path behavior discussed in [#77](https://github.com/matiasb/python-unidiff/issues/77) and implemented in [#79](https://github.com/matiasb/python-unidiff/pull/79). A no-prefix rename where both real directory names resemble single-character prefixes, such as `a/x.py` to `b/x.py`, remains ambiguous from the header and is unchanged. --- tests/test_parser.py | 74 ++++++++++++++++++++++++++++++++++++++++++++ unidiff/patch.py | 15 +++++++-- 2 files changed, 86 insertions(+), 3 deletions(-) diff --git a/tests/test_parser.py b/tests/test_parser.py index 0c1aab9..6500c6b 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -443,6 +443,80 @@ def test_parse_diff_git_no_prefix(self): self.assertTrue(res[2].is_added_file) self.assertEqual(res[2].path, 'file3') + def test_parse_diff_git_no_prefix_rename(self): + # With --no-prefix, the first two characters belong to the filename. + for source, target in ( + ('a.py', 'b.py'), ('v1.txt', 'v2.txt'), ('x', 'y'), + ('ab', 'cd'), ('"a.py"', '"b.py"'), + ('ab/foo', 'cd/foo'), ('"x"', '"y"')): + diff = ( + 'diff --git %s %s\n' + 'similarity index 100%%\n' + 'rename from %s\n' + 'rename to %s\n' % (source, target, source, target)) + for metadata_only in (False, True): + with self.subTest(source=source, target=target, + metadata_only=metadata_only): + res = PatchSet(diff, metadata_only=metadata_only) + self.assertEqual(len(res), 1) + self.assertEqual(res[0].source_file, source) + self.assertEqual(res[0].target_file, target) + self.assertTrue(res[0].is_rename) + self.assertEqual(res[0].path, target) + self.assertEqual(str(res), diff) + + def test_parse_diff_git_rename_prefixes(self): + cases = ( + ('a.py', 'a.py', False, 'a.py'), + ('old.py', 'new.py', True, 'new.py'), + ('a/old.py', 'b/new.py', True, 'new.py'), + ('a/same.py', 'b/same.py', False, 'same.py'), + ('i/old.py', 'w/new.py', True, 'new.py'), + ('i/same.py', 'w/same.py', False, 'same.py'), + ('c/old.py', 'i/new.py', True, 'new.py'), + ('o/old.py', 'w/new.py', True, 'new.py'), + ('1/old.py', '2/new.py', True, 'new.py'), + ('1/same.py', '2/same.py', False, 'same.py'), + ('a/name.py', 'name.py', True, 'name.py'), + ('name.py', 'b/name.py', True, 'name.py'), + ('"a/name.py"', '"name.py"', True, '"name.py"'), + ('"name.py"', '"b/name.py"', True, '"name.py"'), + ('"a/old name.py"', '"b/new name.py"', True, '"new name.py"'), + ('"a/same name.py"', '"b/same name.py"', False, '"same name.py"'), + ('src://old name.py', 'dst://new name.py', True, + 'dst://new name.py'), + ('src://same name.py', 'src://same name.py', False, + 'src://same name.py'), + ('x/f.py', 'b/f.py', False, 'x/f.py'), + ('x/f.py', 'y/f.py', False, 'x/f.py'), + ('x/old.py', 'y/new.py', True, 'y/new.py'), + ('"x/f.py"', '"b/f.py"', False, '"x/f.py"'), + ('"x/\\303\\251.py"', 'b/e.py', True, 'e.py'), + ('x/e.py', '"b/\\303\\251.py"', True, '"\\303\\251.py"'), + ('"v1.txt"', '"v2.txt"', True, '"v2.txt"'), + ) + for source, target, is_rename, path in cases: + diff = 'diff --git %s %s\n' % (source, target) + for metadata_only in (False, True): + with self.subTest(source=source, target=target, + metadata_only=metadata_only): + res = PatchSet(diff, metadata_only=metadata_only) + self.assertEqual(len(res), 1) + self.assertEqual(res[0].source_file, source) + self.assertEqual(res[0].target_file, target) + self.assertEqual(res[0].is_rename, is_rename) + self.assertEqual(res[0].path, path) + self.assertEqual(str(res), diff) + + def test_parse_diff_git_added_removed_not_renamed(self): + for source, target, path in ( + ('/dev/null', 'b/a.py', 'a.py'), + ('a/a.py', '/dev/null', 'a.py')): + diff = '--- %s\n+++ %s\n' % (source, target) + res = PatchSet(diff) + self.assertFalse(res[0].is_rename) + self.assertEqual(res[0].path, path) + def test_parse_diff_git_mnemonic_prefix(self): # issue #81: git diff with diff.mnemonicPrefix set uses i/ w/ (etc.) # instead of a/ b/; path should still resolve to the plain filename. diff --git a/unidiff/patch.py b/unidiff/patch.py index 11083d8..078673f 100644 --- a/unidiff/patch.py +++ b/unidiff/patch.py @@ -396,9 +396,18 @@ def removed(self) -> int: @property def is_rename(self) -> bool: - return (self.source_file != DEV_NULL - and self.target_file != DEV_NULL - and self.source_file[2:] != self.target_file[2:]) + if self.source_file == DEV_NULL or self.target_file == DEV_NULL: + return False + source = self.source_file + target = self.target_file + if source.startswith('"') and source.endswith('"'): + source = source[1:-1] + if target.startswith('"') and target.endswith('"'): + target = target[1:-1] + # Custom git prefixes need not belong to RE_PATCH_FILE_PREFIX. + if source[1:2] == '/' and target[1:2] == '/': + return source[2:] != target[2:] + return source != target @property def is_added_file(self) -> bool: