Re: [PATCH] Fix CRLF handling

[Kai Willadsen <kai willadsen gmail com>]

On 13 May 2010 17:23, Piotr Piastucki <leech miranda gmail com> wrote:
> Hi,
>
> I have noticed an issue in the way files with CRLF line terminators are
> loaded into text buffers in meld. When such a terminator appears exactly at
> the 4kb boundary an extra empty line will be inserted as 2 characters the
> terminator consists of are inserted separately.
> To reproduce the issue:
> - run "meld msgdialog.c msgdialog.c" (msgdialog.c attached)
> - go to line 833 and see a superfluous empty line
> - change any line after 833 e.g. 840 - see incorrect highlighting

Wow. That's a really nice catch. I'm not even going to try to guess
how many hours of debugging that cost you. Well done.

> The attached patch fixes the issue by changing the way files are loaded into
> text buffer. This is the simplest fix. If for some reason partial inserts
> should not be removed then I will try to provide a more complex patch (ring
> buffer + checking characters at the end of the buffer)

I have no problems with partial inserts being removed. However, the
patch removes all of the buffer clearing on errors; while I agree that
most (if not all) can be removed, they're not really related to the
problem at hand.

More importantly, what we *do* lose with this fix is progressive
display of the file as it's loaded. This is nice for giving the user
an indication of progress. I'm not sure what the performance
implications are, but I think that just replacing the two buf.insert()
calls with buf.set_text() should work?

cheers,
Kai