On 29 March 2013 06:56, Pedro Pedruzzi <pedro pedruzzi gmail com> wrote:
Hello, I have added an initial (broken) support for 3-way in the attached patch and at github [1]. But I need some help to move on: 1. Where to do the tag creation/update logic (still) I've moved the logic to the on_diffs_changed callback because I need the set of chunks that had been changed. But it appears that I can't do this at that point because the textview has not been refreshed yet, neither yranges that are needed for the padding logic. So this is currently broken when some text change affects yranges (adding/removing of newline, line wrap etc). Adding a subsequent space usualy fix things. For my logic to work I would need: * The set of changed chunks (the ones to update the padding). * refreshed textview so yranges are ok.
This is visual, however, so we can't really tie changed chunks to ones that need updated padding. What happens if the user resizes the window and line wrapping changes the offsets? I don't know whether we can do this sanely. We can *create* the tags in on_diffs_changed, but I think we have no choice but to get the yranges and update padding in the expose event. Do you think that that will fix the problem?
I tried to save the changed_chunks on diffs_changed to later process on on_textview_expose_event. But the first call happens before the yranges gets updated, and there are many calls even without text edition. Any thougths?
I think this may be intrinsically slow, so I would say to get something working properly, and then worry about doing invalidation and optimisation. I suspect that it will be very difficult to do this both quickly and correctly.
2. Complete line ranges for 3-way diffs To support 3-way diffs I have to know the line ranges on all three panes for each chunk so I can create the padding tags. However, diffutils only provides line ranges for the panels that differs. For chunks that are identical between two consecutive panes, I don't have the line range for one of them. I took a look at diffutils but it doesn't seem simple enough for me to adapt. Any suggestions?
So... this is a bit of a nightmare. This is only possible with a significant change in our chunk handling... or some very dirty hacks. Luckily, we already have such hacks. The problem is that we discard equal chunks when we do a comparison. This leads to pain. The only short-term solution I could think of was to reuse FileDiff._synth_chunk() and adjust offsets from there. I've attached a patch that does just this, and mostly fixes these problems. There are still issues with this. Most obviously, this doesn't work in expose_event because of the single_changes iteration... though I'm sure we could hack around this. The more serious problem is with the way we do this tag handling. If a single line needs two tags (this can easily happen by combining insert and conflict chunks or similar) then we lose our sync. I'm not sure what the best solution to this is... maybe we need to add some handling for accumulative pixels-below-line tags?
3. Other bugs I have noticed many other bugs. Mainly when diffs change in such a way that the line with padding gets lost of track or when diffs split. But I believe this comes next.
Agreed. Again, I think it's necessary to get things working a piece at a time.
One idea that I wanted to mention was with regard to the tag handling.
My original thought (though I have no idea how well this would work)
was to tie each tag to an individual chunk, not a line. So, for
example, when you created a tag, you would number it according to
which chunk it was attached to. I'm not sure what the implications of
this would be for e.g., the accumulative pixels-below-line problem I
mentioned above, but it might make handling splitting of diffs
easier... or it might not. I just thought I'd raise the possibility.
Also, Meld currently runs (and I test on) Python 2.6. So, for example,
when doing debug lines, etc. please use the {0} syntax for formatting
instead of the {}.
cheers,
Kai
Attachment:
0001-Python-2.6-compat.patch
Description: Binary data
Attachment:
0002-Partially-fix-three-way-diff-in-cases-of-missing-Sam.patch
Description: Binary data