2009/6/7 Piotr Piastucki <leech miranda gmail com>: > >> This isn't (by itself) true. The call you're moving assigns to block, >> which is used in subsequent calls to _merge_blocks(). In order to not >> change behaviour, either the value of block needs to be unchanged by >> the cases that you're now skipping, or the changes to block need to be >> irrelevant. The commit message should include some brief explanation >> about why this is the case. >> >> Kai >> > > Ok, I agree this part may not look obvious, so I will provide further > explanation. Basically, the previous value of block is irrelevant and should > not be passed, because: > > 1) last_diff (previous value of block) is used in merge_blocks() only if > len(using[0]) or len(using[1]) is 0: > ... > if len(using[i]): > d = using[i][0] > low.append( lowc - d[LO] + d[2+LO] ) > d = using[i][-1] > high.append( highc - d[HI] + d[2+HI] ) > else: > d = last_diff > > ... > > The value calculated and returned by merge_blocks() in this case is > obviously not used to compute the result of merge_diffs() > > 2) No matter what previous value of block is passed to merge_blocks() it > will not be used when both len(using[0]) and len(using[1]) are not 0. This > is obviously the only case when the result returned by merge_blocks() will > affect the result of merge_diffs(). > > 3) Having said that it seems to be reasonable to assume that there is no > need to pass any previous value of block to merge_blocks(), because it will > never affect the result of merge_diffs() which is actually the only caller > of merge_blocks(). > > Please let me know your thoughts. Thanks - that's exactly the kind of reasoning I was looking for. I've attached an updated version of the patch including a brief version of this reasoning in the commit message. There's also a small additional cleanup attached. Further review and testing would be much appreciated. cheers, Kai
Attachment:
0001-Simplifying-diff-chunk-merging.patch
Description: Binary data
Attachment:
0002-Clean-up-unused-assignments.patch
Description: Binary data