Update the message tokenizer

Updates the message tokenizer to accept an arg anywhere in the message and to allow unescaped % if it is not followed by a number. Also add a TODO to handle references.
surinder83singh · Jan 13, 2016 · 2ff8ff7 · 2ff8ff7
1 parent 2a5b61b
commit 2ff8ff7
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 25 deletions.
diff --git a/.idea/gradle.xml b/.idea/gradle.xml
diff --git a/.idea/misc.xml b/.idea/misc.xml
diff --git a/Blockly.iml b/Blockly.iml
@@ -8,7 +8,7 @@
       </configuration>
     </facet>
   </component>
-  <component name="NewModuleRootManager" inherit-compiler-output="true">
+  <component name="NewModuleRootManager" LANGUAGE_LEVEL="JDK_1_7" inherit-compiler-output="true">
     <exclude-output />
     <content url="file://$MODULE_DIR$">
       <excludeFolder url="file://$MODULE_DIR$/.gradle" />

diff --git a/app/app.iml b/app/app.iml
@@ -12,10 +12,12 @@
         <option name="SELECTED_TEST_ARTIFACT" value="_android_test_" />
         <option name="ASSEMBLE_TASK_NAME" value="assembleDebug" />
         <option name="COMPILE_JAVA_TASK_NAME" value="compileDebugSources" />
-        <option name="SOURCE_GEN_TASK_NAME" value="generateDebugSources" />
         <option name="ASSEMBLE_TEST_TASK_NAME" value="assembleDebugAndroidTest" />
         <option name="COMPILE_JAVA_TEST_TASK_NAME" value="compileDebugAndroidTestSources" />
-        <option name="TEST_SOURCE_GEN_TASK_NAME" value="generateDebugAndroidTestSources" />
+        <afterSyncTasks>
+          <task>generateDebugAndroidTestSources</task>
+          <task>generateDebugSources</task>
+        </afterSyncTasks>
         <option name="ALLOW_USER_CONFIGURATION" value="false" />
         <option name="MANIFEST_FILE_RELATIVE_PATH" value="/src/main/AndroidManifest.xml" />
         <option name="RES_FOLDER_RELATIVE_PATH" value="/src/main/res" />
@@ -24,7 +26,7 @@
       </configuration>
     </facet>
   </component>
-  <component name="NewModuleRootManager" inherit-compiler-output="false">
+  <component name="NewModuleRootManager" LANGUAGE_LEVEL="JDK_1_7" inherit-compiler-output="false">
     <output url="file://$MODULE_DIR$/build/intermediates/classes/debug" />
     <output-test url="file://$MODULE_DIR$/build/intermediates/classes/androidTest/debug" />
     <exclude-output />

