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

Added german "sharp s" ß #31

Merged
merged 2 commits into from
Nov 24, 2022
Merged

Added german "sharp s" ß #31

merged 2 commits into from
Nov 24, 2022

Conversation

gollenia
Copy link
Contributor

@gollenia gollenia commented Nov 7, 2022

No description provided.

@tyxla
Copy link
Owner

tyxla commented Nov 13, 2022 •
edited
Loading

Hey @gollenia, thanks for the contribution!

Would you mind adding a unit test or updating the existing one to cover ß?

Noting that this is a duplicate of #21 but I'm happy to merge the first one that has a unit test and is good to go.

@tyxla tyxla mentioned this pull request Nov 13, 2022
Closed
@gollenia
Copy link
Contributor Author

Hi, I added the unit test - is that ok so far?

@tyxla
Copy link
Owner

tyxla commented Nov 22, 2022

Hi @gollenia this looks great!

One last thing before we ship it - could you add the ß to the test with all characters in it? We try to maintain a list of all supported characters in that test.

Thanks a ton for your time!

@gollenia
Copy link
Contributor Author

sure, no thing!

tyxla
tyxla approved these changes Nov 24, 2022
View reviewed changes
Copy link
Owner

@tyxla tyxla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a bunch, @gollenia 🙌

@tyxla tyxla merged commit 4b147a0 into tyxla:master Nov 24, 2022
@tyxla tyxla mentioned this pull request Nov 25, 2022
Open
@michaelkirk michaelkirk mentioned this pull request Jun 1, 2023
Closed
@tyxla tyxla mentioned this pull request Jun 27, 2023
Closed
michaelkirk added a commit to michaelkirk/remove-accents that referenced this pull request Jul 11, 2023
@michaelkirk
Revert "Added german "sharp s" ß (tyxla#31)"
02c18a9 
This reverts commit 4b147a0.
@michaelkirk michaelkirk mentioned this pull request Jul 11, 2023
Merged
tyxla pushed a commit that referenced this pull request Jul 24, 2023
@michaelkirk
Restore eszett ß - which is not accented. (#44)
365d297 
* Revert "Added german "sharp s" ß (#31)"

This reverts commit 4b147a0.

* add regression test for ß

* fixup! add regression test for ß

I forgot to call `t.end()` 🤦

The now outdated "ß" -> "ss" was added to the "remove accents from string" test case as part of 1fe0b90

It seems like maybe the misunderstanding was that the string contained
every to-be-sanitized character, but that's not true. Since ß now has
it's own unit test, I've removed it from the "remove accents from
string" test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tyxla tyxla tyxla approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gollenia @tyxla