Re: MC and file highlighting (fwd)
Hello, Please, do keep the discussion on the list. I am forwarding your message there. -- Forwarded message -- Date: Mon, 22 Aug 2005 17:07:17 +0400 From: X-Stranger To: Pavel Tsekov Subject: Re: MC and file highlighting В сообщении от 19 Август 2005 18:40 Вы написали: 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. Fixed. 2) perhaps the big if ... else-if block in file_entry_color() should be moved to is_file_type() . I don't think so. 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. Just try to use MC with this patch and you will see, that it is all right and nothing fails. 4) in file_compute_caller () you've changed the formating of the switch block . file_compute_color, fixed. 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 :) If you don't like this categories, you can name them such as group #1, group #2, etc. But I think that this list of file types is usable for all peoples ;) Final version attached. Regards, X-Stranger mc-4.6.1-file-highlight.patch.bz2 Description: BZip2 compressed data ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: MC and file highlighting (fwd)
Hello, On Mon, 22 Aug 2005, Pavel Tsekov wrote: 2) perhaps the big if ... else-if block in file_entry_color() should be moved to is_file_type() . I don't think so. Why ? 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. Just try to use MC with this patch and you will see, that it is all right and nothing fails. I did and it doesn't work for me. In the dialog which is used to enter the file extensions replace 'bak' with 'Bak' . Create a file with extension Bak and see what happens. You convert the file extension of the file to lower case and then compare to the list. I would not bother to comment a patch if I had not took the time to review it. 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 :) If you don't like this categories, you can name them such as group #1, group #2, etc. But I think that this list of file types is usable for all peoples ;) Well it is arguable. There are certainly benefits if the user could enter its own categories . I.e. not only the five categories that you have introduced. ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: MC and file highlighting
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 :) ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
Re: MC and file highlighting
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). Jindrich -- Jindrich Novy [EMAIL PROTECTED], http://people.redhat.com/jnovy/ (o_ _o) //\ The worst evil in the world is refusal to think. //\ V_/_ _\_V ___ Mc-devel mailing list http://mail.gnome.org/mailman/listinfo/mc-devel
