gEDA-dev: Patrick's patchsets

[Peter TB Brett <peter-at-NOSPAMpeter-b_dot_co_dot_uk>]

Patrick,

Could you please confirm that the following patches don't rely on any changes 
to the way selection lists or bounds calculation work:

  * libgeda.cleanup3.patch (o_text_rotate_world() interface consistency)
  * gschem.cleanup1.patch  (add XOR drawing functions)
  * gschem.cleanup2.patch  (clean up unused o_complex_mirror2() function)

I recommend the above for application (if Ales approves).

----

These patches seem to be to do with bounds calculation/coordinates:

  * libgeda.cleanup1.patch (get_*_bounds() takes OBJECT* as argument)
  * libgeda.cleanup2.patch (general bounds calculation changes)
  * libgeda.cleanup4.patch (coordinates calculation changes)
  * gschem.cleanup3.patch  (general bounds calculation changes)

I don't think libgeda.cleanup1.patch is good.  In order for the functions not 
to break:

  1) The object must not be NULL
  2) The object must actually be of the right type
  3) The object's internal pointer must not be NULL

This would be fine if those were functions only accessible by libgeda (1 & 2 
*should* be checked by the calling function), but they're being exported in 
the public headers, so they *all* ought to do full NULL and type checking.  
Currently, they don't do any.  

I recommend that these patches are put on hold, and noscreen is merged 
instead, since that properly rationalizes the way libgeda/gschem handle 
coordinates.  Although the interface changes in this patchset are sensible, I 
think they should wait for now.

----

This patch seems to be to do with transformations of objects:

  * libgeda.cleanup5.patch (new transformation system)

This patch seems like a good idea, but it seems to use the above selection 
system interface changes.  Would it be possible to adapt?  I'd like this to 
be merged, because it makes the way transformations are handled *much* nicer.

----

All your changes seem like good ones (of course :P ).  FWIW I agree with you 
that the GList implementation of selection lists should be hidden behind an 
opaque interface.  In particular, it would be nice for code in gschem to be 
able to get notification whenever the selection changes, perhaps by some sort 
of gobject signal/closure mechanism, and letting code access the GList in the 
raw API causes problems in that respect.  However, I think that the work done 
by Carlos in stripping out uses of the non-GList mechanism is useful and 
shouldn't just be discarded.  Getting rid of explicit uses of the selection 
list as a GList where it shouldn't be should be as simple as changing 
libgeda's struct.h and hacking until the build stops being broken.

Regards,

Peter



-- 
Fisher Society committee                    http://tinyurl.com/o39w2
CUSBC novices, match and league secretary   http://tinyurl.com/mwrc9
CU Spaceflight                              http://tinyurl.com/ognu2

v3sw6YChw7$ln3pr6$ck3ma8u7+Lw3+2m0l7Ci6e4+8t4Gb8en6g6Pa2Xs5Mr4p4
  hackerkey.com                                  peter-b.co.uk

PGP signature

_______________________________________________
geda-dev mailing list
geda-dev@moria.seul.org
http://www.seul.org/cgi-bin/mailman/listinfo/geda-dev