Bug 5676

Created attachment 4413 [details] [review]
Patch to add new Surface Types

A couple things to note about this patch.  I added exception specifications for
all of the Surface API functions (i.e. functionname() throw(std::bad_alloc);). 
I don't know if this is controversial or not, but it seems like the right thing
to do to me.  Perhaps they should just specify the generic 'throw()' without
specifying which exceptions can be thrown?  I can rework it if people think
it's unnecessary.  

Also, I removed the status check from a couple of functions that (according to
my perusal of the cairo source code) don't seem to change the internal status
of the cairo object.

All Surface functions and classes have API documentation in doxygen.

About exceptions:
We have generally avoided this in gtkmm and libxml++ because it seems to make
life more difficult and it requires us to know about every possible exception at
the time that we stabilize the API/ABI. Sorry, but I don't remember all of the
reasoning. The best I can find at the moment is here:
http://marc.theaimsgroup.com/?l=gtkmm&m=101515928306805&w=2
So, for now, I'd like to remove the throw() specifications.

Doxygen has @throws which does the job of documenting it without making extra
demands on the programmer.

>  I removed the status check from a couple of functions that (according to
> my perusal of the cairo source code) don't seem to change the internal status
> of the cairo object.

Is there any way to be sure that these won't change something in future? I'd
like some documentation/confirmation of this from the cairo developers. So far,
Carl Worth has told us that it's probably a good idea to check the status after
every call, in languages where that is viable.

I don't think that get_user_data() should be wrapped at all. It doesn't seem
useful. I think I have documented that elsewhere. If we add it now, even
protected, we can't remove it later.

What does "XXX" mean in comments? Please be more explicit. I use TODO: for
things that need more attention.

Otherwise this is wonderful and should be committed.

(In reply to comment #2)
> http://marc.theaimsgroup.com/?l=gtkmm&m=101515928306805&w=2
> So, for now, I'd like to remove the throw() specifications.

That sounds reasonable, I'll rework it to remove the exception specifications.

> Carl Worth has told us that it's probably a good idea to check the status after
> every call, in languages where that is viable.

ok.  Then I can easily put them back in.  There should be a fairly negligible
performance impact in any case since they're inlined, and there are only a
couple functions where it even applies.  

> I don't think that get_user_data() should be wrapped at all. It doesn't seem
> useful. I think I have documented that elsewhere. If we add it now, even
> protected, we can't remove it later.

I agree that it's not particularly useful at this point, but I didn't add these
functions.  They were previously wrapped in the public API, I just moved them to
protected.  Should I remove them altogether?  What are your expectations for API
stability for cairomm at this point?  

> They were previously wrapped in the public API, I just moved them to
> protected.  Should I remove them altogether? 

Yes, I think that would be best. Thanks.

>  What are your expectations for API stability for cairomm at this point?  

Let's try not to make unnecessary breaks, particularly when we know that an
application is using the API. But I don't plan to officially freeze the API
until gtkmm 2.9/2.10 freezes it's API, around the time that GTK+ does, around
May 2006:
http://www.gtk.org/plan/2.10/

Only at that time are people likely to really look at the API and tell us about
problems.

Created attachment 4421 [details] [review]
Revised patch with changes noted above

- removed exception specifications
- removed *_user_data() functions
- added back the check_status_and_throw_exception() functions for functions
where it was commented out

I still haven't heard back about my CVS account request, so I'm not able to
commit this myself yet.

Committed. Well done.