diff --git a/app/src/androidTest/java/com/google/blockly/blocks/BlockTest.java b/app/src/androidTest/java/com/google/blockly/blocks/BlockTest.java
@@ -8,6 +8,8 @@
 import org.json.JSONException;
 import org.json.JSONObject;
 
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -90,19 +92,33 @@ public void testJson() {
     public void testMessageTokenizer() {
         String testMessage = "%%5 should have %1 %12 6 tokens %999 in the end";
         List<String> tokens = Block.tokenizeMessage(testMessage);
-        assertEquals("Should have 6 tokens: " + tokens.toString(), 6, tokens.size());
+        List<String> expected = Arrays.asList(
+                new String[] {"%%5 should have", "%1", "%12", "6 tokens", "%999", "in the end"});
+        assertListsMatch(expected, tokens);
 
         testMessage = "This has no args %%5";
         tokens = Block.tokenizeMessage(testMessage);
         assertEquals("Should have 1 token: " + tokens.toString(), 1, tokens.size());
-        assertTrue("Only token should be the original string: " + tokens.toString(),
-                TextUtils.equals(testMessage, tokens.get(0)));
+        assertEquals("Only token should be the original string: " + tokens.toString(),
+                testMessage, tokens.get(0));
 
         testMessage = "%1";
         tokens = Block.tokenizeMessage(testMessage);
         assertEquals("Should have 1 token: " + tokens.toString(), 1, tokens.size());
-        assertTrue("Only token should be the original string: " + tokens.toString(),
-                TextUtils.equals(testMessage, tokens.get(0)));
+        assertEquals("Only token should be the original string: " + tokens.toString(),
+                testMessage, tokens.get(0));
+
+        testMessage = "%Hello";
+        tokens = Block.tokenizeMessage(testMessage);
+        assertEquals("Should have 1 token: " + tokens.toString(), 1, tokens.size());
+        assertEquals("Only token should be the original string: " + tokens.toString(),
+                testMessage, tokens.get(0));
+
+
+        testMessage = "%Hello%1World%";
+        tokens = Block.tokenizeMessage(testMessage);
+        expected = Arrays.asList(new String[] {"%Hello", "%1", "World%"});
+        assertListsMatch(expected, tokens);
     }
 
     public void testBlockFactory() {
@@ -120,4 +136,11 @@ public void testBlockFactory() {
         assertEquals("Frankenblock has the wrong number of inputs", 3,
                 frankenblock.getInputs().size());
     }
+
+    private void assertListsMatch(List<String> expected, List<String> actual) {
+        assertEquals("Wrong number of items in the list.", expected.size(), actual.size());
+        for (int i = 0; i < expected.size(); i++) {
+            assertEquals("Item " + i + " does not match.", expected.get(i), actual.get(i));
+        }
+    }
 }
diff --git a/app/src/main/java/com/google/blockly/blocks/Block.java b/app/src/main/java/com/google/blockly/blocks/Block.java
@@ -132,6 +132,10 @@ public static Block fromJson(String name, JSONObject json) {
                 // If there's no args for this message use an empty array.
                 args = new JSONArray();
             }
+
+            if (message.matches("^%[a-zA-Z_][a-zA-Z_0-9]*$")) {
+                // TODO: load the message from resources.
+            }
             // Split on all argument indices of the form "%N" where N is a number from 1 to
             // the number of args without removing them.
             List<String> tokens = tokenizeMessage(message);
@@ -210,7 +214,9 @@ public static Block fromJson(String name, JSONObject json) {
 
     /**
      * Breaks a block message up into args and text. The returned Strings should all either
-     * exactly match "^%\\d+$" if they are an arg or else are just text for a label.
+     * exactly match "^%\\d+$" if they are an arg or else are just text for a label. %[0-9]+ will
+     * always be treated as an argument regardless of where in the string it appears, unless the
+     * % is escaped (eg "Escaped %%5 has no args")
      *
      * @param message The message to tokenize.
      * @return A list of Strings that are either an arg or plain text.
@@ -229,45 +235,44 @@ public static Block fromJson(String name, JSONObject json) {
             if (currChar == '%') {
                 if (i + 1 < length) {
                     char nextChar = message.charAt(i + 1);
-                    if (nextChar == '%') {
+                    if (nextChar == '%' || !Character.isDigit(nextChar)) {
+                        // If we have %% or this is not an arg don't pull it out of the string.
                         i++;
                         continue;
-                    } else if (!Character.isDigit(nextChar)) {
-                        throw new IllegalArgumentException(
-                                "% must be escaped by another % when not denoting an arg."
-                                + " Bad message: " + message);
                     }
                 } else {
-                    throw new IllegalArgumentException(
-                            "Messages may not end in an unescaped %. Bad message: " + message);
+                    // Done processing the string. Final % will be included in the last token.
+                    continue;
                 }
                 foundPercent = true;
                 lastPercent = i;
             } else if (foundPercent) {
                 if (Character.isDigit(currChar)) {
                     continue;
-                } else if (currChar == ' ') {
+                } else {
                     String potentialText = message.substring(lastSplit, lastPercent).trim();
                     if (!TextUtils.isEmpty(potentialText)) {
                         result.add(potentialText);
                     }
                     result.add(message.substring(lastPercent, i));
                     lastSplit = i;
                     foundPercent = false;
-                } else {
-                    throw new IllegalArgumentException(
-                            "Message args may only contain a % followed by numbers, then a space."
-                            + " Bad message:" + message);
                 }
             }
         }
         if (lastSplit != message.length() - 1) {
             // We have remaining pieces to split
             if (lastPercent > lastSplit) {
-                result.add(message.substring(lastSplit, lastPercent));
+                String potentialText = message.substring(lastSplit, lastPercent).trim();
+                if (!TextUtils.isEmpty(potentialText)) {
+                    result.add(potentialText);
+                }
                 result.add(message.substring(lastPercent, message.length()));
             } else {
-                result.add(message.substring(lastSplit, message.length()));
+                String potentialText = message.substring(lastSplit, message.length()).trim();
+                if (!TextUtils.isEmpty(potentialText)) {
+                    result.add(potentialText);
+                }
             }
         }
 
@@ -347,6 +352,7 @@ public Builder setUuid(String uuid) {
         }
 
         public Builder setColour(int hsvColor) {
+            hsvColor = Math.min(360, Math.max(0, hsvColor));
             mColourHue = hsvColor;
             return this;
         }