Re: MC and file highlighting

[Pavel Tsekov <ptsekov gmx net>]

Hello,

On Thu, 18 Aug 2005, Jindrich Novy wrote:

> On Wed, 2005-08-17 at 17:49 +0400, X-Stranger wrote:
> > Hi all!
> >
> > I think it will be much better, if MC will be able to hightlight files on
> > panels depending of it's extensions. I wrote a small patch (see attached
> > file), that realises this feature. What do you think about to include it in
> > MC source code?
> >
> > Regards, X-Stranger
> >
> > P.S. Patch realises file highlighting feature and a dialog to setup file
> > extensions lists and save it into MC's ini-file.
> > _______________________________________________
> > Mc-devel mailing list
> > http://mail.gnome.org/mailman/listinfo/mc-devel
>
> This is a very nice feature! It's what I missed since I was using the
> old good DOS Navigator very long ago. The patch seems sane, the only bad
> thing I found was the duplicated comment at line 543 in the patch that
> should be removed and one C++ comment at line 219 that should be changed
> to C one.
>
> The other thing that hit my eyes was the Solaris hack at line 428. Is
> that really needed? I feel that strsep() is sufficient here and the
> sunos_get_token() call might be a source of problems in the future
> regarding the portability of mc what brings a neglectable benefit (if
> any).

Several things that caught my eye:

1) free/malloc/strdup/etc usage - please, use the appropriate glib call
   instead i.e. g_free/g_malloc/g_strdup/etc.

2) perhaps the big if ... else-if block in file_entry_color() should be
   moved to is_file_type() .

3) strcmp() is performed on the file extension and an entry from the file
   type extensions list. The code assumes that the file type extensions
   list should be lowercase but one can enter upper / mixed case
   extensions. In this case strcmp() fails.

4) in file_compute_caller () you've  changed the formating of the switch
   block .

5) it would be good if the list of file types is not hardcoded i.e.

   Databases, Graphics, Multimedia, etc.

   but user configurable. just a wish.

I think this is enough for starters :)