feature request: `impl<I: IntoExcelData> IntoExcelData for Option<I>`

[Expurple]

Feature Request

Hi. I have a use case with writing a lot of optional values. Currently, I had to write manual wrappers for handling Options like so:

match opt_value {
  Some(value) => worksheet.write(row, col, value)?,
  None => Ok(worksheet),
};

match opt_value {
  Some(value) => worksheet.write_with_format(row, col, value, format)?,
  None => worksheet.write_blank(row, col, format)?,
};

What do you think about upstreaming it as a trait impl, so that standard write() and write_with_format() can work with Options directly? I only started using this crate yesterday, so I don't know if this would be intuitive (not doing any writes in write()) or consistent with other languages. For me as a newcomer, it would be very intuitive. If you greenlight this, I can attempt implementing this myself.

Alternative

If you don't find this behavior intuitive, maybe we can add separate helper methods for handling Options, something like write_or_blank()?

[jmcnamara]

Thanks for the suggestion.

I'm open to it in principle but I'd like to see some other +1s first and/or to see if anyone else finds this a pain point.

[Expurple]

I checked the Python version, and it follows the same convention already:

None and empty strings "" are written using write_blank().

Should've checked this before posting the issue, this is a good argument in my favor

[Expurple]

And looks like there's already a thumbs up from @2ndDerivative

[jmcnamara]

I've added support for this to main.

You can now use Option in write() for any supported T. For example:

use rust_xlsxwriter::{Format, Workbook, XlsxError};

fn main() -> Result<(), XlsxError> {
    // Create a new Excel file object.
    let mut workbook = Workbook::new();
    let worksheet = workbook.add_worksheet();
    let format = Format::new().set_foreground_color("C6EFCE");

    // Write Option<T> number.
    worksheet.write(0, 0, Some(123))?;

    // Write Option<T> string.
    worksheet.write(1, 0, Some("Ciao"))?;

    // Write Option<T> with format.
    worksheet.write_with_format(2, 0, Some("Bello"), &format)?;

    // Write None but with format. This should write a formatted blank cell.
    worksheet.write_with_format(3, 0, None::<&str>, &format)?;

    // Write None but without format. Should be ignored and shouldn't overwrite the
    // first cell.
    worksheet.write(0, 0, None::<&str>)?;

    // Save the file to disk.
    workbook.save("gh59.xlsx")?;

    Ok(())
}

Output:

Give it a try and let me know how you get on.

[Expurple]

Thank you very much, looks right overall. Although, I should note that write_with_format() becomes an undocumented footgun, because it theory it could also do nothing in case of None. In the docs for Worksheet::write_with_format(), i'd replace

/// - [`Option<T>`]: An `Option<T>` instance where `T` is one of the supported types.

with something explicit like

/// - [`Option<T>`] where `T` is one of the supported types.
///     In case of `None`, [`write_blank()`](Worksheet::write_blank()) is called.
///     If you want to keep the cell unformatted, skip the method call explicitly.

because some users may expect that behavior. Actually, that's what my custom code does, because I wanted to cut down the file size. I "lied" about it in the feature request and suggested write_blank(), because that seems more reasonable in the general case.

I'm not sure what to do for my use case of doing nothing on None. Keep things as is and require explicit pattern matching on the caller side? Add a separate helper method like maybe_write_with_format() (not sure about the name) that only accepts Option<T>? What do you think?

[jmcnamara]

Thank you very much, looks right overall.

Thanks. I'm still not convinced of the utility but I'll leave it in for now. I've improve the docs some more too.

I'm not sure what to do for my use case of doing nothing on None. Keep things as is and require explicit pattern matching on the caller side?

I think that is the best thing for now. If I get a reasonable amount of requests for a different behaviour I'll consider it.

Thanks for the suggestion.