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



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.

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

edit.c: In function `init_dynamic_edit_buffers':
edit.c:204: `O_BINARY' undeclared (first use in this function)
edit.c:204: (Each undeclared identifier is reported only once
edit.c:204: for each function it appears in.)
edit.c:177: warning: unused variable `buf2'
edit.c:176: warning: unused variable `buf'
edit.c:182: warning: `putp' might be used uninitialized in this function
edit.c:188: warning: `need_lf' might be used uninitialized in this 
function

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

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

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

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

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

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

Wrong.
1) Make it compile without warnings.  Warnings are for you, not for me.
2) Make it work on systems other than Cygwin.
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 :)

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

-- 
Regards,
Pavel Roskin




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