review suggestions

Expensify · Dec 12, 2024 · b39723f · b39723f
1 parent cea5128
commit b39723f
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 43 deletions.
diff --git a/android/build.gradle b/android/build.gradle
@@ -176,7 +176,7 @@ dependencies {
   implementation "com.facebook.react:hermes-android" // version substituted by RNGP
   implementation project(":react-native-reanimated")
 
-  testImplementation 'junit:junit:4.13.2'
+  testImplementation "junit:junit:4.13.2"
 }
 
 if (isNewArchitectureEnabled()) {

diff --git a/android/src/main/java/com/expensify/livemarkdown/MarkdownParser.java b/android/src/main/java/com/expensify/livemarkdown/MarkdownParser.java
@@ -54,8 +54,8 @@ private List<MarkdownRange> parseRanges(String rangesJSON, String innerText) {
     } catch (JSONException e) {
       return Collections.emptyList();
     }
-    splitRangesOnEmojis(markdownRanges, "italic");
-    splitRangesOnEmojis(markdownRanges, "strikethrough");
+    markdownRanges = splitRangesOnEmojis(markdownRanges, "italic");
+    markdownRanges = splitRangesOnEmojis(markdownRanges, "strikethrough");
     return markdownRanges;
   }
   public synchronized List<MarkdownRange> parse(@NonNull String text, int parserId) {

diff --git a/android/src/main/java/com/expensify/livemarkdown/MarkdownRange.java b/android/src/main/java/com/expensify/livemarkdown/MarkdownRange.java
@@ -36,4 +36,32 @@ public int getLength() {
   public int getDepth() {
     return mDepth;
   }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) {
+      return true;
+    }
+
+    if (o instanceof MarkdownRange other) {
+      return this.mType.equals(other.mType)
+        && this.mStart == other.mStart
+        && this.mEnd == other.mEnd
+        && this.mLength == other.mLength
+        && this.mDepth == other.mDepth;
+    }
+    return false;
+  }
+
+  @NonNull
+  @Override
+  public String toString() {
+    return "MarkdownRange{" +
+      "type='" + mType + "'" +
+      ", start=" + mStart +
+      ", end=" + mEnd +
+      ", length=" + mLength +
+      ", depth=" + mDepth +
+      "}";
+  }
 }
diff --git a/android/src/main/java/com/expensify/livemarkdown/RangeSplitter.java b/android/src/main/java/com/expensify/livemarkdown/RangeSplitter.java
@@ -1,13 +1,15 @@
 package com.expensify.livemarkdown;
 
