Skip to content
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

Only remove the unescape prefix character when parsing #131

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

jgmchan
Copy link
Contributor

@jgmchan jgmchan commented Sep 18, 2024

This PR addresses

  • Currently the unescape_formulas option when parsing will remove any first character of a cell as long as the second character is part of the @escape_formula_start. This causes issues when the data is something like this:

    "A-1","B+2","C=3","'@D"

    which should parse to this:

    ["A-1", "B+2", "C=3", "@D"]

    but parses to this instead:

    ["-1", "+2", "=3", "@D"]

    This change will check that the first character is actually the escape character we want before removing it.

  • Slight fix up of the documentation

* Currently the `unescape_formulas` option when parsing will remove any
  first character of a cell as long as the second character is part of the
  `@escape_formula_start`. This causes issues when the data is something
  like this:

  ```
  "A-1","B+2","C=3","'@d"
  ```

  which should parse to this:

  ```
  ["A-1", "B+2", "C=3", "@d"]
  ```

  but parses to this instead:

  ```
  ["-1", "+2", "=3", "@d"]
  ```

  This change will check that the first character is actually the escape
  character we want before removing it.

* Slight fix up of the documentation
@jgmchan
Copy link
Contributor Author

jgmchan commented Sep 24, 2024

The failing tests doesn't seem to be related to my change and I can't figure out how to kick it off again. Any ideas @beatrichartz ?

@beatrichartz beatrichartz merged commit e6a7943 into beatrichartz:main Jan 2, 2025
0 of 17 checks passed
@beatrichartz
Copy link
Owner

Looking good, merged. Thanks!

@beatrichartz
Copy link
Owner

Published in 3.2.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants