-
Notifications
You must be signed in to change notification settings - Fork 52
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
Reduce all duplicated spaces #211
base: master
Are you sure you want to change the base?
Reduce all duplicated spaces #211
Conversation
7e9950b
to
06f65f7
Compare
Improve the subject of Git commit message. At least, summarize the work first. |
TEST_F(TestBopomofoUtil, ReduceDuplicatedSpacesInsideBopomofo) | ||
{ | ||
ASSERT_EQ(0, QString::compare("ㄘㄜˋ ㄕˋ", BopomofoUtil::normalize("ㄘㄜˋ ㄕˋ"))); | ||
ASSERT_EQ(0, QString::compare("ㄘㄜˋ ㄕˋ", BopomofoUtil::normalize("ㄘㄜˋ ㄕˋ"))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this test also including the test of replacing "一" to "ㄧ" and "丫" to "ㄚ"?
This commit cover this feature, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nop, this PR is just moving the original part of replacing ambugity words into a sperated file and adding the reducing duplicated spaces function.
But, yes, we still need tests for replacing ambugity words, that's why I move it. After this PR, I'll submit another PR for the tests.
Fixed chewing#204. Move `testBopomofo` method from UserphraseModel into BopomofoUtil for testing.
06f65f7
to
22a954f
Compare
Fixed #204, replacing all duplicated spaces (included single tab and tabs) into single space.
I move
testBopomofo
method fromUserphraseModel
into a new utilBopomofoUtil
, IMO, this function is not that related to the model.And we can create tests for this more easily (without including the main program).
cc @jserv @czchen