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

Implemented display_some and display_some_or #1014

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vallentin
Copy link
Collaborator

@vallentin vallentin commented Apr 21, 2024

Resolves #1007

Questions:

  • Should we #[doc(hidden)] DisplaySome and DisplaySomeOr?
  • Should we move them into a separate module? (Maybe #[doc(hidden)] that?)
  • Maybe even expand it to also hide the JSON related types in json.rs

Alternatively, we could also turn DisplaySome private, and then use:

fn display_some(...) -> askama::Result<impl fmt::Display + 'a>

}
}

pub fn display_some<T>(value: &Option<T>) -> Result<DisplaySome<'_, T>>
Copy link
Owner

Choose a reason for hiding this comment

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

Please put the filter functions before the types that need it.

Can we make DisplaySome and DisplaySomeOr private and yield impl Display from the filter functions?


[#display_some]: #display_some

The `display_some` filter is essentially a shorthand for:
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: I think we don't need the "a" before "shorthand".

@Kijewski
Copy link
Contributor

Kijewski commented Apr 28, 2024

I think we can omit the explicit lifetimes:

pub struct DisplaySome<T>(Option<T>);

impl<T: fmt::Display> fmt::Display for DisplaySome<T> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        if let Some(val) = &self.0 {
            write!(f, "{val}")?;
        }
        Ok(())
    }
}

/// See [`display_some` in the Askama book] for more information.
///
/// See also [`display_some_or`].
///
/// [`display_some` in the Askama book]: https://djc.github.io/askama/filters.html#display_some
pub fn display_some<T: fmt::Display>(value: &Option<T>) -> Result<DisplaySome<&T>> {
    Ok(DisplaySome(value.as_ref()))
}

pub enum DisplaySomeOr<T, U> {
    Value(T),
    Otherwise(U),
}

impl<T: fmt::Display, U: fmt::Display> fmt::Display for DisplaySomeOr<T, U> {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            Self::Value(val) => write!(f, "{val}"),
            Self::Otherwise(val) => write!(f, "{val}"),
        }
    }
}

/// See [`display_some_or` in the Askama book] for more information.
///
/// See also [`display_some`].
///
/// [`display_some_or` in the Askama book]: https://djc.github.io/askama/filters.html#display_some_or
pub fn display_some_or<T: fmt::Display, U: fmt::Display>(
    value: &Option<T>,
    otherwise: U,
) -> Result<DisplaySomeOr<&T, U>> {
    Ok(value
        .as_ref()
        .map_or(DisplaySomeOr::Otherwise(otherwise), DisplaySomeOr::Value))
}

@djc djc mentioned this pull request May 7, 2024
7 tasks
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.

Add display_some filter and friends
3 participants