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

Add ImageFormat::from_extension #1361

Merged
merged 3 commits into from Nov 15, 2020

Conversation

a1phyr
Copy link
Contributor

@a1phyr a1phyr commented Nov 14, 2020

I also moved the inner non-generic function form io::free_functions and removed the intermediate PathError enum.

I license past and future contributions under the dual MIT/Apache-2.0 license,
allowing licensees to chose either at their option.

@a1phyr
Copy link
Contributor Author

a1phyr commented Nov 14, 2020

Not-so-related note: I think it would make more sense for ImageFormatHint::PathExtension to contain an OsString instead of a PathBuf, as it stores a mere extension.

This would be a breaking change, however.

Copy link
Member

@HeroicKatora HeroicKatora left a comment

Choose a reason for hiding this comment

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

👍 on moving the method and making the error handling more concise. And for homomorphizing the body of the conversion. One small nit on the interface.

src/image.rs Outdated Show resolved Hide resolved
@HeroicKatora
Copy link
Member

I think it would make more sense for ImageFormatHint::PathExtension to contain an OsString instead of a PathBuf, as it stores a mere extension.

That would indeed be breaking, but adding a new variant shouldn't be.

src/image.rs Show resolved Hide resolved
@HeroicKatora HeroicKatora merged commit 085300e into image-rs:master Nov 15, 2020
@HeroicKatora
Copy link
Member

Nice, thank you!

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.

None yet

3 participants