-
-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add feature for switching between regex dependencies #106
Conversation
Note: There seem to be 19 failed test cases when using the regex-lite crate with these messages: |
Thanks. I'll take a look at the PR once I'm back online on Monday. Overall it looks good.
That may be a deal breaker. I'll need to look at that in more detail. |
03ea59f
to
28686e0
Compare
Great thank you! Also ran cargo fmt and force-pushed because I forgot |
Thanks. Could you add a GitHub action with regex-lite as well so I can see the failures. Use one of the others actions like the Polars test as an example. |
I added it! By removing the \p{Emoji} parts from those 2 regexes only a single test fails: quote_name11::test_quote_name11 |
That is probably fixable. Python Leave it with me until I'm back online next week. |
I had a look at this in more detail and I don't think it is feasible to add Apart from the failing integration test there are a number of tests that fail with basic Unicode support: https://github.com/jmcnamara/rust_xlsxwriter/blob/main/src/utility/tests.rs#L175-L181 So any sheet name in non-English/ASCII will end up being quoted unnecessarily. This is generally a safe failure since Excel will accept an erroneously quoted sheet name in place of an optimally quoted sheetname. However, there may be cases where this causes an Excel read/load error. So I would need to enable the I'll leave this open to see if other people add +1s for the use case and I may add it at some stage with a "Developer Beware" warning. |
@Degubi, in case this discussion comes up again in the future could you add some example size differences between the Regex and Regex-Lite versions, as a reference. Thanks. |
Sure thing @jmcnamara ! |
@jmcnamara Here are some of my results:
Code: https://github.com/jmcnamara/rust_xlsxwriter?tab=readme-ov-file#example Cargo.toml: [package]
name = "excel-size-diff"
version = "1.0.0"
edition = "2021"
[dependencies]
# rust_xlsxwriter = { git = "https://github.com/Degubi/rust_xlsxwriter.git", branch = "feature-regex-lite", default-features = false, features = [ "regex" ] }
rust_xlsxwriter = { git = "https://github.com/Degubi/rust_xlsxwriter.git", branch = "feature-regex-lite", default-features = false, features = [ "regex-lite" ] }
[profile.release]
strip = "symbols"
lto = true
codegen-units = 1 |
Looking at the conversation in #108 and the work done on the #no_regex branch I'm gonna assume that this PR can be closed. |
Implementation for request in issue #105