Re: [Gnome-print] First draft of new encoding routines

[Lauris Kaplinski <lauris ximian com>]

Hello!

Scott!
If you want your filters to be included in gnome-print, you have to
make these available under LGPL. I started porting it to
gnome-print stable tree, but discovered, that legally
I have no permission to distribute it! So I do not commit,
which is BAD THING, because then we all cannot test single
build tree :(
I understand, that often such issues are splitting of hair, but
after all we want to show everyone, that WE follow
copyright/licensing terms of free software, to be able to
ask that from others ;-)

On 17 Jun 2001 00:54:41 -0500, Chema Celorio wrote:
> Looks good, some comments :
> - Please add the GPL header to all the .c files. 
> - Patch agains -ps2 not -ps
> - Use a better Prefix. "Filter" and "filter_.." is too vauge. Since this
> symbols will end up in gnome-print, an app that uses gnome-print might
> have a symbol crash, think about filter_free for example.
> - Add comments to the source, we might need to fix bugs here later and
> the comments will save us time. I looked for example at the rle filter
> and at the old one, adding this notes really help if we need to look at
> this code after we forget about it.
> -  You don't seem to want to use glib functions.
> assert, strdup, malloc, realloc can be replaced with
> g_assert, g_strdup, g_malloc, g_realloc.
> Maybe you don't want this code to depend on glibc. Is this your
> intention ?
> - Dont' hardcode sizes of buffers, use a #define instead
> grep for 128 for example.
> - If we move this code to the gnomeprint directory we should change the
> filenames to gnome-print-filter

Well, most of these are fortunately minor problems, that can be resolved
in work.

I will place your code into separate subdirectory filter under 
libgnomeprint at first, so we create temporary .la library
during compilation. I you plan (we/somebody plans) to distribute
these in separate library, we can easily switch over using it.

As of namespace, well, little google search would do good. I checked
'filter_init' and it gave me quite a long list of matches (although
I had no time to check, whether any of these could cause real
problems).
I would suggest prefixing everything with something like sff_
(Scott Free Filter). There were some matches to sff too, but these
seemed to come from some higher-level language.
The real problem is, that most 2-letter namespaces are already
occupied, as are big number of 3-letter ones. And you cannot
actually use too long prefix, as AFAIK there are some silly
limits of distinguishable name length on some systems. Plus
long prefixes are extremely difficult to read/type/distinguish.

I do not care about coding style, as long as code is clearly
separate unit (as your filter code will be), and does not need
extensive maintenance/bugfixing on my side. But if I have to do that,
I tend to convert everything to my personal style ;-) That is quite
logical too, as every programmers eye is trained with certain
visual patterns in code, so finding bugs from differently
formatted code is much bigger pain.

Anyways, keep doing good work!
Lauris Kaplinski


> [The rest of the comments fall on the personal coding style, inside
> gnome-print we have several coding sytles mixed. This is not what we
> want and should standarize, I just don't want to make this even worse if
> possible. Hoever, this is a separate library so if you have strong
> preferences you can leave it like that]
> 
> - We (in GNOME/GTK) tend to use long function names, some people think
> shorter names save typing, but (we think that) longer names are better
> in the long run for clarity.
> 
> I would need to read the code to know what you meant by
> filter_opt_setstr, we tend to use 
> gnome_filter_options_set_string () in our code. For big projects this
> does make a difference, having a consistent naming convention is
> valuable.
> 
> We also tend to use longer variable names (althou there are exceptions
> Laris, boc, raph for example).
> 
> (Filter *filt, char *opt, char *value)
> 
> i would have written it like :
> 
> (Filter *filter, char *option, char *value)
> 
> If i am looking at some code written by miguel for example, i know most
> of the time what is the variable name by the type and the context.
> options is "option" not opt or optn. And a GtkWindow is "window" not "w"
> or "win".
> - The coding style with the already inconsistent style of gnome-print.
> We have 3 styles right now :
> 1. Miguel, Zucchi, Chema, Federico style
> (as in gnome-print.c, gnome-print-master.c, gnome-print-ps2,
> gnome-canvas-hacktext.c)
> 2. Lauris style, similar to 1 but he uses :
> "if (something) do something;" in one line and smaller function
> prototypes (as in gp-ps-unicode.c)
> 3. Raph style (as in gnome-print-ps.c)
> 
> There are some files that are even a mix of styles, but this is another
> issue. I'll write an email about style to the list its a good time to
> fix it. 
> 
> It would be nice if you used either style 1 or style 2.
> If you want to look at what style 1 looks like you can do :
> #cp filter.c f.c
> #indent -kr -i8 -cs -pcs -psl f.c
> 
> and look at f.c
> 
> 
> <end of comments>
> 
> I am sure more comments will come up, but most of the important stuff is
> here. I just looked at the code, i didn't had time to actually play with
> it. How much testing have you done ? can you write a test program so
> that we can look at it ? 
> We need to move this code to the cvs as soon as we can.
> 
> regards,
> Chema