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

[Chema Celorio <chema ximian com>]

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

[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