Implement Error trait for cairo::Success

[bilelmoussaoui]

I will send a merge request for this one

[sdroege]

And change it to cairo::Error with only the error cases, and convert to Result<T, cairo::Error> where current cairo::Status::Success maps to Ok and the other things to Err :)

[bilelmoussaoui]

I have a few questions:

• I noticed a lot of ensure_status() which just get the status of X and calls ensure_valid() which itself just checks if the Status is Success or not. I don't think it makes sense to keep any of those around any more, does it?
• the ensure_status() is called at unexpected places sometimes, where we assume we will not have a Result<T, cairo::Error> at all, for example https://github.com/gtk-rs/cairo/blob/master/src/font/font_options.rs#L41. Does it make more sense to just convert to a Result<T, cairo::Error> there as well?
• a bunch of cairo objects exposes a pub fn status(&self) -> Status, does it make sense to still expose that? it might be useful in some weird use cases but doesn't look right in Rust. We should probably remove that and just return a Result whenever something might fail.

My questions are not super clear, sorry for that :)

[sdroege]

* I noticed a lot of `ensure_status()` which just get the status of  X and calls `ensure_valid()` which itself just checks if the `Status` is `Success` or not. I don't think it makes sense to keep any of those around any more, does it?

I guess those could still stay around as Result<(), cairo::Error>... not sure why they exist at all, shouldn't operations that cause things to fail return a Result instead? But that needs some more investigation so I would do the removal of those as a second step.

* the `ensure_status()` is called at unexpected places sometimes, where we assume we will not have a Result<T, cairo::Error> at all, for example https://github.com/gtk-rs/cairo/blob/master/src/font/font_options.rs#L41. Does it make more sense to just convert to a Result<T, cairo::Error> there as well?

I assume creating a FontOptions can fail? As is this is a bit useless, it should either not check it (-> it can't fail) or it should return a Result like you suggest.

* a bunch of cairo objects exposes a `pub fn status(&self) -> Status`, does it make sense to still expose that? it might be useful in some weird use cases but doesn't look right in Rust. We should probably remove that and just return a Result whenever something might fail.

I agree, but same as with the (public!) ensure_status() I think this needs some more research though, same as above.

Does this help?

[bilelmoussaoui]

I'm thinking of doing a first merge request and check the API for each method that might fail and remove the unneeded ensure_status() as a first step and then move the whole thing to use a cairo::Error where it make sense. Does it sounds good to you? thanks for the feedback & help

[sdroege]

Sounds like a plan. Thanks!