From a3fb678e17ec81a68d4f5648feacc5c488fbd19f Mon Sep 17 00:00:00 2001
From: oSumAtrIX <johan.melkonyan1@web.de>
Date: Mon, 12 Aug 2024 21:26:50 +0200
Subject: [PATCH] feat: Merge selection and option commands to arg groups

---
 docs/1_usage.md                               |  10 +-
 .../app/revanced/cli/command/CommandUtils.kt  | 105 ++++++
 .../cli/command/ListCompatibleVersions.kt     |   2 +-
 .../cli/command/ListPatchesCommand.kt         |   2 +-
 .../app/revanced/cli/command/PatchCommand.kt  | 339 +++++++-----------
 .../cli/command/utility/InstallCommand.kt     |   2 +-
 .../cli/command/utility/UninstallCommand.kt   |   7 +-
 7 files changed, 255 insertions(+), 212 deletions(-)
 create mode 100644 src/main/kotlin/app/revanced/cli/command/CommandUtils.kt

diff --git a/docs/1_usage.md b/docs/1_usage.md
index 27296610..7e515426 100644
--- a/docs/1_usage.md
+++ b/docs/1_usage.md
@@ -79,25 +79,25 @@ java -jar revanced-cli.jar patch -b revanced-patches.rvp --exclusive -i "Patch n
 > adb install input.apk
 > ```
 
-Patches can have options you can set using the option `--set-options`.
+Patches can have options you can set using the option `-O` alongside the option to include the patch by name or index.
 To know the options of a patch, use the option `--with-options` when listing patches:
 
 ```bash
 java -jar revanced-cli.jar list-patches --with-options revanced-patches.rvp
 ```
 
-Each patch can have multiple options. You can set them using the option `--set-options`.
+Each patch can have multiple options. You can set them using the option `-O`.
 For example, to set the options for the patch with the name `Patch name`
 with the key `key1` and `key2` to `value1` and `value2` respectively, use the following command:
 
 ```bash
-java -jar revanced-cli.jar patch -b revanced-patches.rvp --set-options "Patch name" -Okey1=value1 -Okey2=value2 input.apk
+java -jar revanced-cli.jar patch -b revanced-patches.rvp -i "Patch name" -Okey1=value1 -Okey2=value2 input.apk
 ```
 
 If you want to set a value to `null`, you can omit the value:
 
 ```bash
-java -jar revanced-cli.jar patch -b revanced-patches.rvp --set-options "Patch name" -Okey1 input.apk
+java -jar revanced-cli.jar patch -b revanced-patches.rvp -i "Patch name" -Okey1 input.apk
 ```
 
 > [!WARNING]
@@ -131,7 +131,7 @@ java -jar revanced-cli.jar patch -b revanced-patches.rvp --set-options "Patch na
 > Example command with an escaped integer as a string:
 > 
 > ```bash
