From 500dea6ad45d82d745902103067885f58b42fddf Mon Sep 17 00:00:00 2001
From: Kurt Partridge <kep@google.com>
Date: Thu, 16 May 2013 18:39:40 -0700
Subject: [PATCH] Fix punctuation logging

Punctuation is currently put into the same LogUnit as the
following word.  This is counter-intuitive and makes
post-processing more complicated.

This change causes punctuation to be put into its own
LogUnit.

A tricky case is the phantom space.  A phantom space is
inserted right before other text, whose MotionEvents have
already been collected and inserted into mCurrentLogBuffer.
Phantom spaces are handled by creating their own LogUnit and
inserting it into the LogBuffer right before
mCurrentLogBuffer is shifted in.

Change-Id: I102d7cd93982ad37ba0abb6c1ac0f66b85b5f95d
---
 .../android/inputmethod/latin/LatinIME.java   |  8 ++--
 .../inputmethod/research/ResearchLogger.java  | 46 +++++++++++++++----
 2 files changed, 40 insertions(+), 14 deletions(-)

diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java
index ad0c1c8b36..883c9469bc 100644
--- a/java/src/com/android/inputmethod/latin/LatinIME.java
+++ b/java/src/com/android/inputmethod/latin/LatinIME.java
@@ -2021,9 +2021,6 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
     // Returns true if we did an autocorrection, false otherwise.
     private boolean handleSeparator(final int primaryCode, final int x, final int y,
             final int spaceState) {
-        if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) {
-            ResearchLogger.latinIME_handleSeparator(primaryCode, mWordComposer.isComposingWord());
-        }
         boolean didAutoCorrect = false;
         if (mWordComposer.isCursorFrontOrMiddleOfComposingWord()) {
             // If we are in the middle of a recorrection, we need to commit the recorrection
@@ -2047,6 +2044,9 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
                 mSettings.getCurrent().isUsuallyPrecededBySpace(primaryCode)) {
             promotePhantomSpace();
         }
+        if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) {
+            ResearchLogger.latinIME_handleSeparator(primaryCode, mWordComposer.isComposingWord());
+        }
         sendKeyCodePoint(primaryCode);
 
         if (Constants.CODE_SPACE == primaryCode) {
@@ -2594,10 +2594,10 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen
     public void promotePhantomSpace() {
         if (mSettings.getCurrent().shouldInsertSpacesAutomatically()
                 && !mConnection.textBeforeCursorLooksLikeURL()) {
-            sendKeyCodePoint(Constants.CODE_SPACE);
             if (ProductionFlag.USES_DEVELOPMENT_ONLY_DIAGNOSTICS) {
                 ResearchLogger.latinIME_promotePhantomSpace();
             }
+            sendKeyCodePoint(Constants.CODE_SPACE);
         }
     }
 
diff --git a/java/src/com/android/inputmethod/research/ResearchLogger.java b/java/src/com/android/inputmethod/research/ResearchLogger.java
index c5a6f6bae3..f426d58d56 100644
--- a/java/src/com/android/inputmethod/research/ResearchLogger.java
+++ b/java/src/com/android/inputmethod/research/ResearchLogger.java
@@ -198,6 +198,11 @@ public class ResearchLogger implements SharedPreferences.OnSharedPreferenceChang
     private Handler mUserRecordingTimeoutHandler;
     private static final long USER_RECORDING_TIMEOUT_MS = 30L * DateUtils.SECOND_IN_MILLIS;
 
+    // Stores a temporary LogUnit while generating a phantom space.  Needed because phantom spaces
+    // are issued out-of-order, immediately before the characters generated by other operations that
+    // have already outputted LogStatements.
+    private LogUnit mPhantomSpaceLogUnit = null;
+
     private ResearchLogger() {
         mStatistics = Statistics.getInstance();
     }
@@ -1291,17 +1296,32 @@ public class ResearchLogger implements SharedPreferences.OnSharedPreferenceChang
     /**
      * Log a call to LatinIME.sendKeyCodePoint().
      *
-     * SystemResponse: The IME is inserting text into the TextView for numbers, fixed strings, or
-     * some other unusual mechanism.
+     * SystemResponse: The IME is inserting text into the TextView for non-word-constituent,
+     * strings (separators, numbers, other symbols).
      */
     private static final LogStatement LOGSTATEMENT_LATINIME_SENDKEYCODEPOINT =
             new LogStatement("LatinIMESendKeyCodePoint", true, false, "code");
     public static void latinIME_sendKeyCodePoint(final int code) {
         final ResearchLogger researchLogger = getInstance();
-        researchLogger.enqueueEvent(LOGSTATEMENT_LATINIME_SENDKEYCODEPOINT,
-                Constants.printableCode(scrubDigitFromCodePoint(code)));
-        if (Character.isDigit(code)) {
-            researchLogger.setCurrentLogUnitContainsDigitFlag();
+        final LogUnit phantomSpaceLogUnit = researchLogger.mPhantomSpaceLogUnit;
+        if (phantomSpaceLogUnit == null) {
+            researchLogger.enqueueEvent(LOGSTATEMENT_LATINIME_SENDKEYCODEPOINT,
+                    Constants.printableCode(scrubDigitFromCodePoint(code)));
+            if (Character.isDigit(code)) {
+                researchLogger.setCurrentLogUnitContainsDigitFlag();
+            }
+            researchLogger.commitCurrentLogUnit();
+        } else {
+            researchLogger.enqueueEvent(phantomSpaceLogUnit, LOGSTATEMENT_LATINIME_SENDKEYCODEPOINT,
+                    Constants.printableCode(scrubDigitFromCodePoint(code)));
+            if (Character.isDigit(code)) {
+                phantomSpaceLogUnit.setMayContainDigit();
+            }
+            researchLogger.mMainLogBuffer.shiftIn(phantomSpaceLogUnit);
+            if (researchLogger.mUserRecordingLogBuffer != null) {
+                researchLogger.mUserRecordingLogBuffer.shiftIn(phantomSpaceLogUnit);
+            }
+            researchLogger.mPhantomSpaceLogUnit = null;
         }
     }
 
@@ -1311,12 +1331,18 @@ public class ResearchLogger implements SharedPreferences.OnSharedPreferenceChang
      * SystemResponse: The IME is inserting a real space in place of a phantom space.
      */
     private static final LogStatement LOGSTATEMENT_LATINIME_PROMOTEPHANTOMSPACE =
-            new LogStatement("LatinIMEPromotPhantomSpace", false, false);
+            new LogStatement("LatinIMEPromotePhantomSpace", false, false);
     public static void latinIME_promotePhantomSpace() {
+        // A phantom space is always added before the text that triggered it.  The triggering text
+        // and the events that created it will be in mCurrentLogUnit, but the phantom space should
+        // be in its own LogUnit, committed before the triggering text.  Although it is created
+        // here, it is not added to the LogBuffer until the following call to
+        // latinIME_sendKeyCodePoint, because SENDKEYCODEPOINT LogStatement also must go into that
+        // LogUnit.
         final ResearchLogger researchLogger = getInstance();
-        final LogUnit logUnit;
-        logUnit = researchLogger.mMainLogBuffer.peekLastLogUnit();
-        researchLogger.enqueueEvent(logUnit, LOGSTATEMENT_LATINIME_PROMOTEPHANTOMSPACE);
+        researchLogger.mPhantomSpaceLogUnit = new LogUnit();
+        researchLogger.enqueueEvent(researchLogger.mPhantomSpaceLogUnit,
+                LOGSTATEMENT_LATINIME_PROMOTEPHANTOMSPACE);
     }
 
     /**
-- 
GitLab