On 01/03/2017 09:09 AM, Andrew Shadura wrote:
Right, I made a mistake of rebasing it to the stable branch as I thought
OOK ran stable, which it didn't. Sure, there's no op there, the crash
happens on the default branch only, it's

'in <string>' requires string as left operand, not NoneType

in the line with "if op in 'DM':".

The context is, for example,
https://kallithea-scm.org/repos/kallithea/diff/rhodecode/public/images/user14.png?diff1=41a695e604ba65f2943cbc7996038c524d692603&diff2=5351a3a32381d3f118720d3e9e1a48b4bdc8b85a&diff=diff&fulldiff=1
(it's patched in the public instance though).

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?
Well, that's the commit which in fact created it :)

Thanks for clarifying - it would be nice to have some of these details and context in the commit message.

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 ...
Are we really saving enough to be worth the troubles?

I guess it would be justified to drop the special handling of binary files as you suggest. But then we also have to make sure we handle the case of too-large files too so we *never* return unknown op. Perhaps by computing the diff but just not render it as html? And perhaps add an assert to make sure _parse_gitdiff never returns None? Or just add a "if op" in the template?

Considering these comments, how do you suggest moving forward on the 2
branches?
Can we detect addition or removal without actually parsing diffs? I
haven't found an easy way yet :(


For Mercurial, we could look at the output of 'status' between the two revisions. For full revision diffs, that could also allow us to compute the diffs file by file and perhaps make it easier to fetch and render on demand (even thought it probably is better to do that once and cache it ...).

I doubt that is the right solution right now.

/Mads

_______________________________________________
kallithea-general mailing list
[email protected]
https://lists.sfconservancy.org/mailman/listinfo/kallithea-general

Reply via email to