-> java -jar revanced-cli.jar -b revanced-patches.rvp --set-options "Patch name" -OstringKey=\'1\' input.apk
+> java -jar revanced-cli.jar -b revanced-patches.rvp -i "Patch name" -OstringKey=\'1\' input.apk
 > ```
 ## 📦 Install an app manually 
 
diff --git a/src/main/kotlin/app/revanced/cli/command/CommandUtils.kt b/src/main/kotlin/app/revanced/cli/command/CommandUtils.kt
new file mode 100644
index 00000000..afb365b6
--- /dev/null
+++ b/src/main/kotlin/app/revanced/cli/command/CommandUtils.kt
@@ -0,0 +1,105 @@
+package app.revanced.cli.command
+
+import picocli.CommandLine
+
+class OptionKeyConverter : CommandLine.ITypeConverter<String> {
+    override fun convert(value: String): String = value
+}
+
+class OptionValueConverter : CommandLine.ITypeConverter<Any?> {
+    override fun convert(value: String?): Any? {
+        value ?: return null
+
+        return when {
+            value.startsWith("[") && value.endsWith("]") -> {
+                val innerValue = value.substring(1, value.length - 1)
+
+                buildList {
+                    var nestLevel = 0
+                    var insideQuote = false
+                    var escaped = false
+
+                    val item = buildString {
+                        for (char in innerValue) {
+                            when (char) {
+                                '\\' -> {
+                                    if (escaped || nestLevel != 0) {
+                                        append(char)
+                                    }
+
+                                    escaped = !escaped
+                                }
+
+                                '"', '\'' -> {
+                                    if (!escaped) {
+                                        insideQuote = !insideQuote
+                                    } else {
+                                        escaped = false
+                                    }
+
+                                    append(char)
+                                }
+
+                                '[' -> {
+                                    if (!insideQuote) {
+                                        nestLevel++
+                                    }
+
+                                    append(char)
+                                }
+
+                                ']' -> {
+                                    if (!insideQuote) {
+                                        nestLevel--
+
+                                        if (nestLevel == -1) {
+                                            return value
+                                        }
+                                    }
+
+                                    append(char)
+                                }
+
+                                ',' -> if (nestLevel == 0) {
+                                    if (insideQuote) {
+                                        append(char)
+                                    } else {
+                                        add(convert(toString()))
+                                        setLength(0)
+                                    }
+                                } else {
+                                    append(char)
+                                }
+
+                                else -> append(char)
+                            }
+                        }
+                    }
+
+                    if (item.isNotEmpty()) {
+                        add(convert(item))
+                    }
+                }
+            }
+
+            value.startsWith("\"") && value.endsWith("\"") -> value.substring(1, value.length - 1)
+            value.startsWith("'") && value.endsWith("'") -> value.substring(1, value.length - 1)
+            value.endsWith("f") -> value.dropLast(1).toFloat()
+            value.endsWith("L") -> value.dropLast(1).toLong()
+            value.equals("true", ignoreCase = true) -> true
+            value.equals("false", ignoreCase = true) -> false
+            value.toIntOrNull() != null -> value.toInt()
+            value.toLongOrNull() != null -> value.toLong()
+            value.toDoubleOrNull() != null -> value.toDouble()
+            value.toFloatOrNull() != null -> value.toFloat()
+            value == "null" -> null
+            value == "int[]" -> emptyList<Int>()
+            value == "long[]" -> emptyList<Long>()
+            value == "double[]" -> emptyList<Double>()
+            value == "float[]" -> emptyList<Float>()
+            value == "boolean[]" -> emptyList<Boolean>()
+            value == "string[]" -> emptyList<String>()
+            else -> value
+        }
+    }
+}
diff --git a/src/main/kotlin/app/revanced/cli/command/ListCompatibleVersions.kt b/src/main/kotlin/app/revanced/cli/command/ListCompatibleVersions.kt
index 9816fcf2..ba993c7c 100644
--- a/src/main/kotlin/app/revanced/cli/command/ListCompatibleVersions.kt
+++ b/src/main/kotlin/app/revanced/cli/command/ListCompatibleVersions.kt
@@ -16,7 +16,7 @@ import java.util.logging.Logger
     ],
 )
 internal class ListCompatibleVersions : Runnable {
-    private val logger = Logger.getLogger(ListCompatibleVersions::class.java.name)
+    private val logger = Logger.getLogger(this::class.java.name)
 
     @CommandLine.Parameters(
         description = ["Paths to patch bundles."],
diff --git a/src/main/kotlin/app/revanced/cli/command/ListPatchesCommand.kt b/src/main/kotlin/app/revanced/cli/command/ListPatchesCommand.kt
index 079d8c0d..fcdb3a57 100644
--- a/src/main/kotlin/app/revanced/cli/command/ListPatchesCommand.kt
+++ b/src/main/kotlin/app/revanced/cli/command/ListPatchesCommand.kt
@@ -14,7 +14,7 @@ import app.revanced.patcher.patch.Option as PatchOption
     description = ["List patches from supplied patch bundles."],
 )
 internal object ListPatchesCommand : Runnable {
-    private val logger = Logger.getLogger(ListPatchesCommand::class.java.name)
+    private val logger = Logger.getLogger(this::class.java.name)
 
     @Parameters(
         description = ["Paths to patch bundles."],
diff --git a/src/main/kotlin/app/revanced/cli/command/PatchCommand.kt b/src/main/kotlin/app/revanced/cli/command/PatchCommand.kt
index c31afd18..96d8d7c4 100644
--- a/src/main/kotlin/app/revanced/cli/command/PatchCommand.kt
+++ b/src/main/kotlin/app/revanced/cli/command/PatchCommand.kt
@@ -24,57 +24,70 @@ import java.util.logging.Logger
     description = ["Patch an APK file."],
 )
 internal object PatchCommand : Runnable {
-    private val logger = Logger.getLogger(PatchCommand::class.java.name)
+    private val logger = Logger.getLogger(this::class.java.name)
 
     @Spec
-    lateinit var spec: CommandSpec
+    private lateinit var spec: CommandSpec
 
-    private lateinit var apk: File
+    @ArgGroup(multiplicity = "0..*")
+    private lateinit var selection: Set<Selection>
 
-    private var patchBundles = emptySet<File>()
+    internal class Selection {
+        @ArgGroup(exclusive = false, multiplicity = "1")
+        internal var include: IncludeSelection? = null
 
-    @CommandLine.Option(
-        names = ["-i", "--include"],
-        description = ["List of patches to include by name."],
-    )
-    private var includedPatches = hashSetOf<String>()
+        internal class IncludeSelection {
+            @ArgGroup(multiplicity = "1")
+            internal lateinit var selector: IncludeSelector
 
-    @CommandLine.Option(
-        names = ["--ii"],
-        description = ["List of patches to include by their index in the combined list of all supplied patch bundles."],
-    )
-    private var includedPatchesByIndex = setOf<Int>()
+            internal class IncludeSelector {
+                @CommandLine.Option(
+                    names = ["-i", "--include"],
+                    description = ["The name of the patch."],
+                    required = true,
+                )
+                internal var name: String? = null
 
-    @CommandLine.Option(
-        names = ["-e", "--exclude"],
-        description = ["List of patches to exclude."],
-    )
-    private var excludedPatches = hashSetOf<String>()
+                @CommandLine.Option(
+                    names = ["--ii"],
+                    description = ["The index of the patch in the combined list of all supplied patch bundles."],
+                    required = true,
+                )
+                internal var index: Int? = null
+            }
 
-    @CommandLine.Option(
-        names = ["--ei"],
-        description = ["List of patches to exclude by their index in the combined list of all supplied patch bundles."],
-    )
-    private var excludedPatchesByIndex = setOf<Int>()
-
-    @ArgGroup(exclusive = false, multiplicity = "0..*", heading = "Options:%n")
-    private lateinit var options: Set<Option>
-
-    private class Option {
-        @CommandLine.Option(
-            names = ["--set-options"],
-            description = ["The name of the patch to set the options of."],
-            required = true,
-        )
-        lateinit var patch: String
-
-        @CommandLine.Option(
-            names = ["-O", "--options"],
-            description = ["The option values keyed by the option keys."],
-            mapFallbackValue = CommandLine.Option.NULL_VALUE,
-            converter = [OptionKeyConverter::class, OptionValueConverter::class],
-        )
-        lateinit var options: Map<String, Any?>
+            @CommandLine.Option(
+                names = ["-O", "--options"],
+                description = ["The option values keyed by the option keys."],
+                mapFallbackValue = CommandLine.Option.NULL_VALUE,
+                converter = [OptionKeyConverter::class, OptionValueConverter::class],
+            )
+            internal var options = mutableMapOf<String, Any?>()
+        }
+
+        @ArgGroup(exclusive = false, multiplicity = "1")
+        internal var exclude: ExcludeSelection? = null
+
+        internal class ExcludeSelection {
+            @ArgGroup(multiplicity = "1")
+            internal lateinit var selector: ExcludeSelector
+
+            internal class ExcludeSelector {
+                @CommandLine.Option(
+                    names = ["-e", "--exclude"],
+                    description = ["The name of the patch."],
+                    required = true,
+                )
+                internal var name: String? = null
+
+                @CommandLine.Option(
+                    names = ["--ie"],
+                    description = ["The index of the patch in the combined list of all supplied patch bundles."],
+                    required = true,
+                )
+                internal var index: Int? = null
+            }
+        }
     }
 
     @CommandLine.Option(
@@ -155,7 +168,7 @@ internal object PatchCommand : Runnable {
 
     @CommandLine.Option(
         names = ["-t", "--temporary-files-path"],
-        description = ["Path to temporary files directory."],
+        description = ["Path to store temporary files."],
     )
     private var temporaryFilesPath: File? = null
 
@@ -183,6 +196,8 @@ internal object PatchCommand : Runnable {
         this.apk = apk
     }
 
+    private lateinit var apk: File
+
     @CommandLine.Option(
         names = ["-b", "--patch-bundle"],
         description = ["One or more bundles of patches."],
@@ -196,6 +211,8 @@ internal object PatchCommand : Runnable {
         this.patchBundles = patchBundles
     }
 
+    private var patchBundles = emptySet<File>()
+
     @CommandLine.Option(
         names = ["--custom-aapt2-binary"],
         description = ["Path to a custom AAPT binary to compile resources with."],
@@ -249,10 +266,20 @@ internal object PatchCommand : Runnable {
                 true,
             ),
         ).use { patcher ->
-            val filteredPatches = patcher.filterPatchSelection(patches)
+            val packageName = patcher.context.packageMetadata.packageName
+            val packageVersion = patcher.context.packageMetadata.packageVersion
+
+            val filteredPatches = patches.filterPatchSelection(packageName, packageVersion)
 
             logger.info("Setting patch options")
-            filteredPatches.setOptions(options.associate { it.patch to it.options })
+
+            val patchesList = patches.toList()
+            selection.filter { it.include != null }.associate {
+                val includeSelection = it.include!!
+
+                (includeSelection.selector.name ?: patchesList[includeSelection.selector.index!!].name!!) to
+                    includeSelection.options
+            }.let(filteredPatches::setOptions)
 
             patcher += filteredPatches
 
@@ -273,7 +300,7 @@ internal object PatchCommand : Runnable {
             patcher.context.packageMetadata.packageName to patcher.get()
         }
 
-        // region Save
+        // region Save.
 
         apk.copyTo(temporaryFilesPath.resolve(apk.name), overwrite = true).apply {
             patcherResult.applyTo(this)
@@ -299,9 +326,9 @@ internal object PatchCommand : Runnable {
 
         // endregion
 
-        // region Install
+        // region Install.
 
-        deviceSerial?.let { it ->
+        deviceSerial?.let {
             val deviceSerial = it.ifEmpty { null }
 
             runBlocking {
@@ -328,64 +355,72 @@ internal object PatchCommand : Runnable {
     }
 
     /**
-     * Filter the patches to be added to the patcher. The filter is based on the following:
+     * Filter the patches based on the selection.
      *
-     * @param patches The patches to filter.
+     * @param packageName The package name of the APK file to be patched.
+     * @param packageVersion The version of the APK file to be patched.
      * @return The filtered patches.
      */
-    private fun Patcher.filterPatchSelection(patches: Set<Patch<*>>): Set<Patch<*>> =
-        buildSet {
-            val packageName = context.packageMetadata.packageName
-            val packageVersion = context.packageMetadata.packageVersion
-
-            patches.withIndex().forEach patch@{ (i, patch) ->
-                val patchName = patch.name!!
-
-                val explicitlyExcluded = excludedPatches.contains(patchName) || excludedPatchesByIndex.contains(i)
-                if (explicitlyExcluded) return@patch logger.info("\"$patchName\" excluded manually")
-
-                // Make sure the patch is compatible with the supplied APK files package name and version.
-                patch.compatiblePackages?.let { packages ->
-                    packages.singleOrNull { (name, _) -> name == packageName }?.let { (_, versions) ->
-                        if (versions?.isEmpty() == true) {
-                            return@patch logger.warning("\"$patchName\" incompatible with \"$packageName\"")
-                        }
-
-                        val matchesVersion = force ||
-                            versions?.let { it.any { version -> version == packageVersion } }
-                                ?: true
-
-                        if (!matchesVersion) {
-                            return@patch logger.warning(
-                                "\"$patchName\" incompatible with $packageName $packageVersion " +
-                                    "but compatible with " +
-                                    packages.joinToString("; ") { (packageName, versions) ->
-                                        packageName + " " + versions!!.joinToString(", ")
-                                    },
-                            )
-                        }
-                    } ?: return@patch logger.fine(
-                        "\"$patchName\" incompatible with $packageName. " +
-                            "It is only compatible with " +
-                            packages.joinToString(", ") { (name, _) -> name },
-                    )
-
-                    return@let
-                } ?: logger.fine("\"$patchName\" has no package constraints")
-
-                // If the patch is implicitly used, it will be only included if [exclusive] is false.
-                val implicitlyIncluded = !exclusive && patch.use
-                // If the patch is explicitly used, it will be included even if [exclusive] is false.
-                val explicitlyIncluded = includedPatches.contains(patchName) || includedPatchesByIndex.contains(i)
-
-                val included = implicitlyIncluded || explicitlyIncluded
-                if (!included) return@patch logger.info("\"$patchName\" excluded") // Case 1.
-
-                add(patch)
-
-                logger.fine("\"$patchName\" added")
+    private fun Set<Patch<*>>.filterPatchSelection(
+        packageName: String,
+        packageVersion: String,
+    ): Set<Patch<*>> = buildSet {
+        val includedPatchesByName =
+            selection.asSequence().mapNotNull { it.include?.selector?.name }.toSet()
+        val includedPatchesByIndex =
+            selection.asSequence().mapNotNull { it.include?.selector?.index }.toSet()
+
+        val excludedPatches =
+            selection.asSequence().mapNotNull { it.exclude?.selector?.name }.toSet()
+        val excludedPatchesByIndex =
+            selection.asSequence().mapNotNull { it.exclude?.selector?.index }.toSet()
+
+        this@filterPatchSelection.withIndex().forEach patchLoop@{ (i, patch) ->
+            val patchName = patch.name!!
+
+            val isManuallyExcluded = patchName in excludedPatches || i in excludedPatchesByIndex
+            if (isManuallyExcluded) return@patchLoop logger.info("\"$patchName\" excluded manually")
+
+            // Make sure the patch is compatible with the supplied APK files package name and version.
+            patch.compatiblePackages?.let { packages ->
+                packages.singleOrNull { (name, _) -> name == packageName }?.let { (_, versions) ->
+                    if (versions?.isEmpty() == true) {
+                        return@patchLoop logger.warning("\"$patchName\" incompatible with \"$packageName\"")
+                    }
+
+                    val matchesVersion =
+                        force || versions?.let { it.any { version -> version == packageVersion } } ?: true
+
+                    if (!matchesVersion) {
+                        return@patchLoop logger.warning(
+                            "\"$patchName\" incompatible with $packageName $packageVersion " +
+                                "but compatible with " +
+                                packages.joinToString("; ") { (packageName, versions) ->
+                                    packageName + " " + versions!!.joinToString(", ")
+                                },
+                        )
+                    }
+                } ?: return@patchLoop logger.fine(
+                    "\"$patchName\" incompatible with $packageName. " +
+                        "It is only compatible with " +
+                        packages.joinToString(", ") { (name, _) -> name },
+                )
+
+                return@let
+            } ?: logger.fine("\"$patchName\" has no package constraints")
+
+            val isIncluded = !exclusive && patch.use
+            val isManuallyIncluded = patchName in includedPatchesByName || i in includedPatchesByIndex
+
+            if (!(isIncluded || isManuallyIncluded)) {
+                return@patchLoop logger.info("\"$patchName\" excluded")
             }
+
+            add(patch)
+
+            logger.fine("\"$patchName\" added")
         }
+    }
 
     private fun purge(resourceCachePath: File) {
         val result =
@@ -397,103 +432,3 @@ internal object PatchCommand : Runnable {
         logger.info(result)
     }
 }
-
-class OptionKeyConverter : CommandLine.ITypeConverter<String> {
-    override fun convert(value: String): String = value
-}
-
-class OptionValueConverter : CommandLine.ITypeConverter<Any?> {
-    override fun convert(value: String?): Any? {
-        value ?: return null
-
-        return when {
-            value.startsWith("[") && value.endsWith("]") -> {
-                val innerValue = value.substring(1, value.length - 1)
-
-                buildList {
-                    var nestLevel = 0
-                    var insideQuote = false
-                    var escaped = false
-
-                    val item = buildString {
-                        for (char in innerValue) {
-                            when (char) {
-                                '\\' -> {
-                                    if (escaped || nestLevel != 0) {
-                                        append(char)
-                                    }
-
-                                    escaped = !escaped
-                                }
-                                '"', '\'' -> {
-                                    if (!escaped) {
-                                        insideQuote = !insideQuote
-                                    } else {
-                                        escaped = false
-                                    }
-
-                                    append(char)
-                                }
-
-                                '[' -> {
-                                    if (!insideQuote) {
-                                        nestLevel++
-                                    }
-
-                                    append(char)
-                                }
-
-                                ']' -> {
-                                    if (!insideQuote) {
-                                        nestLevel--
-
-                                        if (nestLevel == -1) {
-                                            return value
-                                        }
-                                    }
-
-                                    append(char)
-                                }
-
-                                ',' -> if (nestLevel == 0) {
-                                    if (insideQuote) {
-                                        append(char)
-                                    } else {
-                                        add(convert(toString()))
-                                        setLength(0)
-                                    }
-                                } else {
-                                    append(char)
-                                }
-
-                                else -> append(char)
-                            }
-                        }
-                    }
-
-                    if (item.isNotEmpty()) {
-                        add(convert(item))
-                    }
-                }
-            }
-            value.startsWith("\"") && value.endsWith("\"") -> value.substring(1, value.length - 1)
-            value.startsWith("'") && value.endsWith("'") -> value.substring(1, value.length - 1)
-            value.endsWith("f") -> value.dropLast(1).toFloat()
-            value.endsWith("L") -> value.dropLast(1).toLong()
-            value.equals("true", ignoreCase = true) -> true
-            value.equals("false", ignoreCase = true) -> false
-            value.toIntOrNull() != null -> value.toInt()
-            value.toLongOrNull() != null -> value.toLong()
-            value.toDoubleOrNull() != null -> value.toDouble()
-            value.toFloatOrNull() != null -> value.toFloat()
-            value == "null" -> null
-            value == "int[]" -> emptyList<Int>()
-            value == "long[]" -> emptyList<Long>()
-            value == "double[]" -> emptyList<Double>()
-            value == "float[]" -> emptyList<Float>()
-            value == "boolean[]" -> emptyList<Boolean>()
-            value == "string[]" -> emptyList<String>()
-            else -> value
-        }
-    }
-}
diff --git a/src/main/kotlin/app/revanced/cli/command/utility/InstallCommand.kt b/src/main/kotlin/app/revanced/cli/command/utility/InstallCommand.kt
index 573205e0..6fbb5467 100644
--- a/src/main/kotlin/app/revanced/cli/command/utility/InstallCommand.kt
+++ b/src/main/kotlin/app/revanced/cli/command/utility/InstallCommand.kt
@@ -13,7 +13,7 @@ import java.util.logging.Logger
     description = ["Install an APK file to devices with the supplied ADB device serials"],
 )
 internal object InstallCommand : Runnable {
-    private val logger = Logger.getLogger(InstallCommand::class.java.name)
+    private val logger = Logger.getLogger(this::class.java.name)
 
     @Parameters(
         description = ["ADB device serials. If not supplied, the first connected device will be used."],
diff --git a/src/main/kotlin/app/revanced/cli/command/utility/UninstallCommand.kt b/src/main/kotlin/app/revanced/cli/command/utility/UninstallCommand.kt
index e446e001..6f825c65 100644
--- a/src/main/kotlin/app/revanced/cli/command/utility/UninstallCommand.kt
+++ b/src/main/kotlin/app/revanced/cli/command/utility/UninstallCommand.kt
@@ -1,6 +1,9 @@
 package app.revanced.cli.command.utility
 
-import app.revanced.library.installation.installer.*
+import app.revanced.library.installation.installer.AdbInstaller
+import app.revanced.library.installation.installer.AdbInstallerResult
+import app.revanced.library.installation.installer.AdbRootInstaller
+import app.revanced.library.installation.installer.RootInstallerResult
 import kotlinx.coroutines.async
 import kotlinx.coroutines.awaitAll
 import kotlinx.coroutines.runBlocking
@@ -13,7 +16,7 @@ import java.util.logging.Logger
     description = ["Uninstall a patched app from the devices with the supplied ADB device serials"],
 )
 internal object UninstallCommand : Runnable {
-    private val logger = Logger.getLogger(UninstallCommand::class.java.name)
+    private val logger = Logger.getLogger(this::class.java.name)
 
     @Parameters(
         description = ["ADB device serials. If not supplied, the first connected device will be used."],