Skip to content

Conversation

@JWill392
Copy link
Contributor

Make integration hook extensions case-insensitive, matching built-in formats. Also added some tests to show it failing before the change; not sure if they're useful or follow standards. New to Rust and first time contributing, so any feedback is welcome!

See issue #2659

.extension()
.filter(|ext| !ext.is_empty())
.map(|ext| Format::Extension(ext.to_owned()));
.map(|ext| Format::Extension(ext.to_ascii_lowercase()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Please add a comment on the enum's variant that its contents are supposed to be normalized to ascii lowercase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of lower-casing here, I think it would be better to allow arbitrary casing in Format::Extension and lower-case in ImageReader::make_decoder where casing causes problems.

I'm saying this in the context of #2662 where I added a second code path that creates a Format::Extension.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm good point, that fits the existing pattern of ImageFormat::from_extension better. I don't want to affect that since your PR and issue 2616 might change it.

@JWill392 JWill392 marked this pull request as draft November 25, 2025 17:47
@JWill392 JWill392 marked this pull request as ready for review November 25, 2025 18:29
@197g 197g merged commit f969bd5 into image-rs:main Nov 26, 2025
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants