From ae3b96b26ef86a9f342b4aeb2047abdaac5e57a5 Mon Sep 17 00:00:00 2001
From: Jean Chalard <jchalard@google.com>
Date: Fri, 11 Jan 2013 20:23:48 +0900
Subject: [PATCH] Simplify the space-before and space-after logic.

Bug: 7889078
Bug: 7268000
Change-Id: I6c77b8c9e60ef69f02526b407124d2f5d02818ee
---
 java/res/values-fr/donottranslate.xml         | 18 +++--
 java/res/values/donottranslate.xml            | 19 +++---
 .../android/inputmethod/latin/LatinIME.java   | 49 ++++++--------
 .../latin/RichInputConnection.java            | 13 ++--
 .../inputmethod/latin/SettingsValues.java     | 66 ++++++-------------
 5 files changed, 59 insertions(+), 106 deletions(-)

diff --git a/java/res/values-fr/donottranslate.xml b/java/res/values-fr/donottranslate.xml
index e11e65c79..2e916a734 100644
--- a/java/res/values-fr/donottranslate.xml
+++ b/java/res/values-fr/donottranslate.xml
@@ -18,15 +18,13 @@
 */
 -->
 <resources xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
-    <!-- Symbols that should be swapped with a magic space -->
-    <string name="weak_space_swapping_symbols">.,)]}</string>
-    <!-- Symbols that should strip a magic space -->
+    <!-- Symbols that are normally preceded by a space (used to add an auto-space before these) -->
+    <string name="symbols_preceded_by_space">([{*&amp;;:!?</string>
+    <!-- Symbols that are normally followed by a space (used to add an auto-space after these) -->
+    <string name="symbols_followed_by_space">.,;:!?)]}*&amp;</string>
+    <!-- Symbols that separate words -->
     <!-- Don't remove the enclosing double quotes, they protect whitespace (not just U+0020) -->
-    <string name="weak_space_stripping_symbols">"&#x0009;&#x0020;\n"\'-/_\"</string>
-    <!-- Symbols that should promote magic spaces into real space -->
-    <string name="phantom_space_promoting_symbols">;:!?([*&amp;@{&lt;&gt;+=|</string>
-    <!-- Symbols that do NOT separate words -->
-    <!-- Note that this is identical to the default value, but since the above ones are different
-         and those variables only make sense together, this is kept here for readability. -->
-    <string name="symbols_excluded_from_word_separators">\'-</string>
+    <string name="symbols_word_separators">"&#x0009;&#x0020;\n"()[]{}*&amp;&lt;&gt;+=|.,;:!?/_\"</string>
+    <!-- Word connectors -->
+    <string name="symbols_word_connectors">\'-</string>
 </resources>
diff --git a/java/res/values/donottranslate.xml b/java/res/values/donottranslate.xml
index 193a0191e..70ace776f 100644
--- a/java/res/values/donottranslate.xml
+++ b/java/res/values/donottranslate.xml
@@ -20,18 +20,15 @@
 <resources xmlns:xliff="urn:oasis:names:tc:xliff:document:1.2">
     <!-- Symbols that are suggested between words -->
     <string name="suggested_punctuations">!,?,\\,,:,;,\",(,),\',-,/,@,_</string>
-    <!-- Symbols that should be swapped with a weak space -->
-    <string name="weak_space_swapping_symbols">.,;:!?)]}</string>
-    <!-- Symbols that should strip a weak space -->
+    <!-- Symbols that are normally preceded by a space (used to add an auto-space before these) -->
+    <string name="symbols_preceded_by_space">([{*&amp;</string>
+    <!-- Symbols that are normally followed by a space (used to add an auto-space after these) -->
+    <string name="symbols_followed_by_space">.,;:!?)]}*&amp;</string>
+    <!-- Symbols that separate words -->
     <!-- Don't remove the enclosing double quotes, they protect whitespace (not just U+0020) -->
