Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking issue: Converting error representations #1134

Closed
20 tasks done
HeroicKatora opened this issue Feb 7, 2020 · 3 comments · Fixed by #1205
Closed
20 tasks done

Tracking issue: Converting error representations #1134

HeroicKatora opened this issue Feb 7, 2020 · 3 comments · Fixed by #1205

Comments

@HeroicKatora
Copy link
Member

HeroicKatora commented Feb 7, 2020

The 0.23 brought a more detailed and strictly typed error representation. We kept an internal compatibility interface here to reduce the rework effort required. The plan is to incrementally port these to use more of the new, shiny features. For a basic example, see the png module. To help, claim one of the image formats by a dedicated tracking PR or issue and rework its decoder and encoder implementation or claim a legacy constructors and deprecate and remove it.

Formats:

Constructors:

@nabijaczleweli
Copy link
Member

nabijaczleweli commented Apr 12, 2020

Does this solution make sense? (diff based on 3a94fc9)

diff --git a/src/dynimage.rs b/src/dynimage.rs
index 4415fe6e..4163e6e6 100644
--- a/src/dynimage.rs
+++ b/src/dynimage.rs
@@ -758,7 +758,9 @@ impl DynamicImage {
             }
 
             image::ImageOutputFormat::Unsupported(msg) => {
-                Err(ImageError::UnsupportedError(msg))
+                Err(ImageError::UnsupportedError(UnsupportedError::from_format_and_kind(
+                    ImageFormatHint::Unknown,
+                    UnsupportedErrorKind::Format(ImageFormatHint::Name(msg)))))
             },
 
             image::ImageOutputFormat::__NonExhaustive(marker) => match marker._private {},
diff --git a/src/image.rs b/src/image.rs
index d931ed1d..d45b77a6 100644
--- a/src/image.rs
+++ b/src/image.rs
@@ -128,10 +128,7 @@ impl From<ImageFormat> for ImageOutputFormat {
             #[cfg(feature = "farbfeld")]
             ImageFormat::Farbfeld => ImageOutputFormat::Farbfeld,
 
-            f => ImageOutputFormat::Unsupported(format!(
-                "Image format {:?} not supported for encoding.",
-                f
-            )),
+            f => ImageOutputFormat::Unsupported(format!("{:?}", f)),
         }
     }
 }

nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 12, 2020
"Image too big" condition now Decoding instead of Unsupported, like BMP

Ref: image-rs#1134
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 12, 2020
@HeroicKatora
Copy link
Member Author

HeroicKatora commented Apr 12, 2020

That should work. It's also more in line with how the variant was supposed to work.

nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 12, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 12, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 12, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 13, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 13, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 13, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 13, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 13, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 13, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 17, 2020
…from PNM decoder

Also extract ArbitraryTuplType::name() and replace PNMHeader::write()'s
Arbitrary case with that + a simple wrapper

Ref: image-rs#1195
Ref: image-rs#1134
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 18, 2020
…from PNM decoder

Also extract ArbitraryTuplType::name() and replace PNMHeader::write()'s
Arbitrary case with that + a simple wrapper

Ref: image-rs#1195
Ref: image-rs#1134
Fixes image-rs@e6f4038#r38569558
Closes image-rs#1202
@nabijaczleweli
Copy link
Member

nabijaczleweli commented Apr 18, 2020

Status with #1203:

P:\Rust\image\src>git grep :UnsupportedError
tga/decoder.rs:            return Err(ImageError::UnsupportedError(
tga/decoder.rs:            return Err(ImageError::UnsupportedError(
tga/decoder.rs:                return Err(ImageError::UnsupportedError(
tga/decoder.rs:                return Err(ImageError::UnsupportedError(format!(
tiff.rs:            tiff::TiffError::UnsupportedError(desc) => {
tiff.rs:            tiff::TiffError::UnsupportedError(desc) => {

P:\Rust\image\src>git grep :FormatError
flat.rs:            Error::NormalFormRequired(form) => ImageError::FormatError(
tiff.rs:            err @ tiff::TiffError::FormatError(_) => {
tiff.rs:            err @ tiff::TiffError::FormatError(_) => {

P:\Rust\image\src>git grep :UnsupportedColor
dynimage.rs:        _ => return Err(ImageError::UnsupportedColor(color_type.into())),
farbfeld.rs:            return Err(ImageError::UnsupportedColor(color_type.into()));
flat.rs:            Error::WrongColor(color) => ImageError::UnsupportedColor(color.into()),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::L1)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::La1)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgb1)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgba1)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::L2)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::La2)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgb2)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgba2)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::L4)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::La4)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgb4)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Rgba4)),
png.rs:                return Err(ImageError::UnsupportedColor(ExtendedColorType::Unknown(bits as u8))),
png.rs:            _ => return Err(ImageError::UnsupportedColor(color.into())),

Untagging PNG, TIFF, and PNM.
Adding FFI and farbfeld.

nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 18, 2020
…from PNM decoder

Also extract ArbitraryTuplType::name() and replace PNMHeader::write()'s
Arbitrary case with that + a simple wrapper

Ref: image-rs#1195
Ref: image-rs#1134
Fixes image-rs@e6f4038#r38569558
Closes image-rs#1202
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 18, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 18, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 18, 2020
nabijaczleweli added a commit to nabijaczleweli/image that referenced this issue Apr 19, 2020
@HeroicKatora HeroicKatora moved this from Cleanup to Done in Version 0.23 Jun 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Version 0.23
  
Done
Development

Successfully merging a pull request may close this issue.

2 participants