From 2cd2712589179ba80e4bb7ca09d03fcf56aeab05 Mon Sep 17 00:00:00 2001
From: Islon Scherer <i_desouzascherer@apple.com>
Date: Thu, 25 Jul 2024 09:27:38 +0200
Subject: [PATCH] Fix property parsing bug in the cli (#596)

---
 .../org/pkl/commons/cli/commands/BaseOptions.kt    | 14 ++++++++++++--
 .../kotlin/org/pkl/commons/cli/BaseCommandTest.kt  |  4 ++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt
index 99ca3094..f924278d 100644
--- a/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt
+++ b/pkl-commons-cli/src/main/kotlin/org/pkl/commons/cli/commands/BaseOptions.kt
@@ -64,6 +64,16 @@ class BaseOptions : OptionGroup() {
             throw CliException(message)
           }
       }
+
+    fun RawOption.associateProps():
+      OptionWithValues<Map<String, String>, Pair<String, String>, Pair<String, String>> {
+      return convert {
+          val parts = it.split("=")
+          if (parts.size <= 1) parts[0] to "true" else parts[0] to parts[1]
+        }
+        .multiple()
+        .toMap()
+    }
   }
 
   private val defaults = CliBaseOptions()
@@ -116,7 +126,7 @@ class BaseOptions : OptionGroup() {
         metavar = "<name=value>",
         help = "External property to set (repeatable)."
       )
-      .associate()
+      .associateProps()
 
   val noCache: Boolean by
     option(names = arrayOf("--no-cache"), help = "Disable caching of packages")
@@ -215,7 +225,7 @@ class BaseOptions : OptionGroup() {
       allowedModules = allowedModules.ifEmpty { null },
       allowedResources = allowedResources.ifEmpty { null },
       environmentVariables = envVars.ifEmpty { null },
-      externalProperties = properties.mapValues { it.value.ifBlank { "true" } }.ifEmpty { null },
+      externalProperties = properties.ifEmpty { null },
       modulePath = modulePath.ifEmpty { null },
       workingDir = workingDir,
       settings = settings,
diff --git a/pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/BaseCommandTest.kt b/pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/BaseCommandTest.kt
index e9e1b42c..490ef855 100644
--- a/pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/BaseCommandTest.kt
+++ b/pkl-commons-cli/src/test/kotlin/org/pkl/commons/cli/BaseCommandTest.kt
@@ -45,10 +45,10 @@ class BaseCommandTest {
 
   @Test
   fun `external properties without value default to 'true'`() {
-    cmd.parse(arrayOf("-p", "flag1", "-p", "flag2", "-p", "FOO=bar"))
+    cmd.parse(arrayOf("-p", "flag1", "-p", "flag2=", "-p", "FOO=bar"))
     val props = cmd.baseOptions.baseOptions(emptyList()).externalProperties
 
-    assertThat(props).isEqualTo(mapOf("flag1" to "true", "flag2" to "true", "FOO" to "bar"))
+    assertThat(props).isEqualTo(mapOf("flag1" to "true", "flag2" to "", "FOO" to "bar"))
   }
 
   @Test