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.

Sign up for GitHub

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

Jump to bottom

Escape any dollar signs when formatting cell data #4243

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Escape any dollar signs when formatting cell data #4243

wants to merge 3 commits into from

Conversation

sirbaconjr
Copy link

@sirbaconjr sirbaconjr commented Nov 25, 2024 •
edited
Loading

This is:

  • a bugfix
  • a new feature
  • refactoring
  • additional unit tests

Checklist:

  • Changes are covered by unit tests
    • Changes are covered by existing unit tests
    • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change and a link to the pull request if applicable
  • Documentation is updated as necessary

Why this change is needed?

Fix #4242. When a cell is formatted as text and contains a number prefixed with dollar sign, this number is getting replaced with 0. The replacement happens due to the preg_replace function. This commit escapes any sollar signs on the cell data, so those are not replaced.

@sirbaconjr sirbaconjr changed the title Escale any dollar signs when formatting cell data Escape any dollar signs when formatting cell data Nov 25, 2024
@sirbaconjr sirbaconjr mentioned this pull request Nov 25, 2024
Open
8 tasks
@sirbaconjr sirbaconjr force-pushed the fix-dollar-sign-issue-4242 branch 2 times, most recently from a726ff9 to 3af8f4e Compare November 25, 2024 20:59
@sirbaconjr
Ignore cell formatting when the format is a single @
3eee749 
This commit fix two issues that happened when the a cell was formatted as text
* When the cell contains a number prefixed with dollar sign, this number is getting replaced with 0. The replacement happens due to the preg_replace function.
* When the cell contains quotes, the quote would be removed.
@oleibman
Copy link
Collaborator

I finally figured it out - we want special processing for quote marks if they are part of the format, but not if they are part of the value. Updated your code slightly, added a new test case. I think this is what we need, or pretty close if not. I'll continue to think about for a day or two, and then merge.

@oleibman
Copy link
Collaborator

@sirbaconjr We seem to be working at cross purposes. If you have changes you still want to make, please use the current state of the PR as your starting point - commit 4b568b4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oleibman oleibman oleibman left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Number prefixed with dollar sign is replaced when using rangeToArray
2 participants
@sirbaconjr @oleibman