Restructuring of the compareTo() method of MsgId, fix small issue…

… with JS regexes
floscher · Feb 6, 2022 · 42b56ed · 42b56ed
1 parent c740ca3
commit 42b56ed
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 15 deletions.
diff --git a/i18n/src/commonMain/kotlin/org/openstreetmap/josm/gradle/plugin/i18n/io/MsgId.kt b/i18n/src/commonMain/kotlin/org/openstreetmap/josm/gradle/plugin/i18n/io/MsgId.kt
@@ -7,19 +7,20 @@ package org.openstreetmap.josm.gradle.plugin.i18n.io
  *   (e.g. for disambiguation of multiple identicals strings that should be translated differently in different situations)
  */
 public data class MsgId(val id: MsgStr, val context: String? = null): Comparable<MsgId> {
-  override fun compareTo(other: MsgId): Int {
-    val grammaticalFormsComparison = id.strings.size.compareTo(other.id.strings.size)
-    if (grammaticalFormsComparison != 0) {
-      return grammaticalFormsComparison
+  override fun compareTo(other: MsgId): Int =
+    // first sort IDs with fewer grammatical forms first
+    id.strings.size.compareTo(other.id.strings.size).takeIf { it != 0 }
+    // fallback to comparing contexts
+    ?: when {
+      // If my context is null, while the other's context is non-null: I go first. Otherwise fallback to next check.
+      context == null -> other.context?.let { -1 }
+      // If my context is non-null, while the other's context is null: The other one goes first
+      other.context == null -> 1
+      // If both contexts are non-null: If they are different, use the order of the contexts. Otherwise fallback to next check.
+      else -> context.compareTo(other.context).takeIf { it != 0 }
     }
-
-    if (context == null && other.context != null) {
-      return -1
-    } else if (context != null && other.context == null) {
-      return 1
-    } else if (context != null && other.context != null) {
-      context.compareTo(other.context).takeIf { it != 0 }?.apply { return this }
-    }
-    return id.strings.zip(other.id.strings).map { (myString, otherString) -> myString.compareTo(otherString) }.firstOrNull { it != 0 } ?: 0
-  }
+    // Go through all the strings and compare to the corresponding string of the other object. Return the comparison result of the first difference.
+    ?: id.strings.zip(other.id.strings).map { (myString, otherString) -> myString.compareTo(otherString) }.firstOrNull { it != 0 }
+    // At this point the objects are identical (same number of strings, same context and all strings are the same)
+    ?: 0
 }
diff --git a/i18n/src/commonMain/kotlin/org/openstreetmap/josm/gradle/plugin/i18n/io/PoFileDecoder.kt b/i18n/src/commonMain/kotlin/org/openstreetmap/josm/gradle/plugin/i18n/io/PoFileDecoder.kt
@@ -5,7 +5,8 @@ public object PoFileDecoder: I18nFileDecoder {
   private val REGEX_MSGID = Regex("^msgid \"(.*)\"$")
   private val REGEX_MSGID_PLURAL = Regex("^msgid_plural \"(.*)\"$")
   private val REGEX_MSGSTR = Regex("^msgstr \"(.*)\"$")
-  private val REGEX_MSGSTR_INDEXED = Regex("^msgstr\\[([0-9]+)] \"(.*)\"$")
+  @Suppress("RegExpRedundantEscape") // Actually when compiling to JS, we need to escape the closing bracket `\\]`
+  private val REGEX_MSGSTR_INDEXED = Regex("^msgstr\\[([0-9]+)\\] \"(.*)\"$")
 
   /**
    * Matches every time a line ends with a double quote and the next line starts with a double quote.

diff --git a/i18n/src/commonTest/kotlin/org/openstreetmap/josm/gradle/plugin/i18n/io/MsgIdTest.kt b/i18n/src/commonTest/kotlin/org/openstreetmap/josm/gradle/plugin/i18n/io/MsgIdTest.kt
@@ -10,15 +10,18 @@ class MsgIdTest {
       MsgId(MsgStr("A")), MsgId(MsgStr("B")), MsgId(MsgStr("C")),
       MsgId(MsgStr("B"), "a"), MsgId(MsgStr("B"), "b"), MsgId(MsgStr("B"), "c"),
       MsgId(MsgStr("A", "B")), MsgId(MsgStr("A", "A")), MsgId(MsgStr("B", "C")),
+      MsgId(MsgStr("A")), MsgId(MsgStr("C"), "b"),
     )
 
     assertEquals(
       listOf(
+        MsgId(MsgStr("A")),
         MsgId(MsgStr("A")),
         MsgId(MsgStr("B")),
         MsgId(MsgStr("C")),
         MsgId(MsgStr("B"), "a"),
         MsgId(MsgStr("B"), "b"),
+        MsgId(MsgStr("C"), "b"),
         MsgId(MsgStr("B"), "c"),
         MsgId(MsgStr("A", "A")),
         MsgId(MsgStr("A", "B")),
@@ -33,11 +36,13 @@ class MsgIdTest {
         MsgId(MsgStr("A", "B")),
         MsgId(MsgStr("A", "A")),
         MsgId(MsgStr("B"), "c"),
+        MsgId(MsgStr("C"), "b"),
         MsgId(MsgStr("B"), "b"),
         MsgId(MsgStr("B"), "a"),
         MsgId(MsgStr("C")),
         MsgId(MsgStr("B")),
         MsgId(MsgStr("A")),
+        MsgId(MsgStr("A")),
       ),
       baseList.sortedDescending()
     )