From 7cd7cf73f4ce6f0e577d6382eb0fc25f60dc63e1 Mon Sep 17 00:00:00 2001 From: Jean Chalard Date: Mon, 16 Dec 2013 21:41:03 +0900 Subject: [PATCH] Fix a bug with languages without spaces and predictions This is simpler and more correct. Change-Id: I41806d2fc12b4ca25f76e32972b38f91f3d05c2b --- .../android/inputmethod/latin/LatinIME.java | 19 ++++++++-- .../inputmethod/latin/WordComposer.java | 38 +++++++++---------- .../inputmethod/latin/utils/StringUtils.java | 18 +++++++++ .../inputmethod/latin/WordComposerTests.java | 16 ++++---- .../latin/utils/StringAndJsonUtilsTests.java | 14 +++++++ 5 files changed, 73 insertions(+), 32 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index 1f6091e90..72d5435a8 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -1469,7 +1469,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen ResearchLogger.latinIME_maybeDoubleSpacePeriod(textToInsert, false /* isBatchMode */); } - mWordComposer.doubleSpacePeriod(); + mWordComposer.discardPreviousWordForSuggestion(); mKeyboardSwitcher.updateShiftState(); return true; } @@ -2567,7 +2567,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen if (DEBUG) { if (mWordComposer.isComposingWord() || mWordComposer.isBatchMode()) { - final String previousWord = mWordComposer.getPreviousWord(); + final String previousWord = mWordComposer.getPreviousWordForSuggestion(); // TODO: this is for checking consistency with older versions. Remove this when // we are confident this is stable. // We're checking the previous word in the text field against the memorized previous @@ -2581,7 +2581,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } } } - suggest.getSuggestedWords(mWordComposer, mWordComposer.getPreviousWord(), + suggest.getSuggestedWords(mWordComposer, mWordComposer.getPreviousWordForSuggestion(), keyboard.getProximityInfo(), currentSettings.mBlockPotentiallyOffensive, currentSettings.mCorrectionEnabled, additionalFeaturesOptions, sessionId, sequenceNumber, callback); @@ -2824,6 +2824,19 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // strings. mLastComposedWord = mWordComposer.commitWord(commitType, chosenWord, separatorString, prevWord); + final boolean shouldDiscardPreviousWordForSuggestion; + if (0 == StringUtils.codePointCount(separatorString)) { + // Separator is 0-length. Discard the word only if the current language has spaces. + shouldDiscardPreviousWordForSuggestion = + mSettings.getCurrent().mCurrentLanguageHasSpaces; + } else { + // Otherwise, we discard if the separator contains any non-whitespace. + shouldDiscardPreviousWordForSuggestion = + !StringUtils.containsOnlyWhitespace(separatorString); + } + if (shouldDiscardPreviousWordForSuggestion) { + mWordComposer.discardPreviousWordForSuggestion(); + } } private void setPunctuationSuggestions() { diff --git a/java/src/com/android/inputmethod/latin/WordComposer.java b/java/src/com/android/inputmethod/latin/WordComposer.java index 5ecfc67b2..7da97e57a 100644 --- a/java/src/com/android/inputmethod/latin/WordComposer.java +++ b/java/src/com/android/inputmethod/latin/WordComposer.java @@ -50,8 +50,9 @@ public final class WordComposer { private final StringBuilder mTypedWord; // The previous word (before the composing word). Used as context for suggestions. May be null // after resetting and before starting a new composing word, or when there is no context like - // at the start of text for example. - private String mPreviousWord; + // at the start of text for example. It can also be set to null externally when the user + // enters a separator that does not let bigrams across, like a period or a comma. + private String mPreviousWordForSuggestion; private String mAutoCorrection; private boolean mIsResumed; private boolean mIsBatchMode; @@ -89,7 +90,7 @@ public final class WordComposer { mIsBatchMode = false; mCursorPositionWithinWord = 0; mRejectedBatchModeSuggestion = null; - mPreviousWord = null; + mPreviousWordForSuggestion = null; refreshSize(); } @@ -106,7 +107,7 @@ public final class WordComposer { mIsBatchMode = source.mIsBatchMode; mCursorPositionWithinWord = source.mCursorPositionWithinWord; mRejectedBatchModeSuggestion = source.mRejectedBatchModeSuggestion; - mPreviousWord = source.mPreviousWord; + mPreviousWordForSuggestion = source.mPreviousWordForSuggestion; refreshSize(); } @@ -124,7 +125,7 @@ public final class WordComposer { mIsBatchMode = false; mCursorPositionWithinWord = 0; mRejectedBatchModeSuggestion = null; - mPreviousWord = null; + mPreviousWordForSuggestion = null; refreshSize(); } @@ -305,7 +306,7 @@ public final class WordComposer { addKeyInfo(codePoint, keyboard); } mIsResumed = true; - mPreviousWord = previousWord; + mPreviousWordForSuggestion = previousWord; } /** @@ -356,8 +357,8 @@ public final class WordComposer { return mTypedWord.toString(); } - public String getPreviousWord() { - return mPreviousWord; + public String getPreviousWordForSuggestion() { + return mPreviousWordForSuggestion; } /** @@ -419,7 +420,7 @@ public final class WordComposer { public void setCapitalizedModeAndPreviousWordAtStartComposingTime(final int mode, final String previousWord) { mCapitalizedMode = mode; - mPreviousWord = previousWord; + mPreviousWordForSuggestion = previousWord; } /** @@ -471,12 +472,7 @@ public final class WordComposer { mCapsCount = 0; mDigitsCount = 0; mIsBatchMode = false; - final boolean isWhitespace = 1 == StringUtils.codePointCount(separatorString) - && Character.isWhitespace(separatorString.codePointAt(0)); - // If not whitespace, we don't use the previous word for suggestion. This is consistent - // with how we get the previous word for suggestion: see RichInputConnection#spaceRegex and - // LatinIME#getNthPreviousWordForSuggestion. - mPreviousWord = isWhitespace ? mTypedWord.toString() : null; + mPreviousWordForSuggestion = mTypedWord.toString(); mTypedWord.setLength(0); mCodePointSize = 0; mTrailingSingleQuotesCount = 0; @@ -490,11 +486,11 @@ public final class WordComposer { return lastComposedWord; } - public void doubleSpacePeriod() { - // When a period was entered with a double space, the separator we got has been - // changed by a period (see #commitWord). We should not use the previous word for - // suggestion. - mPreviousWord = null; + // Call this when the recorded previous word should be discarded. This is typically called + // when the user inputs a separator that's not whitespace (including the case of the + // double-space-to-period feature). + public void discardPreviousWordForSuggestion() { + mPreviousWordForSuggestion = null; } public void resumeSuggestionOnLastComposedWord(final LastComposedWord lastComposedWord, @@ -509,7 +505,7 @@ public final class WordComposer { mCursorPositionWithinWord = mCodePointSize; mRejectedBatchModeSuggestion = null; mIsResumed = true; - mPreviousWord = previousWord; + mPreviousWordForSuggestion = previousWord; } public boolean isBatchMode() { diff --git a/java/src/com/android/inputmethod/latin/utils/StringUtils.java b/java/src/com/android/inputmethod/latin/utils/StringUtils.java index df420417d..85f44541e 100644 --- a/java/src/com/android/inputmethod/latin/utils/StringUtils.java +++ b/java/src/com/android/inputmethod/latin/utils/StringUtils.java @@ -250,6 +250,24 @@ public final class StringUtils { return true; } + /** + * Returns true if all code points in text are whitespace, false otherwise. Empty is true. + */ + // Interestingly enough, U+00A0 NO-BREAK SPACE and U+200B ZERO-WIDTH SPACE are not considered + // whitespace, while EN SPACE, EM SPACE and IDEOGRAPHIC SPACES are. + public static boolean containsOnlyWhitespace(final String text) { + final int length = text.length(); + int i = 0; + while (i < length) { + final int codePoint = text.codePointAt(i); + if (!Character.isWhitespace(codePoint)) { + return false; + } + i += Character.charCount(codePoint); + } + return true; + } + @UsedForTesting public static boolean looksValidForDictionaryInsertion(final CharSequence text, final SettingsValues settings) { diff --git a/tests/src/com/android/inputmethod/latin/WordComposerTests.java b/tests/src/com/android/inputmethod/latin/WordComposerTests.java index a67f6a4ac..1336c6d1a 100644 --- a/tests/src/com/android/inputmethod/latin/WordComposerTests.java +++ b/tests/src/com/android/inputmethod/latin/WordComposerTests.java @@ -51,14 +51,14 @@ public class WordComposerTests extends AndroidTestCase { assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(1)); assertFalse(wc.isCursorFrontOrMiddleOfComposingWord()); // Check the previous word is still there - assertEquals(PREVWORD, wc.getPreviousWord()); + assertEquals(PREVWORD, wc.getPreviousWordForSuggestion()); // Move the cursor past the end of the word assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(1)); assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(15)); // Do what LatinIME does when the cursor is moved outside of the word, // and check the behavior is correct. wc.reset(); - assertNull(wc.getPreviousWord()); + assertNull(wc.getPreviousWordForSuggestion()); // \uD861\uDED7 is 𨛗, a character outside the BMP final String STR_WITH_SUPPLEMENTARY_CHAR = "abcde\uD861\uDED7fgh"; @@ -73,35 +73,35 @@ public class WordComposerTests extends AndroidTestCase { assertTrue(wc.isCursorFrontOrMiddleOfComposingWord()); assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(1)); assertFalse(wc.isCursorFrontOrMiddleOfComposingWord()); - assertNull(wc.getPreviousWord()); + assertNull(wc.getPreviousWordForSuggestion()); wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, STR_WITHIN_BMP, null /* keyboard */); wc.setCursorPositionWithinWord(3); assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(7)); - assertEquals(STR_WITHIN_BMP, wc.getPreviousWord()); + assertEquals(STR_WITHIN_BMP, wc.getPreviousWordForSuggestion()); wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, STR_WITH_SUPPLEMENTARY_CHAR, null /* keyboard */); wc.setCursorPositionWithinWord(3); assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(7)); - assertEquals(STR_WITH_SUPPLEMENTARY_CHAR, wc.getPreviousWord()); + assertEquals(STR_WITH_SUPPLEMENTARY_CHAR, wc.getPreviousWordForSuggestion()); wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, STR_WITHIN_BMP, null /* keyboard */); wc.setCursorPositionWithinWord(3); assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(-3)); assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(-1)); - assertEquals(STR_WITHIN_BMP, wc.getPreviousWord()); + assertEquals(STR_WITHIN_BMP, wc.getPreviousWordForSuggestion()); wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null /* previousWord */, null /* keyboard */); wc.setCursorPositionWithinWord(3); assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(-9)); - assertNull(wc.getPreviousWord()); + assertNull(wc.getPreviousWordForSuggestion()); wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, STR_WITH_SUPPLEMENTARY_CHAR, null /* keyboard */); assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(-10)); - assertEquals(STR_WITH_SUPPLEMENTARY_CHAR, wc.getPreviousWord()); + assertEquals(STR_WITH_SUPPLEMENTARY_CHAR, wc.getPreviousWordForSuggestion()); wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null /* previousWord */, null /* keyboard */); diff --git a/tests/src/com/android/inputmethod/latin/utils/StringAndJsonUtilsTests.java b/tests/src/com/android/inputmethod/latin/utils/StringAndJsonUtilsTests.java index 2123e84e8..0c88f34f0 100644 --- a/tests/src/com/android/inputmethod/latin/utils/StringAndJsonUtilsTests.java +++ b/tests/src/com/android/inputmethod/latin/utils/StringAndJsonUtilsTests.java @@ -292,6 +292,20 @@ public class StringAndJsonUtilsTests extends AndroidTestCase { assertTrue(bytesStr.equals(bytesStr2)); } + public void testContainsOnlyWhitespace() { + assertTrue(StringUtils.containsOnlyWhitespace(" ")); + assertTrue(StringUtils.containsOnlyWhitespace("")); + assertTrue(StringUtils.containsOnlyWhitespace(" \n\t\t")); + // U+2002 : EN SPACE + // U+2003 : EM SPACE + // U+3000 : IDEOGRAPHIC SPACE (commonly "double-width space") + assertTrue(StringUtils.containsOnlyWhitespace("\u2002\u2003\u3000")); + assertFalse(StringUtils.containsOnlyWhitespace(" a ")); + assertFalse(StringUtils.containsOnlyWhitespace(". ")); + assertFalse(StringUtils.containsOnlyWhitespace(".")); + assertTrue(StringUtils.containsOnlyWhitespace("")); + } + public void testJsonUtils() { final Object[] objs = new Object[] { 1, "aaa", "bbb", 3 }; final List objArray = Arrays.asList(objs);