gfx_graphics icon indicating copy to clipboard operation
gfx_graphics copied to clipboard

glyph::Error is not std:error::Error

Open ucarion opened this issue 8 years ago • 5 comments

I noticed this when trying to add error_chain to my project, because the following doesn't compile: (piston_window::Glyphs is gfx_graphics::GlyphCache)

let mut glyphs = Glyphs::new("assets/fonts/FiraSans-Regular.ttf", window.factory.clone())
    .chain_err(|| "Could not create glyphs")?;

The error notes:

   = note: the method `chain_err` exists but the following trait bounds were not satisfied: `piston_window::Error : std::error::Error`

I think this would be fixed if glyph::Error implemented std::error::Error. However, I don't claim to fully understand the situation. Could this implementation be added?

ucarion avatar Aug 06 '17 18:08 ucarion

The C-GOOD-ERR) API guideline from the Rust API guidelines states that "Error types should be meaningful and well-behaved". If an error type does not implement std::error::Error, it will not be usable in other error handling mechanisms and libraries. Also problematic, as a consequence to this, is the fact that it does not implement Debug, thus disabling trivial methods such as unwrap().

Enet4 avatar Nov 27 '17 00:11 Enet4

Notice that this code has moved here https://github.com/PistonDevelopers/graphics/blob/master/src/glyph_cache/rusttype.rs

bvssvni avatar Nov 27 '17 01:11 bvssvni

According to https://github.com/PistonDevelopers/gfx_texture/blob/master/src/lib.rs, it seems error types are reexported from Gfx.

bvssvni avatar Nov 27 '17 01:11 bvssvni

@bvssvni I was looking for the concrete GlyphCache implementation, but the documentation would direct me to a broken link. Thanks! I also found a related issue: gfx-rs/gfx#506

Enet4 avatar Nov 27 '17 10:11 Enet4

Head's up! The issue was created before LL transition. If you are to submit a PR (that would be great!), it needs to go towards pre-ll branch (assuming you need it for gfx-0.16 and up). If it's going to be non-breaking, we can port the PR to v0.16 branch and release an update.

kvark avatar Nov 27 '17 15:11 kvark