Skip to content

feat: impl Display for FilterType #3071

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.

Sign up for GitHub

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

Open
Its-Just-Nans wants to merge 3 commits into
base: main
Choose a base branch
from
+12 −0
Open

feat: impl Display for FilterType #3071

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
813e043
feat: impl Display for FilterType
Its-Just-Nans Jul 17, 2026
1f858f2
fmt
Its-Just-Nans Jul 17, 2026
7c6ec19
Update sample.rs
Its-Just-Nans Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/imageops/sample.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,18 @@ pub enum FilterType {
Lanczos3,
}

impl std::fmt::Display for FilterType {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
FilterType::Nearest => write!(f, "Nearest"),
FilterType::Triangle => write!(f, "Triangle"),
FilterType::CatmullRom => write!(f, "CatmullRom"),

@197g 197g Jul 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There's different requirements here, end-User display implementations should prefer plain english and avoid code formatting. We might certainly consider if Display is applicable but making it work like Debug is not going to happen as I find that simply misleading. Since I can't really convince myself of any particular capitalization, I'm afraid that FilterType has too narrow a scope for implementing Display.

The OsString::display() method demonstrates another alternative to be considered (not done, considered!). Maybe there's context in which all the above ambiguity is resolved and an expected (english) representation exists?

@Its-Just-Nans Its-Just-Nans Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay I've edited the MR, feel free to close it if needed

FilterType::Gaussian => write!(f, "Gaussian"),
FilterType::Lanczos3 => write!(f, "Lanczos3"),
}
}
}

/// A Representation of a separable filter.
pub(crate) struct Filter<'a> {
/// The filter's filter function.
Expand Down
Loading