impl std::fmt::Display for CellValue

[piccoloser]

It's happened on a few occasions that I've needed to reference the value of a cell.
Would implementing std::fmt::Display for CellValue be a good idea?

A quick example of a time I've had to do this is when writing a program to format report outputs, one of which required the name of each sheet to be named after the value of a column value (eg. format!("PO Line {}", line_no); )

This solution to the issue was provided to me, in the form of a function, by a user on Reddit:

fn display_cellvalue(cellvalue: &CellValue) -> impl fmt::Display + '_ {
    struct Wrapper<'a>(&'a CellValue);
    impl<'a> fmt::Display for Wrapper<'a> {
        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
            match &self.0 {
                CellValue::Bool(b) => write!(f, "{}", b),
                CellValue::Number(n) => write!(f, "{}", n),
                CellValue::String(s) => write!(f, "{}", s),
                CellValue::Blank(_) => write!(f, ""),
                CellValue::SharedString(s) => write!(f, "{}", s),
            }
        }
    }
    Wrapper(cellvalue)
}

Calling this function and passing a reference to a CellValue will return the value in a printable form.

[dodomorandi]

I have a small objection the implementation proposal: returning an opaque type makes it unstorable unless boxed. Moreover, in this way the return type does not implement traits that could be useful in niche cases (like Copy). On the other hand, being an opaque type make it resilient to breaking changes, but in this case I don't think that it's worth.

TL;DR: I think that returning a concrete CellValueDisplay<'a> instead of an impl fmt::Display would be better.

[piccoloser]

TL;DR: I think that returning a concrete CellValueDisplay<'a> instead of an impl fmt::Display would be better.

I agree with your point here; the code provided worked for me, but would definitely need to be adjusted for general use.