+import androidx.annotation.NonNull;
+
 import java.util.ArrayList;
 import java.util.List;
 
 public class RangeSplitter {
-  public static void splitRangesOnEmojis(List<MarkdownRange> markdownRanges, String type) {
-    List<MarkdownRange> emojiRanges = new ArrayList<>();
-    List<MarkdownRange> oldRanges = new ArrayList<>(markdownRanges);
-    markdownRanges.clear();
+  public static ArrayList<MarkdownRange> splitRangesOnEmojis(@NonNull List<MarkdownRange> markdownRanges, @NonNull String type) {
+    ArrayList<MarkdownRange> emojiRanges = new ArrayList<>();
+    ArrayList<MarkdownRange> oldRanges = new ArrayList<>(markdownRanges);
+    ArrayList<MarkdownRange> newRanges = new ArrayList<>();
     for (MarkdownRange range : oldRanges) {
       if (range.getType().equals("emoji")) {
         emojiRanges.add(range);
@@ -19,26 +21,33 @@ public static void splitRangesOnEmojis(List<MarkdownRange> markdownRanges, Strin
     while (i < oldRanges.size()) {
       MarkdownRange currentRange = oldRanges.get(i);
       if (!currentRange.getType().equals(type)) {
-        markdownRanges.add(currentRange);
+        newRanges.add(currentRange);
         i += 1;
         continue;
       }
 
-      // Split range
+      // Iterate through all emoji ranges before the end of the current range, splitting the current range at each intersection.
       while (j < emojiRanges.size()) {
         MarkdownRange emojiRange = emojiRanges.get(j);
-        if (emojiRange.getStart() > currentRange.getEnd()) break;
+        if (emojiRange.getStart() > currentRange.getEnd()) {
+          break;
+        }
 
-        if (emojiRange.getStart() >= currentRange.getStart() && emojiRange.getEnd() <= currentRange.getEnd()) {
-          MarkdownRange newRange = new MarkdownRange(currentRange.getType(), currentRange.getStart(), emojiRange.getStart() - currentRange.getStart(), currentRange.getDepth());
-          currentRange = new MarkdownRange(currentRange.getType(), emojiRange.getEnd(), currentRange.getEnd() - emojiRange.getEnd(), currentRange.getDepth());
+        int currentStart = currentRange.getStart();
+        int currentEnd = currentRange.getEnd();
+        int emojiStart = emojiRange.getStart();
+        int emojiEnd = emojiRange.getEnd();
+        if (emojiStart >= currentStart && emojiEnd <= currentEnd) { // Intersection
+          MarkdownRange newRange = new MarkdownRange(currentRange.getType(), currentStart, emojiStart - currentStart, currentRange.getDepth());
+          currentRange = new MarkdownRange(currentRange.getType(), emojiEnd, currentEnd - emojiEnd, currentRange.getDepth());
 
-          markdownRanges.add(newRange);
+          newRanges.add(newRange);
         }
         j += 1;
       }
-      markdownRanges.add(currentRange);
+      newRanges.add(currentRange);
       i += 1;
     }
+    return newRanges;
   }
 }
diff --git a/android/src/test/java/com/expensify/livemarkdown/RangeSplitterTest.java b/android/src/test/java/com/expensify/livemarkdown/RangeSplitterTest.java
@@ -2,7 +2,6 @@
 
 import static com.expensify.livemarkdown.RangeSplitter.splitRangesOnEmojis;
 
-
 import static org.junit.Assert.assertEquals;
 
 import org.junit.Test;
@@ -13,54 +12,47 @@
 
 public class RangeSplitterTest {
 
-  private void testRange(MarkdownRange range, int start, int end, String type) {
-    assertEquals(start, range.getStart());
-    assertEquals(end, range.getEnd());
-    assertEquals(type, range.getType());
-  }
-
   @Test
   public void testNoOverlap() {
     List<MarkdownRange> markdownRanges = new ArrayList<>();
     markdownRanges.add(new MarkdownRange("strikethrough", 0, 10, 1));
     markdownRanges.add(new MarkdownRange("emoji", 12, 2, 1));
 
-    splitRangesOnEmojis(markdownRanges, "strikethrough");
+    markdownRanges = splitRangesOnEmojis(markdownRanges, "strikethrough");
 
     assertEquals(2, markdownRanges.size());
-
-    testRange(markdownRanges.get(0), 0, 10, "strikethrough");
-    testRange(markdownRanges.get(1), 12, 14, "emoji");
+    assertEquals(new MarkdownRange("strikethrough", 0, 10, 1), markdownRanges.get(0));
+    assertEquals(new MarkdownRange("emoji", 12, 2, 1), markdownRanges.get(1));
   }
 
   @Test
-  public void testOverlapWrongType() {
+  public void testOverlapDifferentType() {
     List<MarkdownRange> markdownRanges = new ArrayList<>();
     markdownRanges.add(new MarkdownRange("strikethrough", 0, 10, 1));
     markdownRanges.add(new MarkdownRange("emoji", 3, 4, 1));
 
-    splitRangesOnEmojis(markdownRanges, "italic");
+    markdownRanges = splitRangesOnEmojis(markdownRanges, "italic");
 
     assertEquals(2, markdownRanges.size());
-    testRange(markdownRanges.get(0), 0, 10, "strikethrough");
-    testRange(markdownRanges.get(1), 3, 7, "emoji");
+    assertEquals(new MarkdownRange("strikethrough", 0, 10, 1), markdownRanges.get(0));
+    assertEquals(new MarkdownRange("emoji", 3, 4, 1), markdownRanges.get(1));
   }
 
   @Test
   public void testSingleOverlap() {
     List<MarkdownRange> markdownRanges = new ArrayList<>();
     markdownRanges.add(new MarkdownRange("strikethrough", 0, 10, 1));
-    markdownRanges.add(new MarkdownRange("emoji", 3, 4, 1)); // This should split the strikethrough range
+    markdownRanges.add(new MarkdownRange("emoji", 3, 4, 1)); // This range should split the strikethrough range
 
-    splitRangesOnEmojis(markdownRanges, "strikethrough");
+    markdownRanges = splitRangesOnEmojis(markdownRanges, "strikethrough");
 
     // Sort is needed because ranges may get mixed while splitting
     Collections.sort(markdownRanges, (r1, r2) -> Integer.compare(r1.getStart(), r2.getStart()));
 
     assertEquals(3, markdownRanges.size());
-    testRange(markdownRanges.get(0), 0, 3, "strikethrough");
-    testRange(markdownRanges.get(1), 3, 7, "emoji");
-    testRange(markdownRanges.get(2), 7, 10, "strikethrough");
+    assertEquals(new MarkdownRange("strikethrough", 0, 3, 1), markdownRanges.get(0));
+    assertEquals(new MarkdownRange("emoji", 3, 4, 1), markdownRanges.get(1));
+    assertEquals(new MarkdownRange("strikethrough", 7, 3, 1), markdownRanges.get(2));
   }
 
   @Test
@@ -72,18 +64,18 @@ public void testMultipleOverlapsMultipleTypes() {
     markdownRanges.add(new MarkdownRange("emoji", 8, 2, 1));
     markdownRanges.add(new MarkdownRange("strikethrough", 22, 5, 1));
 
-    splitRangesOnEmojis(markdownRanges, "strikethrough");
+    markdownRanges = splitRangesOnEmojis(markdownRanges, "strikethrough");
 
     // Sort is needed because ranges may get mixed while splitting
     Collections.sort(markdownRanges, (r1, r2) -> Integer.compare(r1.getStart(), r2.getStart()));
 
     assertEquals(7, markdownRanges.size());
-    testRange(markdownRanges.get(0), 0, 20, "italic");
-    testRange(markdownRanges.get(1), 2, 3, "strikethrough");
-    testRange(markdownRanges.get(2), 3, 4, "emoji");
-    testRange(markdownRanges.get(3), 4, 8, "strikethrough");
-    testRange(markdownRanges.get(4), 8, 10, "emoji");
-    testRange(markdownRanges.get(5), 10, 14, "strikethrough");
-    testRange(markdownRanges.get(6), 22, 27, "strikethrough");
+    assertEquals(new MarkdownRange("italic", 0, 20, 1), markdownRanges.get(0));
+    assertEquals(new MarkdownRange("strikethrough", 2, 1, 1), markdownRanges.get(1));
+    assertEquals(new MarkdownRange("emoji", 3, 1, 1), markdownRanges.get(2));
+    assertEquals(new MarkdownRange("strikethrough", 4, 4, 1), markdownRanges.get(3));
+    assertEquals(new MarkdownRange("emoji", 8, 2, 1), markdownRanges.get(4));
+    assertEquals(new MarkdownRange("strikethrough", 10, 4, 1), markdownRanges.get(5));
+    assertEquals(new MarkdownRange("strikethrough", 22, 5, 1), markdownRanges.get(6));
   }
 }