search

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

Centralize name validation into worksheet::Name struct #46

Closed 2ndDerivative closed 1 year ago

2ndDerivative commented 1 year ago

Regarding issue #45

I added a struct Name in the worksheet module. it's a string wrapper that includes all the logic in set_name, and set_name was pointed to this struct. I took care to not change and public interface so far, and as the field "name" was crate-public, I changed it to an Option<Name> so the validation could stay guaranteed. So far this setup passes all the tests, and any tests concering set_name were kept, so this should work as before.

2ndDerivative commented 1 year ago

Okay fixed it. Accidently added the wrong commit to the PR. Sorry for that. We can squash all of these later

jmcnamara commented 1 year ago

Thanks for the effort but It isn't how I'd want to handle it so I will pass on this.

For future reference keep an eye on failing tests (they are detailed in the Github Actions) and keep the commit history flat and clean.

            XlsxError::SheetnameReserved => {
                write!(
                    f,
                    "\"History\" is a reserved worksheet name"
                )
            }

"History" is a reserved name but not in all language versions of Excel. I had a similar check and exception in the Python version of the library and had to remove it because it is allowed in the Russian version of Excel. See this bug report: https://github.com/jmcnamara/XlsxWriter/issues/688

2ndDerivative commented 1 year ago

yeah I was wondering about the tests. The cargo formatter didn't work on this project because of a file name length or something?