Commit c767b52
committed
Now Diff.__str__ works correctly in all python versions.
Additionally, unicode handling was improved to the point where we deal
with all diff(create_path=True) data as binary.
Therefore we don't claim to know all encodings of all textfiles in the world,
even though we still assume that everything git throws at us is utf-8 encoded.
Fixes gitpython-developers#1131 parent 85a5a8c commit c767b52
2 files changed
Lines changed: 31 additions & 9 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -10,7 +10,10 @@ | |||
| 10 | 10 | from .objects.blob import Blob | |
| 11 | 11 | from .objects.util import mode_str_to_int | |
| 12 | 12 | ||
| 13 | - from git.compat import defenc | ||
| 13 | + from git.compat import ( | ||
| 14 | + defenc, | ||
| 15 | + PY3 | ||
| 16 | + ) | ||
| 14 | 17 | ||
| 15 | 18 | ||
| 16 | 19 | __all__ = ('Diffable', 'DiffIndex', 'Diff') | |
@@ -194,7 +197,7 @@ class Diff(object): | |||
| 194 | 197 | (?:^deleted[ ]file[ ]mode[ ](?P<deleted_file_mode>.+)(?:\n|$))? | |
| 195 | 198 | (?:^index[ ](?P<a_blob_id>[0-9A-Fa-f]+) | |
| 196 | 199 | \.\.(?P<b_blob_id>[0-9A-Fa-f]+)[ ]?(?P<b_mode>.+)?(?:\n|$))? | |
| 197 | - """, re.VERBOSE | re.MULTILINE) | ||
| 200 | + """.encode('ascii'), re.VERBOSE | re.MULTILINE) | ||
| 198 | 201 | # can be used for comparisons | |
| 199 | 202 | NULL_HEX_SHA = "0" * 40 | |
| 200 | 203 | NULL_BIN_SHA = b"\0" * 20 | |
@@ -280,11 +283,21 @@ def __str__(self): | |||
| 280 | 283 | msg += '\nfile renamed to %r' % self.rename_to | |
| 281 | 284 | if self.diff: | |
| 282 | 285 | msg += '\n---' | |
| 283 | - msg += self.diff | ||
| 286 | + try: | ||
| 287 | + msg += self.diff.decode(defenc) | ||
| 288 | + except UnicodeDecodeError: | ||
| 289 | + msg += 'OMITTED BINARY DATA' | ||
| 290 | + # end handle encoding | ||
| 284 | 291 | msg += '\n---' | |
| 285 | 292 | # END diff info | |
| 286 | 293 | ||
| 287 | - return h + msg | ||
| 294 | + # Python2 sillyness: have to assure we convert our likely to be unicode object to a string with the | ||
| 295 | + # right encoding. Otherwise it tries to convert it using ascii, which may fail ungracefully | ||
| 296 | + res = h + msg | ||
| 297 | + if not PY3: | ||
| 298 | + res = res.encode(defenc) | ||
| 299 | + # end | ||
| 300 | + return res | ||
| 288 | 301 | ||
| 289 | 302 | @property | |
| 290 | 303 | def renamed(self): | |
@@ -298,7 +311,7 @@ def _index_from_patch_format(cls, repo, stream): | |||
| 298 | 311 | :param stream: result of 'git diff' as a stream (supporting file protocol) | |
| 299 | 312 | :return: git.DiffIndex """ | |
| 300 | 313 | # for now, we have to bake the stream | |
| 301 | - text = stream.read().decode(defenc) | ||
| 314 | + text = stream.read() | ||
| 302 | 315 | index = DiffIndex() | |
| 303 | 316 | previous_header = None | |
| 304 | 317 | for header in cls.re_header.finditer(text): | |
@@ -317,8 +330,17 @@ def _index_from_patch_format(cls, repo, stream): | |||
| 317 | 330 | # We just use the one mode we should have parsed | |
| 318 | 331 | a_mode = old_mode or deleted_file_mode or (a_path and (b_mode or new_mode or new_file_mode)) | |
| 319 | 332 | b_mode = b_mode or new_mode or new_file_mode or (b_path and a_mode) | |
| 320 | - index.append(Diff(repo, a_path, b_path, a_blob_id, b_blob_id, a_mode, b_mode, | ||
| 321 | - new_file, deleted_file, rename_from, rename_to, None)) | ||
| 333 | + index.append(Diff(repo, | ||
| 334 | + a_path and a_path.decode(defenc), | ||
| 335 | + b_path and b_path.decode(defenc), | ||
| 336 | + a_blob_id and a_blob_id.decode(defenc), | ||
| 337 | + b_blob_id and b_blob_id.decode(defenc), | ||
| 338 | + a_mode and a_mode.decode(defenc), | ||
| 339 | + b_mode and b_mode.decode(defenc), | ||
| 340 | + new_file, deleted_file, | ||
| 341 | + rename_from and rename_from.decode(defenc), | ||
| 342 | + rename_to and rename_to.decode(defenc), | ||
| 343 | + None)) | ||
| 322 | 344 | ||
| 323 | 345 | previous_header = header | |
| 324 | 346 | # end for each header we parse | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -72,7 +72,7 @@ def test_binary_diff(self): | |||
| 72 | 72 | assert len(res) == 1 | |
| 73 | 73 | assert len(list(res.iter_change_type('M'))) == 1 | |
| 74 | 74 | if res[0].diff: | |
| 75 | - assert res[0].diff == "Binary files a/rps and b/rps differ\n", "in patch mode, we get a diff text" | ||
| 75 | + assert res[0].diff == b"Binary files a/rps and b/rps differ\n", "in patch mode, we get a diff text" | ||
| 76 | 76 | assert str(res[0]), "This call should just work" | |
| 77 | 77 | # end for each method to test | |
| 78 | 78 | ||
@@ -86,7 +86,7 @@ def test_diff_index(self): | |||
| 86 | 86 | # end for each diff | |
| 87 | 87 | ||
| 88 | 88 | dr = res[3] | |
| 89 | - assert dr.diff.endswith("+Binary files a/rps and b/rps differ\n") | ||
| 89 | + assert dr.diff.endswith(b"+Binary files a/rps and b/rps differ\n") | ||
| 90 | 90 | ||
| 91 | 91 | def test_diff_patch_format(self): | |
| 92 | 92 | # test all of the 'old' format diffs for completness - it should at least | |
0 commit comments