jmcnamara / rust_xlsxwriter

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

Feature request: Centralize worksheet name validation #45

Closed 2ndDerivative closed 1 year ago

2ndDerivative commented 1 year ago

Feature Request

Since I've looked at the code here for the first time I want to suggest that Worksheet name validation gets centralized into one String wrapper struct that can be reused throughout the program.

Since the appropriate errors are already in the XlsxError that wouldn't add any additional overhead and would allow to reuse that validation for ChartRange or Range string validation.

jmcnamara commented 1 year ago

Thanks. Good suggestion. I'll add that.

jmcnamara commented 1 year ago

Also, the following should raise an error:

use rust_xlsxwriter::{Chart, ChartType, Workbook, XlsxError};

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

    let data = [1, 2, 3, 2, 1];
    worksheet.write_column(0, 0, data)?;

    let mut chart = Chart::new(ChartType::Column);
    chart.add_series().set_values("'Sheet1!$A$1:$A$5");
    worksheet.insert_chart(0, 2, &chart)?;

    workbook.save("chart.xlsx")?;

    Ok(())
}
jmcnamara commented 1 year ago

I've moved the worksheet naming checks to utility.rs and now check the worksheet name during the chart.validate() phase. So the sample code above now raises an error:

$ cargo run --example gh45
   Compiling rust_xlsxwriter v0.41.0 (/Users/John/Development/xlsx/rust/rust_xlsxwriter)
    Finished dev [unoptimized + debuginfo] target(s) in 4.98s
     Running `target/debug/examples/gh45`
Error: SheetnameStartsOrEndsWithApostrophe("Sheet name in chart series range: 'Sheet1!$A$1:$A$5")

I've also added a test to ensure that the sheet name exists in the workbook. So this now raises an error:

use rust_xlsxwriter::{Chart, ChartType, Workbook, XlsxError};

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

    let data = [1, 2, 3, 2, 1];
    worksheet.write_column(0, 0, data)?;

    let mut chart = Chart::new(ChartType::Column);
    chart.add_series().set_values("Sheet7!$A$1:$A$5");
    worksheet.insert_chart(0, 2, &chart)?;

    workbook.save("chart.xlsx")?;

    Ok(())
}

Output:

$ cargo run --example gh45
   Compiling rust_xlsxwriter v0.41.0 (/Users/John/Development/xlsx/rust/rust_xlsxwriter)
    Finished dev [unoptimized + debuginfo] target(s) in 3.78s
     Running `target/debug/examples/gh45`
Error: UnknownWorksheetNameOrIndex("Unknown worksheet name 'Sheet7' in chart range 'Sheet7!$A$1:$A$5'")

This would also have caught the apostrophe error since all sheet names are validated by the workbook.

Thanks for the input. Closing.