On 01/02/2017 07:17 PM, Andrew Shadura wrote:
kallithea/lib/diffs.py | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
# HG changeset patch
# User Andrew Shadura <[email protected]>
# Date 1483380766 -3600
# Mon Jan 02 19:12:46 2017 +0100
# Branch stable
# Node ID ee8c9993a3aacb371fc5e59addc9434008290679
# Parent b4dd4c16c12d8932d99a2182c7e8ed0f456fb85f
diff: don't cut binary diff parsing short
With binary diffs, user might want to see something more meaningful
than just 'Binary file', for example, an image diff. However,
interrupting parsing on encountering a binary file leaves us in
an inconsistent state: op is None, which causes exception later
in processing, force setting it to 'M' isn't correct as the diff
may be a deletion or creation of a file as well as a simple
modification, so we need to actually parse the diff to find out
what are we dealing with.
With that in mind, this shortcut doesn't save us anything really,
but only creates troubles. Hence, this changeset gets rid of it.
Thanks!
It looks like this patch only touch the case of looking at a diff for a
file, not for a changeset / pull request / compare diff? It would be
nice to get that clarified in the commit message.
Since we apparently had a serious crash here, can you add a test for
this case? We don't have other test coverage of wrapped_diff, but
test_diff_parsers.py comes close and suggests how it perhaps could be
tested.
Exactly what exception did you get? The patch seems to apply to stable,
but there is no 'op' there? I would like to understand the problem so I
can review and test the fix.
It seems like I might have fixed the exception on the default branch in
https://kallithea-scm.org/repos/kallithea/changeset/79676fef1ae0 - can
you confirm that?
Creating and parsing a diff can be slow - especially if it is a big or
hard diff, as binary diffs often will be. Just showing "binary file" is
thus a nice optimization. But showing the right M/A/D is also nice ...
Considering these comments, how do you suggest moving forward on the 2
branches?
/Mads
diff --git a/kallithea/lib/diffs.py b/kallithea/lib/diffs.py
--- a/kallithea/lib/diffs.py
+++ b/kallithea/lib/diffs.py
@@ -62,12 +62,7 @@ def wrapped_diff(filenode_old, filenode_
if filenode_old is None:
filenode_old = FileNode(filenode_new.path, '', EmptyChangeset())
- if filenode_old.is_binary or filenode_new.is_binary:
- diff = wrap_to_table(_('Binary file'))
- stats = (0, 0)
- size = 0
-
- elif cut_off_limit != -1 and (cut_off_limit is None or
+ if cut_off_limit != -1 and (cut_off_limit is None or
(filenode_old.size < cut_off_limit and filenode_new.size <
cut_off_limit)):
f_gitdiff = get_gitdiff(filenode_old, filenode_new,
_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general
