image-rs / image-png

PNG decoding and encoding library in pure Rust
https://docs.rs/png
Apache License 2.0
357 stars 140 forks source link

Validate provided info in Encoder::with_info #442

Closed fintelia closed 9 months ago

fintelia commented 9 months ago

This would normally be a breaking change, but the Encoder::with_info API hasn't yet been included in a release.

hoijui commented 9 months ago

Good call! I now imagine something like Info::validate(&self) -> Result<(), ...>, which is called from withing Encoder::with_info(). What I do not understand though, is how to prevent a lot of code duplication between this validate function and all the set_* functions in Encoder, as the former would validate all at once, and the later validates parts separately. Would we want to have separate Info::validate_* functions as well, and call the appropriate one from each Encoder::set_* method, and also call all of them in sequence from Info::validate()?

hoijui commented 9 months ago

ouh you already did this in code.. sorry, did not see it is a PR ups. ... Is that already all the validation necessary?

fintelia commented 9 months ago

This is all the validation we were already doing in other cases. Not sure if any further validation makes sense