Skip to content

Re-implement ASC function and add unit tests #4513

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

internauticos
Copy link

This commit re-implements the ASC spreadsheet function, which converts full-width (double-byte) characters to their corresponding half-width (single-byte) characters. This work is being recommitted to a new branch to incorporate recent updates from the master branch.

The implementation resides in the TextData class, and the Calculation class has been updated to delegate calls to ASC to this implementation.

Unit tests have been included in TextDataTest.php to cover various scenarios:

  • Full-width alphanumeric characters.
  • Full-width symbols.
  • Mixed full-width and half-width characters.
  • Empty strings.
  • Boolean and numeric inputs.
  • Japanese Katakana and Hiragana characters.

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?

Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.

This commit re-implements the `ASC` spreadsheet function, which converts full-width (double-byte) characters to their corresponding half-width (single-byte) characters. This work is being recommitted to a new branch to incorporate recent updates from the master branch.

The implementation resides in the `TextData` class, and the `Calculation` class has been updated to delegate calls to `ASC` to this implementation.

Unit tests have been included in `TextDataTest.php` to cover various scenarios:
- Full-width alphanumeric characters.
- Full-width symbols.
- Mixed full-width and half-width characters.
- Empty strings.
- Boolean and numeric inputs.
- Japanese Katakana and Hiragana characters.
@oleibman
Copy link
Collaborator

I thought I posted this earlier, but I don't see it. So I'll try again. I assume you will be able to straighten out your change so that it only changes a few members instead of the 3,121 (!) this affects. But, before you do, I'm interested in one of your test cases.

['カタカナ', '=ASC("カタカナ")'], // Full-width Katakana to half-width

Here's what I see when I try this in Excel:
image

The output seems to match the input; it hasn't been changed from full-width to half-width. There may be many possible explanations for why this doesn't agree with your test case, including font or system options. Do you have any explanation for why my Excel doesn't match your expectation?

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

Successfully merging this pull request may close these issues.

2 participants