diff --git a/app/src/main/java/helium314/keyboard/latin/inputlogic/BackspaceUnitStack.java b/app/src/main/java/helium314/keyboard/latin/inputlogic/BackspaceUnitStack.java new file mode 100644 index 00000000..52d05e70 --- /dev/null +++ b/app/src/main/java/helium314/keyboard/latin/inputlogic/BackspaceUnitStack.java @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: GPL-3.0-only +package helium314.keyboard.latin.inputlogic; + +import java.util.ArrayList; +import java.util.List; + +/** + * One revertible input-unit stack for backspace (issue #31). + * + *

Consolidates the length bookkeeping that drives fragment- and whole-word-backspace, which + * was previously three separate fields scattered across {@link InputLogic}: + *

+ * + *

This class owns only the unit-length bookkeeping. The editor side effects (composing-text + * updates, {@code deleteTextBeforeCursor}, stats) and the policy decisions (when to record/pop) + * stay in {@link InputLogic}. Behaviour is identical to the pre-extraction code; the value is a + * single, separately unit-testable home for the corruption-prone boundary math. + */ +final class BackspaceUnitStack { + + /** Composing side: cumulative fragment boundaries (strictly increasing word lengths). */ + private final ArrayList mComposingBoundaries = new ArrayList<>(); + + /** Committed side: total length of the last committed gesture word (0 = none / tap-only). */ + private int mCommittedLength; + /** Committed side: per-fragment lengths of the last committed gesture word. */ + private final ArrayList mCommittedFragmentLengths = new ArrayList<>(); + + // ===== composing side ===== + + boolean hasComposingBoundaries() { + return !mComposingBoundaries.isEmpty(); + } + + /** + * Record a fragment boundary at the given composing-word length. No-op for a non-positive + * length or a duplicate of the current top (the same fragment appended twice in quick + * succession). + */ + void recordComposingBoundary(final int len) { + if (len <= 0) return; + if (!mComposingBoundaries.isEmpty() + && mComposingBoundaries.get(mComposingBoundaries.size() - 1) == len) { + return; + } + mComposingBoundaries.add(len); + } + + /** Drop all composing boundaries. Call after committing / resetting the composing word. */ + void clearComposing() { + if (!mComposingBoundaries.isEmpty()) mComposingBoundaries.clear(); + } + + /** + * Pop the most-recent composing fragment, given the current composing-word length. + * + *

Stale boundaries past {@code currentLen} are trimmed first. Returns the new word length + * the composing word should shrink to: the previous boundary (or {@code 0} for a + * single-fragment word) when the top marker is the current fragment end, or the top boundary + * itself as a defensive fallback when the current fragment end was never recorded. Returns + * {@code -1} when there is no fragment to pop (caller should fall through to char-delete). + */ + int popComposingFragment(final int currentLen) { + if (mComposingBoundaries.isEmpty()) return -1; + while (!mComposingBoundaries.isEmpty() + && mComposingBoundaries.get(mComposingBoundaries.size() - 1) > currentLen) { + mComposingBoundaries.remove(mComposingBoundaries.size() - 1); + } + if (mComposingBoundaries.isEmpty()) return -1; + final int lastBoundary = mComposingBoundaries.get(mComposingBoundaries.size() - 1); + if (lastBoundary == currentLen) { + // Top marker is the end of the current fragment: pop it and shrink to the previous + // marker, or to 0 for a single-fragment word. + mComposingBoundaries.remove(mComposingBoundaries.size() - 1); + return mComposingBoundaries.isEmpty() + ? 0 + : mComposingBoundaries.get(mComposingBoundaries.size() - 1); + } + // Defensive fallback for words whose current fragment end was not recorded. + return lastBoundary; + } + + /** + * Build the per-fragment lengths (deltas) for a composing word of {@code currentLen} that is + * about to be committed: a delta per in-range boundary, plus a trailing fragment for any tail + * past the last boundary. + */ + ArrayList fragmentLengthsForCommit(final int currentLen) { + final ArrayList fragmentLengths = new ArrayList<>(); + if (currentLen <= 0) return fragmentLengths; + int previousBoundary = 0; + for (int i = 0; i < mComposingBoundaries.size(); ++i) { + final int boundary = mComposingBoundaries.get(i); + if (boundary <= previousBoundary || boundary > currentLen) continue; + fragmentLengths.add(boundary - previousBoundary); + previousBoundary = boundary; + } + if (previousBoundary < currentLen) { + fragmentLengths.add(currentLen - previousBoundary); + } + return fragmentLengths; + } + + // ===== committed side ===== + + int committedLength() { + return mCommittedLength; + } + + /** A defensive copy of the committed fragment lengths (snapshot before a pop). */ + ArrayList copyCommittedFragmentLengths() { + return new ArrayList<>(mCommittedFragmentLengths); + } + + /** Replace the committed gesture word: its total length and per-fragment lengths. */ + void setCommitted(final int length, final List fragmentLengths) { + mCommittedLength = length; + mCommittedFragmentLengths.clear(); + mCommittedFragmentLengths.addAll(fragmentLengths); + } + + /** Replace just the committed fragment lengths (after popping one fragment off the top). */ + void setCommittedFragmentLengths(final List fragmentLengths) { + mCommittedFragmentLengths.clear(); + mCommittedFragmentLengths.addAll(fragmentLengths); + } + + /** Reset all committed-gesture state (total length + fragment lengths). */ + void clearCommitted() { + mCommittedLength = 0; + if (!mCommittedFragmentLengths.isEmpty()) mCommittedFragmentLengths.clear(); + } +} diff --git a/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java b/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java index ee4bcdb0..b4cec9e3 100644 --- a/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java +++ b/app/src/main/java/helium314/keyboard/latin/inputlogic/InputLogic.java @@ -127,18 +127,11 @@ public final class InputLogic { * because PHANTOM would make the next letter ALSO write a space, double-spacing. */ private boolean mAutospaceJustWritten; - // Two-thumb typing (#1.1, sub-option PREF_GESTURE_FRAGMENT_BACKSPACE): char-offsets that - // mark the END of each "fragment" appended to the current composing word under manual - // spacing. A fragment is one gesture's output OR one tap's letter. With the pref on, - // backspace pops the most recent fragment in one keystroke instead of deleting one - // character at a time. The list is kept in sync with {@code mWordComposer.getTypedWord()}: - // entries past the current length are filtered at read time, and the list is cleared - // outright whenever the composing word is committed or reset. - private final ArrayList mGestureFragmentBoundaries = new ArrayList<>(); - /** Fragment lengths within the most recent gesture-driven commit. The last entry includes - * the trailing autospace if one was inserted. Used by "Delete last fragment" after the - * composing word has already been auto-committed. */ - private final ArrayList mLastGestureCommittedFragmentLengths = new ArrayList<>(); + // Backspace input-unit stack (#31): the single home for the fragment- / whole-word-backspace + // length bookkeeping. Owns both the active composing word's fragment boundaries (one entry + // per recorded gesture/tap fragment, kept in sync with mWordComposer.getTypedWord()) and the + // last committed gesture word's total + per-fragment lengths. See BackspaceUnitStack. + private final BackspaceUnitStack mBackspaceUnits = new BackspaceUnitStack(); // ---- Unified combining-mode state machine ---------------------------------------------- // After every composing-word-extending event (tap of a letter OR gesture completion), @@ -956,25 +949,16 @@ private void recordFragmentBoundaryIfTracking(final SettingsValues sv) { /** Record a fragment boundary at a known composing-word length. */ private void recordFragmentBoundaryIfTracking(final SettingsValues sv, final int len) { if (!shouldTrackFragmentBoundaries(sv)) return; - if (len <= 0) return; - // Don't record duplicates (e.g. the same fragment appended twice in quick succession). - if (!mGestureFragmentBoundaries.isEmpty() - && mGestureFragmentBoundaries.get(mGestureFragmentBoundaries.size() - 1) == len) { - return; - } - mGestureFragmentBoundaries.add(len); + mBackspaceUnits.recordComposingBoundary(len); } /** Clear all recorded fragment boundaries. Call after committing / resetting the composing word. */ private void clearFragmentBoundaries() { - if (!mGestureFragmentBoundaries.isEmpty()) mGestureFragmentBoundaries.clear(); + mBackspaceUnits.clearComposing(); } private void clearCommittedGestureBackspaceState() { - mLastGestureCommittedLength = 0; - if (!mLastGestureCommittedFragmentLengths.isEmpty()) { - mLastGestureCommittedFragmentLengths.clear(); - } + mBackspaceUnits.clearCommitted(); } // ---- Unified combining-mode helpers -------------------------------------------------- @@ -1177,13 +1161,6 @@ public boolean isInCombiningMode() { * Cleared on any new input that arms combining mode, on cancel, on the next space tap, * and after a suggestion-pick. */ private int mAutoCommitRevertLength; - /** Length of the most recent gesture-driven commit (typed word + autospace). Set in - * {@link #onCombiningGraceExpired} when {@code mWordComposer.isBatchMode()} was true at - * commit time. Consumed by the first backspace tap when - * {@code PREF_COMBINING_BACKSPACE_DELETES_GESTURE_WORD} is on, deleting the whole word - * + space in one keystroke (unless an autocorrect-revert applies first — that always - * wins). Cleared on any new input that arms combining mode. */ - private int mLastGestureCommittedLength; /** Set in {@link #onPickSuggestionManually} when the picker reverted an auto-committed * word; consumed at the bottom of the same method to insert a visible trailing space so * the cursor lands at "the |" instead of "the|". */ @@ -1300,8 +1277,7 @@ private void onCombiningGraceExpired() { // arm "first backspace deletes the whole word" — see handleBackspaceEvent. Stays // 0 for tap-only commits, where char-by-char delete is the right behavior. if (wordHadGestureFragment) { - mLastGestureCommittedLength = writtenChars; - mLastGestureCommittedFragmentLengths.clear(); + final ArrayList committedFragments = new ArrayList<>(); if (!fragmentLengthsAtCommit.isEmpty()) { final int committedDelta = committedLen - typedWordAtCommit.length(); final int lastIndex = fragmentLengthsAtCommit.size() - 1; @@ -1309,13 +1285,14 @@ private void onCombiningGraceExpired() { fragmentLengthsAtCommit.get(lastIndex) + committedDelta + autospaceChars; if (adjustedLastFragmentLen > 0) { fragmentLengthsAtCommit.set(lastIndex, adjustedLastFragmentLen); - mLastGestureCommittedFragmentLengths.addAll(fragmentLengthsAtCommit); + committedFragments.addAll(fragmentLengthsAtCommit); } else if (writtenChars > 0) { - mLastGestureCommittedFragmentLengths.add(writtenChars); + committedFragments.add(writtenChars); } } else if (writtenChars > 0) { - mLastGestureCommittedFragmentLengths.add(writtenChars); + committedFragments.add(writtenChars); } + mBackspaceUnits.setCommitted(writtenChars, committedFragments); } // "keep_alternatives" — fall through, do nothing. } @@ -1337,7 +1314,7 @@ private boolean tryFragmentBackspace(final SettingsValues sv) { && sv.mGestureFragmentBackspace; if (!legacyTracking && !multipartTracking) return false; if (sv.mCombiningBackspaceDeletesGestureWord) return false; - if (mGestureFragmentBoundaries.isEmpty()) return false; + if (!mBackspaceUnits.hasComposingBoundaries()) return false; if (!mWordComposer.isComposingWord()) { clearFragmentBoundaries(); return false; @@ -1345,26 +1322,8 @@ private boolean tryFragmentBackspace(final SettingsValues sv) { if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) return false; final int currentLen = mWordComposer.getTypedWord().length(); - // Filter out stale boundaries past the current length. - while (!mGestureFragmentBoundaries.isEmpty() - && mGestureFragmentBoundaries.get(mGestureFragmentBoundaries.size() - 1) > currentLen) { - mGestureFragmentBoundaries.remove(mGestureFragmentBoundaries.size() - 1); - } - if (mGestureFragmentBoundaries.isEmpty()) return false; - - final int lastBoundary = mGestureFragmentBoundaries.get(mGestureFragmentBoundaries.size() - 1); - final int newLen; - if (lastBoundary == currentLen) { - // The last marker is the end of the current fragment. Pop it and shrink to the - // previous marker, or to 0 for a single-fragment word. - mGestureFragmentBoundaries.remove(mGestureFragmentBoundaries.size() - 1); - newLen = mGestureFragmentBoundaries.isEmpty() - ? 0 - : mGestureFragmentBoundaries.get(mGestureFragmentBoundaries.size() - 1); - } else { - // Defensive fallback for words whose current fragment end was not recorded. - newLen = lastBoundary; - } + final int newLen = mBackspaceUnits.popComposingFragment(currentLen); + if (newLen < 0) return false; final String oldWord = mWordComposer.getTypedWord(); final String newWord = newLen <= 0 @@ -1392,19 +1351,7 @@ private boolean tryFragmentBackspace(final SettingsValues sv) { } private ArrayList getFragmentLengthsForCommit(final int currentLen) { - final ArrayList fragmentLengths = new ArrayList<>(); - if (currentLen <= 0) return fragmentLengths; - int previousBoundary = 0; - for (int i = 0; i < mGestureFragmentBoundaries.size(); ++i) { - final int boundary = mGestureFragmentBoundaries.get(i); - if (boundary <= previousBoundary || boundary > currentLen) continue; - fragmentLengths.add(boundary - previousBoundary); - previousBoundary = boundary; - } - if (previousBoundary < currentLen) { - fragmentLengths.add(currentLen - previousBoundary); - } - return fragmentLengths; + return mBackspaceUnits.fragmentLengthsForCommit(currentLen); } // TODO: on the long term, this method should become private, but it will be @@ -2408,9 +2355,9 @@ private void handleBackspaceEvent(final Event event, final InputTransaction inpu // Combining mode: snapshot the gesture-word-length flag BEFORE cancelCombiningMode // clears it. If non-zero (the previous commit was a gesture), this backspace MIGHT // delete the whole word — see further down, after the autocorrect-revert branch. - final int gestureCommittedLen = mLastGestureCommittedLength; + final int gestureCommittedLen = mBackspaceUnits.committedLength(); final ArrayList gestureCommittedFragmentLengths = - new ArrayList<>(mLastGestureCommittedFragmentLengths); + mBackspaceUnits.copyCommittedFragmentLengths(); // Combining mode: a backspace always cancels the pending commit. The user is // explicitly retracting input; we don't want the timer to fire mid-correction. cancelCombiningMode(); @@ -2578,8 +2525,7 @@ private void handleBackspaceEvent(final Event event, final InputTransaction inpu mConnection.beginBatchEdit(); mConnection.deleteTextBeforeCursor(gestureCommittedFragmentLen); mConnection.endBatchEdit(); - mLastGestureCommittedFragmentLengths.clear(); - mLastGestureCommittedFragmentLengths.addAll(gestureCommittedFragmentLengths); + mBackspaceUnits.setCommittedFragmentLengths(gestureCommittedFragmentLengths); StatsUtils.onBackspaceWordDelete(gestureCommittedFragmentLen); inputTransaction.setRequiresUpdateSuggestions(); return; diff --git a/app/src/test/java/helium314/keyboard/latin/inputlogic/BackspaceUnitStackTest.kt b/app/src/test/java/helium314/keyboard/latin/inputlogic/BackspaceUnitStackTest.kt new file mode 100644 index 00000000..8e45c2ed --- /dev/null +++ b/app/src/test/java/helium314/keyboard/latin/inputlogic/BackspaceUnitStackTest.kt @@ -0,0 +1,157 @@ +// SPDX-License-Identifier: GPL-3.0-only +package helium314.keyboard.latin.inputlogic + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * Direct unit tests for [BackspaceUnitStack] (#31): the fragment- / whole-word-backspace + * length math, previously only exercised indirectly through InputLogic. Pure logic, no + * Robolectric needed. + */ +class BackspaceUnitStackTest { + + private fun stackWith(vararg boundaries: Int) = BackspaceUnitStack().apply { + boundaries.forEach { recordComposingBoundary(it) } + } + + // ---- recordComposingBoundary ---- + + @Test fun `record ignores non-positive lengths`() { + val s = BackspaceUnitStack() + s.recordComposingBoundary(0) + s.recordComposingBoundary(-3) + assertFalse(s.hasComposingBoundaries()) + } + + @Test fun `record dedups the current top`() { + val s = stackWith(3, 3, 3) + // Three identical records collapse to one boundary -> one pop empties it. + assertEquals(0, s.popComposingFragment(3)) + assertEquals(-1, s.popComposingFragment(0)) + } + + @Test fun `clearComposing drops boundaries`() { + val s = stackWith(2, 5) + assertTrue(s.hasComposingBoundaries()) + s.clearComposing() + assertFalse(s.hasComposingBoundaries()) + assertEquals(-1, s.popComposingFragment(5)) + } + + // ---- popComposingFragment ---- + + @Test fun `pop on empty returns -1`() { + assertEquals(-1, BackspaceUnitStack().popComposingFragment(4)) + } + + @Test fun `pop single fragment shrinks to zero`() { + val s = stackWith(5) + assertEquals(0, s.popComposingFragment(5)) + } + + @Test fun `pop multi-fragment shrinks to previous boundary`() { + // Two gesture fragments: "tech"(4) + "nology"(->10). Pop the second -> back to "tech". + val s = stackWith(4, 10) + assertEquals(4, s.popComposingFragment(10)) + // Pop again -> empties. + assertEquals(0, s.popComposingFragment(4)) + assertEquals(-1, s.popComposingFragment(0)) + } + + @Test fun `pop trims stale boundaries past current length`() { + // Boundary 10 is stale (word already shrank to 7 by other means): trimmed, then the + // remaining top (4) != currentLen(7) so the defensive fallback returns 4. + val s = stackWith(4, 10) + assertEquals(4, s.popComposingFragment(7)) + } + + @Test fun `pop falls back to top boundary when current fragment end unrecorded`() { + // Top boundary 4 but the word is length 6 (last 2 chars' boundary never recorded): + // fallback returns the top boundary without popping it. + val s = stackWith(4) + assertEquals(4, s.popComposingFragment(6)) + // Boundary not consumed: a pop at the recorded length still empties it. + assertEquals(0, s.popComposingFragment(4)) + } + + // ---- fragmentLengthsForCommit ---- + + @Test fun `commit lengths are deltas between boundaries`() { + val s = stackWith(4, 10) + assertEquals(listOf(4, 6), s.fragmentLengthsForCommit(10)) + } + + @Test fun `commit lengths add a trailing tail past the last boundary`() { + val s = stackWith(4) + // "tech"(4) + 2 trailing chars with no recorded boundary -> [4, 2]. + assertEquals(listOf(4, 2), s.fragmentLengthsForCommit(6)) + } + + @Test fun `commit lengths for an untracked word are one whole fragment`() { + assertEquals(listOf(5), BackspaceUnitStack().fragmentLengthsForCommit(5)) + } + + @Test fun `commit lengths for zero length are empty`() { + assertEquals(emptyList(), stackWith(4).fragmentLengthsForCommit(0)) + } + + @Test fun `commit lengths ignore boundaries past current length`() { + val s = stackWith(4, 10) + // Committing at length 7: boundary 10 is out of range, tail = 7-4 = 3. + assertEquals(listOf(4, 3), s.fragmentLengthsForCommit(7)) + } + + // ---- committed side ---- + + @Test fun `setCommitted stores length and a defensive copy of fragments`() { + val s = BackspaceUnitStack() + val src = arrayListOf(4, 7) + s.setCommitted(11, src) + assertEquals(11, s.committedLength()) + assertEquals(listOf(4, 7), s.copyCommittedFragmentLengths()) + // Mutating the source after the call must not leak into the stack. + src.add(99) + assertEquals(listOf(4, 7), s.copyCommittedFragmentLengths()) + // The returned copy is defensive too. + s.copyCommittedFragmentLengths().add(99) + assertEquals(listOf(4, 7), s.copyCommittedFragmentLengths()) + } + + @Test fun `setCommitted with no fragments still arms whole-word delete`() { + // A gesture word committed with no recorded fragments: committedLength must STILL be + // set (it arms the first-backspace whole-word delete) while the fragment list stays + // empty. The empty-list commit is the easiest place to silently drop the length. + val s = BackspaceUnitStack() + s.setCommitted(7, emptyList()) + assertEquals(7, s.committedLength()) + assertEquals(emptyList(), s.copyCommittedFragmentLengths()) + } + + @Test fun `setCommittedFragmentLengths replaces fragments but keeps length`() { + val s = BackspaceUnitStack() + s.setCommitted(11, listOf(4, 7)) + s.setCommittedFragmentLengths(listOf(4)) // popped the trailing fragment off the editor + assertEquals(listOf(4), s.copyCommittedFragmentLengths()) + assertEquals(11, s.committedLength()) + } + + @Test fun `clearCommitted resets length and fragments`() { + val s = BackspaceUnitStack() + s.setCommitted(11, listOf(4, 7)) + s.clearCommitted() + assertEquals(0, s.committedLength()) + assertEquals(emptyList(), s.copyCommittedFragmentLengths()) + } + + @Test fun `composing and committed sides are independent`() { + val s = stackWith(4, 10) + s.setCommitted(11, listOf(4, 7)) + s.clearComposing() + // Clearing composing left the committed side intact. + assertEquals(11, s.committedLength()) + assertEquals(listOf(4, 7), s.copyCommittedFragmentLengths()) + } +}