jmcnamara / rust_xlsxwriter

A Rust library for creating Excel XLSX files.
https://crates.io/crates/rust_xlsxwriter
Apache License 2.0
250 stars 23 forks source link

Bug: `IntoExcelData::write_with_format` should have a free lifetime for `format` argument #85

Closed dodomorandi closed 5 months ago

dodomorandi commented 5 months ago

Current behavior

At the moment the lifetime of format is bound to the lifetime of worksheet in IntoExcelData::write_with_format.

Expected behavior

format should have a free lifetime, in order to detach it from worksheet and the output type. With this change it will be possible to have auto-formatted types.

Sample code to reproduce

use chrono::{NaiveDateTime, Utc};
use rust_xlsxwriter::{ColNum, Format, IntoExcelData, RowNum, Workbook, Worksheet, XlsxError};

struct FormattedDate {
    date: NaiveDateTime,
    format: Format,
}

impl FormattedDate {
    fn new(date: NaiveDateTime) -> Self {
        Self {
            date,
            format: Format::new().set_num_format("dd/mm/yyyy hh:mm AM/PM"),
        }
    }
}

impl IntoExcelData for FormattedDate {
    #[inline]
    fn write(
        self,
        worksheet: &mut Worksheet,
        row: RowNum,
        col: ColNum,
    ) -> Result<&mut Worksheet, XlsxError> {
        let Self { date, format } = self;

        date.write_with_format(worksheet, row, col, &format)
    }

    #[inline]
    fn write_with_format<'a>(
        self,
        worksheet: &'a mut Worksheet,
        row: RowNum,
        col: ColNum,
        format: &Format,
    ) -> Result<&'a mut Worksheet, XlsxError> {
        self.date.write_with_format(worksheet, row, col, format)
    }
}

fn main() -> Result<(), XlsxError> {
    let mut workbook = Workbook::new();
    let worksheet = workbook.add_worksheet();

    let formatted_date = FormattedDate::new(Utc::now().naive_utc());
    worksheet.write(0, 0, formatted_date)?;
    Ok(())
}

### Environment

```text
- `rust_xlsxwriter` version: 0.62
- Cargo.toml dependency line for `rust_xlsxwriter`: `rust_xlsxwriter = { version = "0.62.0", features = ["chrono"] }`

Any other information

No response

jmcnamara commented 5 months ago

Thanks. That is a good suggestion. Format definitely doesn't need a lifetime. It was probably added for some legacy reason that isn't there anymore. I'll review and merge your fix later today.

jmcnamara commented 5 months ago

Fix merged. Thanks.

This is a useful idea. I may add an example like this to the docs.

jmcnamara commented 5 months ago

The PR for this is upstream in v0.63.0. Thanks for the input.