Re: Midnight Commander mod with Colorer-take5 syntax engine

["Igor Russkih" <irusskih gmail com>]

On 6/21/06, Leonard den Ottolander <leonard den ottolander nl> wrote:
> Hello Igor,
>
> original in mc (where can I send patches? Is there a mailing list for
> colorer?). Only have to look at the String type as we normally

Sure, you can use colorer-talks lists sourceforge net
(http://lists.sourceforge.net/mailman/listinfo/colorer-talks)


> Although the startup of the viewer seems a bit slow with colorer this
> has much added value over the default syntax highlighting, and although

Yep, the startup time is a bit heavier. Basically this depends on a
syntax complexity and dependencies.

> I think we could well include the colorer hooks in the main mc tree I
> don't believe we should drop the existing highlighting, over even make
> colorer the default. Firstly the default syntax is more lightweight,
> which some people might prefer. Secondly the use of colorer requires the
> installation of a(n extra) third party library.

This was one of the ideas I've tried to reach in my patch. Colorer
library can be disabled either during compilation time (thus making no
dependencies on libcolorer) or in runtime (in this case regular mc
syntax highlighter can work).

> Is this a fully functional patch? Much lighter than the diff I got
> diffing your tarball against CVS :) .

Tarball can contain some additional generated files or codes which are
not included into SVN repository. From the other side, colorer's SVN
repository contains the only files from MC-2006-05-30-15 snapshot
which I've altered. This can be a reason.

> In that patch I noticed a couple of things:
> - There are a lot of fixes to the po files and some to the Russian and
> Serbian(?) man page. Please submit these to this list separately.
Just a single ru.po file with a few additions.

> - I noticed a CR here and there. There seem to be even more in the patch
> you sent. Please get rid of those (fe with dos2unix).
thanks, that's because of windows-based SVN client.

For all the next comments it seems to be a misunderstanding (possibly
because my diff is based on MC-2006-05-30-15 snapshot, not the latest
one).
> - There are a lot of added comments/fixes in config.h.in that I do not
> see in your patch. Please submit separately.
> - You got rid of the COPYING.LGPL file in vfs. I think this might be a
> mistake on your part. You might want to rectify that for your version.
> - In your version you got rid of the pipethrough files. This has no
> bearing on us as it's not in your patch.
> - You made some fixes to the config files as well that seem same. Please
> submit those separately for review.
> - I noticed changes in vfs/samba/configure. Most of this are somewhat
> odd whitespace changes, but there appear to be some hunks at the top and
> bottom to actually do something. Please submit (not the whitespace) :) .
> - Why did you get rid of vfs/extfs/README.it? (not much of an issue)

I'll upmerge with latest CVS MC revision and resend it.


> > code). As far as I understand MC uses C only. Is this a problem?
> Yes, it might. The overall approach towards accepted language constructs
> is a bit conservative to maintain compatibility with older systems.
> Would it be much work to rewrite these parts?

The main problem with this is that syntax-colorer.cpp file makes
direct connection between mc editor data and colorer's code.
Technically it is possible to extract a kind of 'generic' C API and
include it on library side. In this case MC will have pure C codes.
> From the other side this will not release MC from libcolorer
dependency - and this means that  if user wants to compile and use
mc-colorer, he anyway should have C++ environment.

I mean I see no reason to eliminate that C++ code from MC: those who
have C-only environments can just disable colorer's support in compile
time.

> This is why I do not want to make colorer the default. I think this is
> something we should leave to the distributions.
Agreed.

> P.S. Could you please set your mail client to produce plain text mails
> and use an indentation character and a "replying to" line for quotes?
sorry, fixed.


--
 Igor