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

added more test cases for 渺、埃、尘 (塵)、沙、纤(纖)、微 #442

Merged
merged 8 commits into from
Dec 31, 2019
Merged

Conversation

kaiyuan01
Copy link
Contributor

only test cases for 纤 and 尘 failed. If we support simplified Chinese, we may change the code for compiler (their traditional Chinese character versions work as expected)

@kaiyuan01
Copy link
Contributor Author

The test/build will fail as the current compiler does not support simplied Chinese characters 纤 and 尘. If we support simplified Chinese, we may change the code for compiler (their traditional Chinese character versions work as expected). - to fix this we may have to change the main source code - TDD (test driven development)

@LingDong-
Copy link
Member

LingDong- commented Dec 31, 2019

Hi @kaiyuan01,
Thanks for the PR. Please notice that support for Simplified Chinese characters are not currently under consideration :P

@kaiyuan01
Copy link
Contributor Author

I agree that at the beginning we may only support traditional Chinese, but to reach out to the max (that's the only reason we'll do that IMHO), we may consider adding simplified Chinese support over the long run, gradually, - so I made the change to src/hanzi2num.js and now the test cases pass for both 纤 and 尘.

@kaiyuan01
Copy link
Contributor Author

Another main reason why we wanted to include those characters is that they also show up in ancient books. For example, 尘 is also in Kang Xi Zi Dian:
image

Plus: 尘 is also in Zi Hui Bu:

【字彙補】古文塵字。註詳土部十一畫。

If we don't include those characters as simplified Chinese characters per se, but we may have to consider adding them simply because they are Yi Ti Zi (异体字/異體字) for those traditional Chinese characters. I don't think our work is complete without including those 異體字.

@LingDong-
Copy link
Member

LingDong- commented Dec 31, 2019

Hi @kaiyuan01,

I totally understand and acknowledge that 異體字 are integral parts of the Chinese language, and I'm fully aware of their usage throughout history in books, in carvings, in calligraphy, etc. However, we do not intend to support them, not because that I have any prejudice (or that I'm too lazy), it is simply about having one standard and sticking to it.

Just like the fact that in English, many phrases has similar meanings to if, such as IF, whether, whatIf, whenThisHappens, checkThisCondition, letsSee, whetherOrNot, 💁 etc. etc. NO language that I know of supports them simultaneously. It is simply picking 1 for the language and sticking to it. After all, this is what keywords are meant for.

Even if we are going to support character variations in the end (nope), we will do it in one-shot to avoid any confusion, instead of starting the process gradually. If you really need Simplified characters badly, please check out #440 , you can now add macros to map any characters to the standard ones.

I'm deeply sorry for not being clear enough in the first place, having you done all this work. Meanwhile, there are a lot of other things you could contribute to. For example, check out help-wanted tags in issue section. Thank you for your understanding.

@LingDong- LingDong- closed this Dec 31, 2019
@kaiyuan01
Copy link
Contributor Author

Understood. You may re-open it as the other test cases are still valid. - I added other additional test cases as well

@LingDong- LingDong- reopened this Dec 31, 2019
@kaiyuan01
Copy link
Contributor Author

Tested successfully locally, not sure why the snapshot did not pass. - maybe it error'ed out from other changes not included in this PR?

@LingDong- LingDong- merged commit 9f5cd40 into wenyan-lang:master Dec 31, 2019
@LingDong-
Copy link
Member

@kaiyuan01 , I think you're right, the new errors are due to another stdlib I added just a while ago that involves the DOM which apparently the tests don't have access to.
Thank you very much for the new tests! Your help is very much appreciated.

@antfu antfu mentioned this pull request Dec 31, 2019
LingDong- added a commit that referenced this pull request Jan 20, 2020
added more test cases for 渺、埃、尘 (塵)、沙、纤(纖)、微
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