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

Fix unexpected candidate disorder for '1'. #153

Merged
merged 2 commits into from Aug 2, 2014
Merged

Conversation

@shaform
Copy link
Contributor

@shaform shaform commented Jul 11, 2014

This is related to 64b45c5.
Originally, all symbols that do not have alternative symbol candidates would be assigned a '1' for symbolKeyBuf. And when trying to open a candidate window for these symbols, HaninSymbolInput would be called. However, SymbolChoice does not handle this situation correctly and causes some problems. That is, a new symbol would be inserted without changing the original symbol.

The previous commit hides this problem, but when the candidate symbol is '1', this problem resurfaces.

This should close #44.

@czchen czchen added this to the v0.4.1 milestone Jul 13, 2014
@czchen czchen added the bug label Jul 13, 2014
@czchen
Copy link
Member

@czchen czchen commented Jul 13, 2014

Thank for the contributions. However, please help to add unit test for this scenario, thanks.

@czchen
Copy link
Member

@czchen czchen commented Jul 13, 2014

Is this related to #44?

@shaform
Copy link
Contributor Author

@shaform shaform commented Jul 13, 2014

@czchen Thanks for the reply. Yes, this is exactly #44.
BTW, there are two approaches here. One is like this PR, disable candidate select completely when no alternative symbols available. Another would be to make SymbolChoice update the symbol correctly.
However, since it doesn't make sense to do this specifically for '1', maybe we should update chewingutil.c#358 as well, and change '1' to something that would not collide with actual symbols if we choose approach 2.
Which one do you think is better?

@shaform
Copy link
Contributor Author

@shaform shaform commented Jul 13, 2014

That would be something like this? shaform/libchewing@830c951ece7d593606ac2ea2a84b42770f22c77b

@czchen
Copy link
Member

@czchen czchen commented Jul 15, 2014

Yes, test case in https://github.com/shaform/libchewing/commit/830c951ece7d593606ac2ea2a84b42770f22c77b looks fine.

As for approach 1 & 2, I do not have any option as long as there is a test case for it.

@shaform
Copy link
Contributor Author

@shaform shaform commented Jul 15, 2014

Okay, let's use 2nd then
On Jul 15, 2014 5:59 AM, "ChangZhuo Chen (陳昌倬)" notifications@github.com
wrote:

Yes, test case in shaform@830c951
https://github.com/shaform/libchewing/commit/830c951ece7d593606ac2ea2a84b42770f22c77b
looks fine.

As for approach 1 & 2, I do not have any option as long as there is a test
case for it.


Reply to this email directly or view it on GitHub
#153 (comment).

czchen added a commit that referenced this pull request Aug 2, 2014
Fix unexpected candidate disorder for '1'.
@czchen czchen merged commit a5b7556 into chewing:master Aug 2, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.