-    <string name="weak_space_stripping_symbols">"&#x0009;&#x0020;\n"/_\'-@\"</string>
-    <!-- Symbols that should convert weak spaces into real space -->
-    <string name="phantom_space_promoting_symbols">([*&amp;{&lt;&gt;+=|</string>
-    <!-- Symbols that do NOT separate words -->
-    <string name="symbols_excluded_from_word_separators">\'-</string>
-    <!-- Word separator list is the union of all symbols except those that are not separators:
-    weak_space_swapping_symbols | weak_space_stripping_symbols
-            \ symbols_excluded_from_word_separators -->
+    <string name="symbols_word_separators">"&#x0009;&#x0020;\n"()[]{}*&amp;&lt;&gt;+=|.,;:!?/_\"</string>
+    <!-- Word connectors -->
+    <string name="symbols_word_connectors">\'-</string>
     <!-- Symbol characters list that should switch back to the main layout -->
     <!-- U+2018: "‘" LEFT SINGLE QUOTATION MARK
          U+2019: "’" RIGHT SINGLE QUOTATION MARK
diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java
index 819625225..24d3c089c 100644
--- a/java/src/com/android/inputmethod/latin/LatinIME.java
+++ b/java/src/com/android/inputmethod/latin/LatinIME.java
@@ -1493,12 +1493,7 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
             mSpaceState = SPACE_STATE_PHANTOM;
         } else {
             final int codePointBeforeCursor = mConnection.getCodePointBeforeCursor();
-            // TODO: reverse this logic. We should have the means to determine whether a character
-            // should usually be followed by a space, and it should be more readable.
-            if (Constants.NOT_A_CODE != codePointBeforeCursor
-                    && !Character.isWhitespace(codePointBeforeCursor)
-                    && !mSettings.getCurrent().isPhantomSpacePromotingSymbol(codePointBeforeCursor)
-                    && !mSettings.getCurrent().isWeakSpaceStripper(codePointBeforeCursor)) {
+            if (mSettings.getCurrent().isUsuallyFollowedBySpace(codePointBeforeCursor)) {
                 mSpaceState = SPACE_STATE_PHANTOM;
             }
         }
@@ -1775,25 +1770,22 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
         }
     }
 
+    /*
+     * Strip a trailing space if necessary and returns whether it's a swap weak space situation.
+     */
     private boolean maybeStripSpace(final int code,
             final int spaceState, final boolean isFromSuggestionStrip) {
         if (Constants.CODE_ENTER == code && SPACE_STATE_SWAP_PUNCTUATION == spaceState) {
             mConnection.removeTrailingSpace();
             return false;
-        } else if ((SPACE_STATE_WEAK == spaceState
-                || SPACE_STATE_SWAP_PUNCTUATION == spaceState)
-                && isFromSuggestionStrip) {
-            if (mSettings.getCurrent().isWeakSpaceSwapper(code)) {
-                return true;
-            } else {
-                if (mSettings.getCurrent().isWeakSpaceStripper(code)) {
-                    mConnection.removeTrailingSpace();
-                }
-                return false;
-            }
-        } else {
-            return false;
         }
+        if ((SPACE_STATE_WEAK == spaceState || SPACE_STATE_SWAP_PUNCTUATION == spaceState)
+                && isFromSuggestionStrip) {
+            if (mSettings.getCurrent().isUsuallyPrecededBySpace(code)) return false;
+            if (mSettings.getCurrent().isUsuallyFollowedBySpace(code)) return true;
+            mConnection.removeTrailingSpace();
+        }
+        return false;
     }
 
     private void handleCharacter(final int primaryCode, final int x,
@@ -1801,7 +1793,7 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
         boolean isComposingWord = mWordComposer.isComposingWord();
 
         if (SPACE_STATE_PHANTOM == spaceState &&
-                !mSettings.getCurrent().isSymbolExcludedFromWordSeparators(primaryCode)) {
+                !mSettings.getCurrent().isWordConnector(primaryCode)) {
             if (isComposingWord) {
                 // Sanity check
                 throw new RuntimeException("Should not be composing here");
@@ -1813,7 +1805,7 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
         // dozen milliseconds. Avoid calling it as much as possible, since we are on the UI
         // thread here.
         if (!isComposingWord && (isAlphabet(primaryCode)
-                || mSettings.getCurrent().isSymbolExcludedFromWordSeparators(primaryCode))
+                || mSettings.getCurrent().isWordConnector(primaryCode))
                 && mSettings.getCurrent().isSuggestionsRequested(mDisplayOrientation) &&
                 !mConnection.isCursorTouchingWord(mSettings.getCurrent())) {
             // Reset entirely the composing state anyway, then start composing a new word unless
@@ -1885,7 +1877,7 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
                 Constants.SUGGESTION_STRIP_COORDINATE == x);
 
         if (SPACE_STATE_PHANTOM == spaceState &&
-                mSettings.getCurrent().isPhantomSpacePromotingSymbol(primaryCode)) {
+                mSettings.getCurrent().isUsuallyPrecededBySpace(primaryCode)) {
             promotePhantomSpace();
         }
         sendKeyCodePoint(primaryCode);
@@ -1900,16 +1892,13 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
             }
 
             mHandler.startDoubleSpacePeriodTimer();
-            if (!mConnection.isCursorTouchingWord(mSettings.getCurrent())) {
-                mHandler.postUpdateSuggestionStrip();
-            }
+            mHandler.postUpdateSuggestionStrip();
         } else {
             if (swapWeakSpace) {
                 swapSwapperAndSpace();
                 mSpaceState = SPACE_STATE_SWAP_PUNCTUATION;
             } else if (SPACE_STATE_PHANTOM == spaceState
-                    && !mSettings.getCurrent().isWeakSpaceStripper(primaryCode)
-                    && !mSettings.getCurrent().isPhantomSpacePromotingSymbol(primaryCode)) {
+                    && mSettings.getCurrent().isUsuallyFollowedBySpace(primaryCode)) {
                 // If we are in phantom space state, and the user presses a separator, we want to
                 // stay in phantom space state so that the next keypress has a chance to add the
                 // space. For example, if I type "Good dat", pick "day" from the suggestion strip
@@ -2158,9 +2147,9 @@ public final class LatinIME extends InputMethodService implements KeyboardAction
                 // In the batch input mode, a manually picked suggested word should just replace
                 // the current batch input text and there is no need for a phantom space.
                 && !mWordComposer.isBatchMode()) {
-            int firstChar = Character.codePointAt(suggestion, 0);
-            if ((!mSettings.getCurrent().isWeakSpaceStripper(firstChar))
-                    && (!mSettings.getCurrent().isWeakSpaceSwapper(firstChar))) {
+            final int firstChar = Character.codePointAt(suggestion, 0);
+            if (!mSettings.getCurrent().isWordSeparator(firstChar)
+                    || mSettings.getCurrent().isUsuallyPrecededBySpace(firstChar)) {
                 promotePhantomSpace();
             }
         }
diff --git a/java/src/com/android/inputmethod/latin/RichInputConnection.java b/java/src/com/android/inputmethod/latin/RichInputConnection.java
index f7268fc33..0e75533f5 100644
--- a/java/src/com/android/inputmethod/latin/RichInputConnection.java
+++ b/java/src/com/android/inputmethod/latin/RichInputConnection.java
@@ -577,11 +577,11 @@ public final class RichInputConnection {
         final CharSequence before = getTextBeforeCursor(1, 0);
         final CharSequence after = getTextAfterCursor(1, 0);
         if (!TextUtils.isEmpty(before) && !settingsValues.isWordSeparator(before.charAt(0))
-                && !settingsValues.isSymbolExcludedFromWordSeparators(before.charAt(0))) {
+                && !settingsValues.isWordConnector(before.charAt(0))) {
             return true;
         }
         if (!TextUtils.isEmpty(after) && !settingsValues.isWordSeparator(after.charAt(0))
-                && !settingsValues.isSymbolExcludedFromWordSeparators(after.charAt(0))) {
+                && !settingsValues.isWordConnector(after.charAt(0))) {
             return true;
         }
         return false;
@@ -633,12 +633,9 @@ public final class RichInputConnection {
         final char firstChar = word.charAt(0); // we just tested that word is not empty
         if (word.length() == 1 && !Character.isLetter(firstChar)) return null;
 
-        // We only suggest on words that start with a letter or a symbol that is excluded from
-        // word separators (see #handleCharacterWhileInBatchEdit).
-        if (!(Character.isLetter(firstChar)
-                || settings.isSymbolExcludedFromWordSeparators(firstChar))) {
-            return null;
-        }
+        // We don't restart suggestion if the first character is not a letter, because we don't
+        // start composing when the first character is not a letter.
+        if (!Character.isLetter(firstChar)) return null;
 
         return word;
     }
diff --git a/java/src/com/android/inputmethod/latin/SettingsValues.java b/java/src/com/android/inputmethod/latin/SettingsValues.java
index 9a2024618..93d8fb41f 100644
--- a/java/src/com/android/inputmethod/latin/SettingsValues.java
+++ b/java/src/com/android/inputmethod/latin/SettingsValues.java
@@ -37,11 +37,10 @@ public final class SettingsValues {
 
     // From resources:
     public final int mDelayUpdateOldSuggestions;
-    public final String mWeakSpaceStrippers;
-    public final String mWeakSpaceSwappers;
-    private final String mPhantomSpacePromotingSymbols;
+    public final int[] mSymbolsPrecededBySpace;
+    public final int[] mSymbolsFollowedBySpace;
+    public final int[] mWordConnectors;
     public final SuggestedWords mSuggestPuncList;
-    private final String mSymbolsExcludedFromWordSeparators;
     public final String mWordSeparators;
     public final CharSequence mHintToSaveText;
 
@@ -79,25 +78,19 @@ public final class SettingsValues {
             final InputAttributes inputAttributes) {
         // Get the resources
         mDelayUpdateOldSuggestions = res.getInteger(R.integer.config_delay_update_old_suggestions);
-        mWeakSpaceStrippers = res.getString(R.string.weak_space_stripping_symbols);
-        mWeakSpaceSwappers = res.getString(R.string.weak_space_swapping_symbols);
-        mPhantomSpacePromotingSymbols = res.getString(R.string.phantom_space_promoting_symbols);
-        if (LatinImeLogger.sDBG) {
-            final int length = mWeakSpaceStrippers.length();
-            for (int i = 0; i < length; i = mWeakSpaceStrippers.offsetByCodePoints(i, 1)) {
-                if (isWeakSpaceSwapper(mWeakSpaceStrippers.codePointAt(i))) {
-                    throw new RuntimeException("Char code " + mWeakSpaceStrippers.codePointAt(i)
-                            + " is both a weak space swapper and stripper.");
-                }
-            }
-        }
+        mSymbolsPrecededBySpace =
+                StringUtils.toCodePointArray(res.getString(R.string.symbols_preceded_by_space));
+        Arrays.sort(mSymbolsPrecededBySpace);
+        mSymbolsFollowedBySpace =
+                StringUtils.toCodePointArray(res.getString(R.string.symbols_followed_by_space));
+        Arrays.sort(mSymbolsFollowedBySpace);
+        mWordConnectors =
+                StringUtils.toCodePointArray(res.getString(R.string.symbols_word_connectors));
+        Arrays.sort(mWordConnectors);
         final String[] suggestPuncsSpec = KeySpecParser.parseCsvString(
                 res.getString(R.string.suggested_punctuations), null);
         mSuggestPuncList = createSuggestPuncList(suggestPuncsSpec);
-        mSymbolsExcludedFromWordSeparators =
-                res.getString(R.string.symbols_excluded_from_word_separators);
-        mWordSeparators = createWordSeparators(mWeakSpaceStrippers, mWeakSpaceSwappers,
-                mSymbolsExcludedFromWordSeparators, res);
+        mWordSeparators = res.getString(R.string.symbols_word_separators);
         mHintToSaveText = res.getText(R.string.hint_add_to_dictionary);
 
         // Store the input attributes
@@ -169,25 +162,16 @@ public final class SettingsValues {
         return mWordSeparators.contains(String.valueOf((char)code));
     }
 
-    public boolean isSymbolExcludedFromWordSeparators(final int code) {
-        return mSymbolsExcludedFromWordSeparators.contains(String.valueOf((char)code));
+    public boolean isWordConnector(final int code) {
+        return Arrays.binarySearch(mWordConnectors, code) >= 0;
     }
 
-    // TODO: use "Phantom" instead of "Weak" in this method name
-    public boolean isWeakSpaceStripper(final int code) {
-        // TODO: this does not work if the code does not fit in a char
-        return mWeakSpaceStrippers.contains(String.valueOf((char)code));
+    public boolean isUsuallyPrecededBySpace(final int code) {
+        return Arrays.binarySearch(mSymbolsPrecededBySpace, code) >= 0;
     }
 
-    // TODO: use "Phantom" instead of "Weak" in this method name
-    public boolean isWeakSpaceSwapper(final int code) {
-        // TODO: this does not work if the code does not fit in a char
-        return mWeakSpaceSwappers.contains(String.valueOf((char)code));
-    }
-
-    public boolean isPhantomSpacePromotingSymbol(final int code) {
-        // TODO: this does not work if the code does not fit in a char
-        return mPhantomSpacePromotingSymbols.contains(String.valueOf((char)code));
+    public boolean isUsuallyFollowedBySpace(final int code) {
+        return Arrays.binarySearch(mSymbolsFollowedBySpace, code) >= 0;
     }
 
     public boolean shouldInsertSpacesAutomatically() {
@@ -239,18 +223,6 @@ public final class SettingsValues {
                 false /* isPrediction */);
     }
 
-    private static String createWordSeparators(final String weakSpaceStrippers,
-            final String weakSpaceSwappers, final String symbolsExcludedFromWordSeparators,
-            final Resources res) {
-        String wordSeparators = weakSpaceStrippers + weakSpaceSwappers
-                + res.getString(R.string.phantom_space_promoting_symbols);
-        for (int i = symbolsExcludedFromWordSeparators.length() - 1; i >= 0; --i) {
-            wordSeparators = wordSeparators.replace(
-                    symbolsExcludedFromWordSeparators.substring(i, i + 1), "");
-        }
-        return wordSeparators;
-    }
-
     private static final int SUGGESTION_VISIBILITY_SHOW_VALUE =
             R.string.prefs_suggestion_visibility_show_value;
     private static final int SUGGESTION_VISIBILITY_SHOW_ONLY_PORTRAIT_VALUE =