use CombiningRules extra value for setting the combiner

allows flexibility which is needed for the combiner suggested in GH-214

app/src/main/java/helium314/keyboard/event/CombinerChain.kt

@@ -29,30 +29,18 @@ import java.util.*
  * cursor: we'll start after this.
  * @param initialText The text that has already been combined so far.
  */
-class CombinerChain(initialText: String) {
+class CombinerChain(initialText: String, combiningSpec: String) {
     // The already combined text, as described above
     private val mCombinedText = StringBuilder(initialText)
     // The feedback on the composing state, as described above
     private val mStateFeedback = SpannableStringBuilder()
     private val mCombiners = ArrayList<Combiner>()
-    // Hangul combiner affects other scripts, e.g. period is seen as port of a word for latin,
-    // so we need to remove the combiner when not writing in hangul script.
-    // Maybe it would be better to always have the Hangul combiner, but make sure it doesn't affect
-    // events for other scripts, but how?
-    // todo: this really should be done properly, hangul combiner should do nothing when it's not needed
-    var isHangul = false
-        set(value) {
-            if (field == value) return
-            field = value
-            if (!value)
-                mCombiners.removeAll { it is HangulCombiner }
-            else if (mCombiners.none { it is HangulCombiner })
-                mCombiners.add(HangulCombiner())
-        }
 
     init {
         // The dead key combiner is always active, and always first
         mCombiners.add(DeadKeyCombiner())
+        if (combiningSpec == "hangul")
+            mCombiners.add(HangulCombiner())
     }
 
     fun reset() {

app/src/main/java/helium314/keyboard/event/HangulCombiner.kt

@@ -16,6 +16,8 @@ class HangulCombiner : Combiner {
 
     override fun processEvent(previousEvents: ArrayList<Event>?, event: Event): Event {
         if (event.mKeyCode == KeyCode.SHIFT) return event
+        // previously we only used the combiner if codePoint > 0x1100 or codePoint == -1, but looks here it's not necessary
+        val event = HangulEventDecoder.decodeSoftwareKeyEvent(event)
         if (Character.isWhitespace(event.mCodePoint)) {
             val text = combiningStateFeedback
             reset()

app/src/main/java/helium314/keyboard/latin/App.kt

@@ -34,7 +34,6 @@ import helium314.keyboard.latin.utils.ToolbarKey
 import helium314.keyboard.latin.utils.defaultPinnedToolbarPref
 import helium314.keyboard.latin.utils.getResourceSubtypes
 import helium314.keyboard.latin.utils.locale
-import helium314.keyboard.latin.utils.mainLayoutName
 import helium314.keyboard.latin.utils.mainLayoutNameOrQwerty
 import helium314.keyboard.latin.utils.prefs
 import helium314.keyboard.latin.utils.upgradeToolbarPrefs
@@ -596,6 +595,18 @@ fun checkVersionUpgrade(context: Context) {
                 e.putFloat(createPrefKeyForBooleanSettings(Settings.PREF_KEYBOARD_HEIGHT_SCALE_PREFIX, 1, 1), value as Float)
                 e.putFloat(createPrefKeyForBooleanSettings(Settings.PREF_KEYBOARD_HEIGHT_SCALE_PREFIX, 1, 1), value)
             } else {
+                if (key == Settings.PREF_ADDITIONAL_SUBTYPES || key == Settings.PREF_ENABLED_SUBTYPES) {
+                    val subtypes = prefs.getString(key, "")!!.split(Separators.SETS).filter { it.isNotEmpty() }.map {
+                        val st = it.toSettingsSubtype()
+                        if (st.locale.language == "ko") st.with(ExtraValue.COMBINING_RULES, "hangul")
+                        else st
+                    }
+                    e.putString(key, subtypes.joinToString(Separators.SETS) { it.toPref() })
+                } else if (key == Settings.PREF_SELECTED_SUBTYPE) {
+                    val subtype = prefs.getString(key, "")!!.toSettingsSubtype()
+                    if (subtype.locale.language == "ko")
+                        e.putString(key, subtype.with(ExtraValue.COMBINING_RULES, "hangul").toPref())
+                }
                 return@forEach
             }
             e.remove(key)

app/src/main/java/helium314/keyboard/latin/WordComposer.java

@@ -10,8 +10,6 @@ import androidx.annotation.NonNull;
 
 import helium314.keyboard.event.CombinerChain;
 import helium314.keyboard.event.Event;
-import helium314.keyboard.keyboard.Keyboard;
-import helium314.keyboard.keyboard.KeyboardSwitcher;
 import helium314.keyboard.keyboard.internal.keyboard_parser.floris.KeyCode;
 import helium314.keyboard.latin.SuggestedWords.SuggestedWordInfo;
 import helium314.keyboard.latin.common.ComposedData;
@@ -73,7 +71,7 @@ public final class WordComposer {
     private boolean mIsOnlyFirstCharCapitalized;
 
     public WordComposer() {
-        mCombinerChain = new CombinerChain("");
+        mCombinerChain = new CombinerChain("", "");
         mEvents = new ArrayList<>();
         mAutoCorrection = null;
         mIsResumed = false;
@@ -81,11 +79,6 @@ public final class WordComposer {
         mCursorPositionWithinWord = 0;
         mRejectedBatchModeSuggestion = null;
         refreshTypedWordCache();
-        final Keyboard keyboard = KeyboardSwitcher.getInstance().getKeyboard();
-        if (keyboard != null)
-            // initializing with the right state is important for the spell checker,
-            // which creates a new WordComposer when receiving suggestions
-            mCombinerChain.setHangul(keyboard.mId.mSubtype.getLocale().getLanguage().equals("ko"));
     }
 
     public ComposedData getComposedDataSnapshot() {
@@ -99,14 +92,11 @@ public final class WordComposer {
     public void restartCombining(final String combiningSpec) {
         final String nonNullCombiningSpec = null == combiningSpec ? "" : combiningSpec;
         if (!nonNullCombiningSpec.equals(mCombiningSpec)) {
-            mCombinerChain = new CombinerChain(mCombinerChain.getComposingWordWithCombiningFeedback().toString());
+            mCombinerChain = new CombinerChain(mCombinerChain.getComposingWordWithCombiningFeedback().toString(), nonNullCombiningSpec);
             mCombiningSpec = nonNullCombiningSpec;
         }
     }
 
-    /** Forwards the state to CombinerChain, which disables or enables the Hangul combiner */
-    public void setHangul(final boolean enabled) { mCombinerChain.setHangul(enabled); }
-
     /**
      * Clear out the keys registered so far.
      */
@@ -218,11 +208,6 @@ public final class WordComposer {
         // TODO: compute where that puts us inside the events
     }
 
-    public void resetInvalidCursorPosition() {
-        if (mCursorPositionWithinWord > mCodePointSize)
-            mCursorPositionWithinWord = 0;
-    }
-
     public boolean isCursorFrontOrMiddleOfComposingWord() {
         if (DebugFlags.DEBUG_ENABLED && mCursorPositionWithinWord > mCodePointSize) {
             throw new RuntimeException("Wrong cursor position : " + mCursorPositionWithinWord

app/src/main/java/helium314/keyboard/latin/common/Constants.java

@@ -77,7 +77,7 @@ public final class Constants {
             /** Indicates that this subtype is an additional subtype that the user defined. This extra value is private to LatinIME. */
             public static final String IS_ADDITIONAL_SUBTYPE = "isAdditionalSubtype";
 
-            /** The subtype extra value used to specify the combining rules (currently not used). */
+            /** The subtype extra value used to specify the combining rules. */
             public static final String COMBINING_RULES = "CombiningRules";
 
             /** Overrides the general popup order setting */

app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java

@@ -22,7 +22,6 @@ import android.view.inputmethod.EditorInfo;
 import androidx.annotation.NonNull;
 
 import helium314.keyboard.event.Event;
-import helium314.keyboard.event.HangulEventDecoder;
 import helium314.keyboard.event.InputTransaction;
 import helium314.keyboard.keyboard.Keyboard;
 import helium314.keyboard.keyboard.KeyboardSwitcher;
@@ -130,7 +129,7 @@ public final class InputLogic {
      * <p>
      * Call this when input starts or restarts in some editor (typically, in onStartInputView).
      *
-     * @param combiningSpec the combining spec string for this subtype
+     * @param combiningSpec the combining spec string for this subtype (from extra value)
      * @param settingsValues the current settings values
      */
     public void startInput(final String combiningSpec, final SettingsValues settingsValues) {
@@ -425,35 +424,7 @@ public final class InputLogic {
             final String currentKeyboardScript, final LatinIME.UIHandler handler) {
         mWordBeingCorrectedByCursor = null;
         mJustRevertedACommit = false;
-        final Event processedEvent;
+        final Event processedEvent = mWordComposer.processEvent(event);
-        if (currentKeyboardScript.equals(ScriptUtils.SCRIPT_HANGUL)) {
-            // only use the Hangul chain if codepoint may actually be Hangul
-            // todo: this whole hangul-related logic should probably be somewhere else
-            // need to use hangul combiner for functional keys (codePoint -1), because otherwise the current word
-            // seems to get deleted / replaced by space during mConnection.endBatchEdit()
-            if (event.getMCodePoint() >= 0x1100 || event.getMCodePoint() == -1) {
-                mWordComposer.setHangul(true);
-                final Event hangulDecodedEvent = HangulEventDecoder.decodeSoftwareKeyEvent(event);
-                // todo: here hangul combiner does already consume the event, and appends typed codepoint
-                //  to the current word instead of considering the cursor position
-                //  position is actually not visible to the combiner, how to fix?
-                processedEvent = mWordComposer.processEvent(hangulDecodedEvent);
-                if (event.getMKeyCode() == KeyCode.DELETE)
-                    mWordComposer.resetInvalidCursorPosition();
-            } else {
-                mWordComposer.setHangul(false);
-                final boolean wasComposingWord = mWordComposer.isComposingWord();
-                processedEvent = mWordComposer.processEvent(event);
-                // workaround for space and some other separators deleting / replacing the word
-                if (wasComposingWord && !mWordComposer.isComposingWord()) {
-                    mWordComposer.resetInvalidCursorPosition();
-                    mConnection.finishComposingText();
-                }
-            }
-        } else {
-            mWordComposer.setHangul(false);
-            processedEvent = mWordComposer.processEvent(event);
-        }
         final InputTransaction inputTransaction = new InputTransaction(settingsValues,
                 processedEvent, SystemClock.uptimeMillis(), mSpaceState,
                 getActualCapsMode(settingsValues, keyboardShiftMode));

app/src/main/java/helium314/keyboard/latin/spellcheck/AndroidWordLevelSpellCheckerSession.java

@@ -335,6 +335,8 @@ public abstract class AndroidWordLevelSpellCheckerSession extends Session {
             }
 
             final WordComposer composer = new WordComposer();
+            if (mLocale.getLanguage().equals("ko"))
+                composer.restartCombining("hangul");
             final int[] codePoints = StringUtils.toCodePointArray(text);
             final int[] coordinates;
             coordinates = keyboard.getCoordinates(codePoints);

app/src/main/res/xml/method.xml

@@ -763,7 +763,7 @@
         android:imeSubtypeLocale="ko"
         android:languageTag="ko"
         android:imeSubtypeMode="keyboard"
-        android:imeSubtypeExtraValue="KeyboardLayoutSet=MAIN:korean,SupportTouchPositionCorrection,EmojiCapable"
+        android:imeSubtypeExtraValue="KeyboardLayoutSet=MAIN:korean,CombiningRules=hangul,SupportTouchPositionCorrection,EmojiCapable"
         android:isAsciiCapable="false"
     />
 	<subtype android:icon="@drawable/ic_ime_switcher"
@@ -772,7 +772,7 @@
         android:imeSubtypeLocale="ko"
         android:languageTag="ko"
         android:imeSubtypeMode="keyboard"
-        android:imeSubtypeExtraValue="KeyboardLayoutSet=MAIN:korean_phonetic,SupportTouchPositionCorrection,EmojiCapable"
+        android:imeSubtypeExtraValue="KeyboardLayoutSet=MAIN:korean_phonetic,CombiningRules=hangul,SupportTouchPositionCorrection,EmojiCapable"
         android:isAsciiCapable="false"
     />
     <subtype android:icon="@drawable/ic_ime_switcher"
@@ -781,7 +781,7 @@
         android:imeSubtypeLocale="ko"
         android:languageTag="ko"
         android:imeSubtypeMode="keyboard"
-        android:imeSubtypeExtraValue="KeyboardLayoutSet=MAIN:korean_sebeolsik_390,SupportTouchPositionCorrection,EmojiCapable"
+        android:imeSubtypeExtraValue="KeyboardLayoutSet=MAIN:korean_sebeolsik_390,CombiningRules=hangul,SupportTouchPositionCorrection,EmojiCapable"
         android:isAsciiCapable="false"
     />
     <subtype android:icon="@drawable/ic_ime_switcher"
@@ -790,7 +790,7 @@
         android:imeSubtypeLocale="ko"
         android:languageTag="ko"
         android:imeSubtypeMode="keyboard"
-        android:imeSubtypeExtraValue="KeyboardLayoutSet=MAIN:korean_sebeolsik_final,SupportTouchPositionCorrection,EmojiCapable"
+        android:imeSubtypeExtraValue="KeyboardLayoutSet=MAIN:korean_sebeolsik_final,CombiningRules=hangul,SupportTouchPositionCorrection,EmojiCapable"
         android:isAsciiCapable="false"
     />
     <subtype android:icon="@drawable/ic_ime_switcher"

app/src/test/java/helium314/keyboard/latin/InputLogicTest.kt

@@ -18,11 +18,13 @@ import helium314.keyboard.keyboard.internal.keyboard_parser.floris.KeyCode
 import helium314.keyboard.latin.ShadowFacilitator2.Companion.lastAddedWord
 import helium314.keyboard.latin.SuggestedWords.SuggestedWordInfo
 import helium314.keyboard.latin.common.Constants
+import helium314.keyboard.latin.common.LocaleUtils.constructLocale
 import helium314.keyboard.latin.common.StringUtils
 import helium314.keyboard.latin.inputlogic.InputLogic
 import helium314.keyboard.latin.inputlogic.SpaceState
 import helium314.keyboard.latin.settings.Settings
 import helium314.keyboard.latin.utils.ScriptUtils
+import helium314.keyboard.latin.utils.SubtypeSettings
 import helium314.keyboard.latin.utils.getTimestamp
 import helium314.keyboard.latin.utils.prefs
 import org.junit.runner.RunWith
@@ -141,7 +143,7 @@ class InputLogicTest {
     @Test fun insertLetterIntoWordHangul() {
         if (BuildConfig.BUILD_TYPE == "runTests") return
         reset()
-        currentScript = ScriptUtils.SCRIPT_HANGUL
+        latinIME.switchToSubtype(SubtypeSettings.getResourceSubtypesForLocale("ko".constructLocale()).first())
         chainInput("ㅛㅎㄹㅎㅕㅛ")
         setCursorPosition(3)
         input('ㄲ') // fails, as expected from the hangul issue when processing the event in onCodeInput
@@ -155,7 +157,7 @@ class InputLogicTest {
     // see issue 1447
     @Test fun separatorAfterHangul() {
         reset()
-        currentScript = ScriptUtils.SCRIPT_HANGUL
+        latinIME.switchToSubtype(SubtypeSettings.getResourceSubtypesForLocale("ko".constructLocale()).first())
         chainInput("ㅛ.")
         assertEquals("ㅛ.", text)
     }
@@ -163,7 +165,7 @@ class InputLogicTest {
     // see issue 1551 (debug only)
     @Test fun deleteHangul() {
         reset()
-        currentScript = ScriptUtils.SCRIPT_HANGUL
+        latinIME.switchToSubtype(SubtypeSettings.getResourceSubtypesForLocale("ko".constructLocale()).first())
         setText("ㅛㅛ ")
         functionalKeyPress(KeyCode.DELETE)
         functionalKeyPress(KeyCode.DELETE)