Re[2]: [PATCH] edit.c, init_dynamic_edit_buffers (was Re[2]: CR/LF translation)



Hello, Pavel! :)

Thursday, August 22, 2002, 5:58:49 PM, you wrote:

PR> Hello, Pavel!

>> Attached is a patch which implements it 'seamlessly' to the editor -
>> i.e. it reads all data and it is translated on the fly.

PR> I like the idea of seamless translation.  But your code doesn't compile on
PR> Linux (with -O2 -Wall):

Sorry, I'll fix it. I had Linux on my laptop - I'll doble check next
time.

PR> Please, if you want somebody to take your code seriously, fix the warnings 
PR> yourself, because you know best how to do it.

Sure. I was a little over-excited.

PR> After defining O_BINARY to 0, all files appear empty in the editor.

Should be fixed now.

I did quite some testing and it behaves fine - i.e. DOS line endings
are converted properly and UNIX file are kept as they are. I did some
cleanup and fixed various little things - so this patch should be OK :)
its not perfect though.

PR> Send a working patch, then I all review the code.  Also please remove the 
PR> parts that become unused, don't just comment them out.

Sure.

PR> Do you still rely CR_LF_TRANSLATION?  It is also used in editcmd.c, and I 
PR> don't see any changes there.

No - though there is code which requires special handling. I'll think
of something. The code is edit_stream_[read|write]. My code processes
the data stored in edit->buffers2[] array, while the code of
edit_stream_[read|write] works char by char. As far as I can see it is
used in 'insert file' only.

PR> I think that we together can ultimately make good code, and 
PR> CR_LF_TRANSLATION is known to be broken (at least in design, since it 
PR> should not be a compile time option).  Maybe I can remove all code under 
PR> CR_LF_TRANSLATION now?

Not yet -  see above. Though it is broken anyway.

>> One is: Move that functionality out in a separate function :)

PR> Wrong.
PR> 1) Make it compile without warnings.  Warnings are for you, not for me.
PR> 2) Make it work on systems other than Cygwin.
PR> 3) Make clear what you are going to do with commented out parts.

>> P.S. This patch handles only translation of CRLFs' on input - i plan to
>> do the other if this is going to be accepted in one form or the other.
>> Or maybe part of Alexanders patches can enter CVS with part of mine :)

PR> What does it mean from the user point of view?  If I edit a file with DOS
PR> newlines, will it have UNIX newlines after saving?

Yep. Right now it is like this - but of course I'll be working on that.
I repeate - this is not a patch to be included tommorow in the CVS if
accepted at all. I just wanted to start a discussion. And also there
is the patch of Alexander - I thought you were interested of some
parts of it if not as a whole. Thats why I started my patch - you
showed that his CRLF->LF patch is unacceptable for various reasons. So I
prepared that patch which works with the internal buffers of MC
editor.

Btw there are some unpleasent bugs in the editor - and I dont really think
that it can open gigabyte files. Actually it is limited to 65Mb from
what I can see.


Thanks! :)

Pavel Tsekov

Attachment: mc-init_dynamic_edit_buffers_fixed.patch
Description: Binary data



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]