[gnome-print] Filtering code



Hello Scott,

Your work for the filtering stuff is great, excellent work. 

I started integrating your filter work into gnome-print and did some
modifications to it to GNOMEfy it. As such the code worked fine and
there isn't anything wrong about not beeing GNOMEfied from a code
perspective it works fine, but having a consistent way of writing code
makes GNOME a lot easier to understand. I got a bit carried away but
decided tonight to give you what I currently have so you can continue
working on it.

This are some of the GNOMEisms I changed.

  * #ifdef __GNOME_PRINT_FOO_H__ v.s. #ifdef FOO_H in headers
  * Structures are NamedLikeThis rather_than_small_letters
  * use gchar rather than char (this is changes on some GNOME
    modules, but i like to use the gvariants more)
  * Use of constant arguments whenever possible, this clears up
    a lot who owns that piece of memory
  * Indentation. 
  * The headers are enclosed inside G_BEING_DECLS and G_END_DECLS, this
    is not a problem right for headers that are not installed but if we
    had C++ code inside gnome-print (Think about OMNI link) this might
    cause problems, it is a general good idea to just wrap the .h files
    around them.
  * We define variable names when prototyping functions. The struct
    inside  gnome-print-filter.h should have variable names like :
    "(*foo) (gint a, gchar b)"
    It helps a lot when trying to understand what the code is doing to
    have a meaningful name to attach to the parameters
  * functions that create a new structure/object for you are named _new,
    init  functions usually operate on a structure already created. So
    the _new function might call _init on the object to load it with
    default values.
  * Use gboolean values where applicable, although booleans are really
    just ints it helps a lot because whoever is reading the source code
    thinks about it as a boolean and you can assume that you will only
    have a TRUE or FALSE value in there
  * We have a standard way of documenting functions, this allows us to
    create documentation from source code comments. For example, the gtk
     API documentation as seen here:
     http://developer.gnome.org/doc/API/2.0/gtk/gtk-gtktreemodel.html
     is created by functions being documented with a standard template,
     we have a emacs  macro to add automatically the template for
     functions. If you use emacs you can add a macro
     for it, search for "gnome-doc-insert" in:
     http://www.gnome.org/~chema/.emacs
   * Use g_return_val_if_fail to check arguments passed to functions,
     the main reason is that this macro will emit a useful warning if
     this happens, if you simply return a -1 you might never discover
     the offending code that is calling the function incorrectly. This
     macros  also print the file and line number that the assertion
     failed. For example, in the setsrt
     function we now check if the Filter has a setstr function, if some
     code is calling setstr with a filter that does not have a setstr
     function, having a warning on the console is going to allow is us
     to see this as soon as we make the mistake, also finding the bug is
     going to be very easy because you know something is wrong and where
     it is happening.
   * structure member naming. The names we give to the members of the
     structures is very important, we have to be as clear as we can, so
     ->set_string is prefered to ->setstr.
   * We usualy don't add const to structures when calling a method. In
     filter->filter you had the first  argument as const struct 
      gnome_print_filter_type. The reason for it is that the const means
     the   memory pointed by the pointer will not change. For example,
      if you call a fitler method with   a Hex encode filter, 
       gnome_print_filter_hexenc_filter is modifying the filter  
      (d->outbuffsize, d->linepos, d->outbuf are assigned inside)
   * GnomePrintFitlerType should really be GnomePrintFilterClass, there
     wasn't a clear line on what they instance of a filter was and
     its class.

Here is my current patch for it, You can download it and continue
working on this. 

I didn't finished polishing the rle filter, but added some comments in
the code. I didn't tested the filter code for hex & rle, it was in
"compiles" state ;-). I don't think that i screwed anything up but I
can't bet my life on it. You'll find the patch unfinished and with some
leftover code too which we need to clean before getting comiting it.

On the short term this is what I'd like to do:

- Only get 3 filters working: hex, rle, & flate. flate is the most
important one because the objects and classes might need tweaking to get
this filter to work. Forget about the other filters for now lets get the
structs and API nailed first.
- We need to make the filters work both ways, encode and decode to make
the test suite reliable. Plus we already had code for this in the 1.4
branch.
- Forget about the _set_pointer and _set_string functions for now,
hardcode those values. We will use GPANodes later for it. For now, only
make it so that one filter works and it is passed as an id by the rest
of gnome-print
- In the testsuite code, I'd like to decode files and decode them rather
than having known stuff. This makes our testing code scale, we don't
need to add more files when we port more filters.
- Move as much of the functionality to gnome-print-filter.c as possible
for the filters to use (already did some of this)
- We might want to use a macro to reduce the boilerplate code. See:
gnumeric/src/sheet-object-widget.c , not a high priority right now
thought.

Why did you used size_t for needed bytes instead of gint? I changed it
to gint but have the feeling it should really be size_t.

Excellent work ! Thanks a lot,
Chema





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]