From 7a5ecb8e75f5a84363948999687295644952c231 Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Fri, 11 Oct 2024 20:36:41 -0400 Subject: [PATCH 1/8] Use the appropriate extension for in-place conversion --- vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt index 764ed68a..78ffa043 100644 --- a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt +++ b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt @@ -16,6 +16,7 @@ import org.w3c.dom.Document import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.File +import java.nio.file.Paths import javax.xml.parsers.DocumentBuilderFactory import kotlin.math.absoluteValue import kotlin.math.roundToInt @@ -59,7 +60,7 @@ class Vgo( } } else { inputs.zip(inputs) { a, b -> - Pair(File(a), File(b)) + } }.toMap() @@ -70,6 +71,24 @@ class Vgo( return handleFiles(inputOutputMap, writerOptions) } + private fun pairOutputs(): Map { + val inputFiles = options.input.map(::File) + return if (options.output.isNotEmpty()) { + inputFiles.zip(options.output.map(::File)).toMap() + } else { + inputFiles.mapIndexed { index, input -> + if (!input.isDirectory) { + val outputExtension = when (options.format) { + "svg" -> "svg" + "vd" -> "xml" + else -> input.extension + } + val outputFilePath = options.output.dropLastWhile { it != '.' } + outputExtension + } + }.toMap() + } + } + private fun handleFiles( inputOutputMap: Map, writerOptions: Set, From a5d1a080d59be074335c69d6304fc4e3b33f0318 Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Fri, 11 Oct 2024 21:13:53 -0400 Subject: [PATCH 2/8] Assign a file extension at the last possible second --- vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt | 144 ++++++++++---------- 1 file changed, 70 insertions(+), 74 deletions(-) diff --git a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt index 78ffa043..2e4bda8a 100644 --- a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt +++ b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt @@ -16,8 +16,13 @@ import org.w3c.dom.Document import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream import java.io.File +import java.nio.file.Path import java.nio.file.Paths import javax.xml.parsers.DocumentBuilderFactory +import kotlin.io.path.exists +import kotlin.io.path.isDirectory +import kotlin.io.path.isRegularFile +import kotlin.io.path.isSameFileAs import kotlin.math.absoluteValue import kotlin.math.roundToInt @@ -53,17 +58,7 @@ class Vgo( inputs = standardInPaths } - val inputOutputMap = - if (options.output.isNotEmpty()) { - inputs.zip(options.output) { a, b -> - Pair(File(a), File(b)) - } - } else { - inputs.zip(inputs) { a, b -> - - } - }.toMap() - + val inputOutputMap = pairOutputs() val files = inputOutputMap.count { (input, _) -> input.isFile } val containsDirectory = inputOutputMap.any { (input, _) -> input.isDirectory } printFileNames = options.printStats && (files > 1 || containsDirectory) @@ -71,67 +66,21 @@ class Vgo( return handleFiles(inputOutputMap, writerOptions) } - private fun pairOutputs(): Map { - val inputFiles = options.input.map(::File) + private fun pairOutputs(): Map { return if (options.output.isNotEmpty()) { - inputFiles.zip(options.output.map(::File)).toMap() + options.input.zip(options.output) { a, b -> + Pair(File(a), Paths.get(b)) + } } else { - inputFiles.mapIndexed { index, input -> - if (!input.isDirectory) { - val outputExtension = when (options.format) { - "svg" -> "svg" - "vd" -> "xml" - else -> input.extension - } - val outputFilePath = options.output.dropLastWhile { it != '.' } + outputExtension - } - }.toMap() - } - } - - private fun handleFiles( - inputOutputMap: Map, - writerOptions: Set, - ): Int { - for (entry in inputOutputMap) { - val (input, output) = entry - - when { - entry.isFilePair -> handleFile(input, output, writerOptions) - entry.isDirectoryPair -> handleDirectory(input, output, writerOptions) - !entry.inputExists -> { - System.err.println("${input.path} does not exist.") - return 65 - } - else -> { - System.err.println( - """ - A given input and output pair (grouped positionally) - must be either files or directories. - Input is a ${if (input.isFile) "file" else "directory"} - path: ${input.absolutePath} - exists: ${input.exists()} - isWritable: ${input.canWrite()} - Output is a ${if (output.isFile) "file" else "directory"} - path: ${output.absolutePath} - exists: ${input.exists()} - isWritable: ${input.canWrite()} - - Storage: ${output.usableSpace} / ${output.totalSpace} is usable. - """.trimIndent(), - ) - - return 65 - } + options.input.zip(options.input) { a, b -> + Pair(File(a), Paths.get(b)) } - } - - return 0 + }.toMap() } private fun handleFile( input: File, - output: File, + outputPath: Path, options: Set, ) { input.inputStream().use { inputStream -> @@ -186,7 +135,7 @@ class Vgo( com.jzbrooks.vgo.vd .parse(rootNodes.first()) } - else -> if (input == output) return else null + else -> if (outputPath.isSameFileAs(input.toPath())) return else null } if (graphic is VectorDrawable && this.options.format == "svg") { @@ -204,6 +153,12 @@ class Vgo( optimizationRegistry?.apply(graphic) } + val output = when(this.options.format) { + "vd" -> outputPath.resolveSibling("${outputPath.fileName}.xml") + "svg" -> outputPath.resolveSibling("${outputPath.fileName}.svg") + else -> outputPath + }.toFile() + if (output.parentFile?.exists() == false) output.parentFile.mkdirs() if (!output.exists()) output.createNewFile() @@ -239,16 +194,57 @@ class Vgo( } } + private fun handleFiles( + inputOutputMap: Map, + writerOptions: Set, + ): Int { + for (entry in inputOutputMap) { + val (input, output) = entry + + when { + entry.isFilePair -> handleFile(input, output, writerOptions) + entry.isDirectoryPair -> handleDirectory(input, output, writerOptions) + !entry.inputExists -> { + System.err.println("${input.path} does not exist.") + return 65 + } + else -> { + val output = output.toFile() + System.err.println( + """ + A given input and output pair (grouped positionally) + must be either files or directories. + Input is a ${if (input.isFile) "file" else "directory"} + path: ${input.absolutePath} + exists: ${input.exists()} + isWritable: ${input.canWrite()} + Output is a ${if (output.isFile) "file" else "directory"} + path: ${output.absolutePath} + exists: ${input.exists()} + isWritable: ${input.canWrite()} + + Storage: ${output.usableSpace} / ${output.totalSpace} is usable. + """.trimIndent(), + ) + + return 65 + } + } + } + + return 0 + } + private fun handleDirectory( input: File, - output: File, + output: Path, options: Set, ) { assert(input.isDirectory) - assert(output.isDirectory || !output.exists()) + assert(output.isDirectory() || !output.exists()) for (file in input.walkTopDown().filter { file -> !file.isHidden && !file.isDirectory }) { - handleFile(file, File(output, file.name), options) + handleFile(file, output.resolve(file.name), options) } if (this.options.printStats) { @@ -282,14 +278,14 @@ class Vgo( else -> "$bytes B" } - private val Map.Entry.inputExists + private val Map.Entry.inputExists get() = key.exists() - private val Map.Entry.isFilePair - get() = key.isFile && (value.isFile || !value.exists()) + private val Map.Entry.isFilePair + get() = key.isFile && (value.isRegularFile() || !value.exists()) - private val Map.Entry.isDirectoryPair - get() = key.isDirectory && (value.isDirectory || !value.exists()) + private val Map.Entry.isDirectoryPair + get() = key.isDirectory && (value.isDirectory() || !value.exists()) data class Options( val printVersion: Boolean = false, From abbb4d58946348423e2a9374c505493960675bb3 Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Fri, 11 Oct 2024 21:34:29 -0400 Subject: [PATCH 3/8] Format code --- vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt index 2e4bda8a..a0a569aa 100644 --- a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt +++ b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt @@ -66,8 +66,8 @@ class Vgo( return handleFiles(inputOutputMap, writerOptions) } - private fun pairOutputs(): Map { - return if (options.output.isNotEmpty()) { + private fun pairOutputs(): Map = + if (options.output.isNotEmpty()) { options.input.zip(options.output) { a, b -> Pair(File(a), Paths.get(b)) } @@ -76,7 +76,6 @@ class Vgo( Pair(File(a), Paths.get(b)) } }.toMap() - } private fun handleFile( input: File, @@ -153,11 +152,12 @@ class Vgo( optimizationRegistry?.apply(graphic) } - val output = when(this.options.format) { - "vd" -> outputPath.resolveSibling("${outputPath.fileName}.xml") - "svg" -> outputPath.resolveSibling("${outputPath.fileName}.svg") - else -> outputPath - }.toFile() + val output = + when (this.options.format) { + "vd" -> outputPath.resolveSibling("${outputPath.fileName}.xml") + "svg" -> outputPath.resolveSibling("${outputPath.fileName}.svg") + else -> outputPath + }.toFile() if (output.parentFile?.exists() == false) output.parentFile.mkdirs() if (!output.exists()) output.createNewFile() From 6cc8d7f6e4eb797baa4060462f96598b45f8998d Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Fri, 11 Oct 2024 23:11:40 -0400 Subject: [PATCH 4/8] Avoid duplicate extensions --- vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt index a0a569aa..63b27f18 100644 --- a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt +++ b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt @@ -19,10 +19,13 @@ import java.io.File import java.nio.file.Path import java.nio.file.Paths import javax.xml.parsers.DocumentBuilderFactory +import kotlin.io.path.createDirectories +import kotlin.io.path.createFile import kotlin.io.path.exists import kotlin.io.path.isDirectory import kotlin.io.path.isRegularFile import kotlin.io.path.isSameFileAs +import kotlin.io.path.nameWithoutExtension import kotlin.math.absoluteValue import kotlin.math.roundToInt @@ -154,8 +157,8 @@ class Vgo( val output = when (this.options.format) { - "vd" -> outputPath.resolveSibling("${outputPath.fileName}.xml") - "svg" -> outputPath.resolveSibling("${outputPath.fileName}.svg") + "vd" -> outputPath.resolveSibling("${outputPath.nameWithoutExtension}.xml") + "svg" -> outputPath.resolveSibling("${outputPath.nameWithoutExtension}.svg") else -> outputPath }.toFile() From d59a74d84788ae99555efead0f49de54b2532eb0 Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Fri, 11 Oct 2024 23:27:49 -0400 Subject: [PATCH 5/8] Remove imports --- vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt index 63b27f18..f2a0d9e4 100644 --- a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt +++ b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt @@ -19,8 +19,6 @@ import java.io.File import java.nio.file.Path import java.nio.file.Paths import javax.xml.parsers.DocumentBuilderFactory -import kotlin.io.path.createDirectories -import kotlin.io.path.createFile import kotlin.io.path.exists import kotlin.io.path.isDirectory import kotlin.io.path.isRegularFile From 2dd2e6a96af00dbf7045e6121badc98bc63d9c01 Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Sat, 12 Oct 2024 09:50:21 -0400 Subject: [PATCH 6/8] Add a test --- .../jzbrooks/vgo/InPlaceModificationTest.kt | 54 +++++++++++-------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/vgo/src/test/kotlin/com/jzbrooks/vgo/InPlaceModificationTest.kt b/vgo/src/test/kotlin/com/jzbrooks/vgo/InPlaceModificationTest.kt index a502a06c..984d566e 100644 --- a/vgo/src/test/kotlin/com/jzbrooks/vgo/InPlaceModificationTest.kt +++ b/vgo/src/test/kotlin/com/jzbrooks/vgo/InPlaceModificationTest.kt @@ -3,10 +3,12 @@ package com.jzbrooks.vgo import assertk.assertThat import assertk.assertions.contains import assertk.assertions.doesNotContain +import assertk.assertions.exists import assertk.assertions.isEqualTo import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInfo import java.io.ByteArrayOutputStream import java.io.File import java.io.PrintStream @@ -16,17 +18,10 @@ class InPlaceModificationTest { private lateinit var systemOutput: ByteArrayOutputStream @BeforeEach - fun copyToSide() { + fun copyToSide(info: TestInfo) { val originalFolder = File("src/test/resources/in-place-modify/") - val reservedFolder = File("build/test-results/inPlaceModification/reserved/") - originalFolder.copyRecursively(reservedFolder, true) - } - - @AfterEach - fun resetFiles() { - val originalFolder = File("src/test/resources/in-place-modify/") - val reservedFolder = File("build/test-results/inPlaceModification/reserved/") - reservedFolder.copyRecursively(originalFolder, true) + val workingFolder = File("build/test-results/inPlaceModification/${info.displayName}/") + originalFolder.copyRecursively(workingFolder, true) } @BeforeEach @@ -41,10 +36,10 @@ class InPlaceModificationTest { } @Test - fun `in-place optimization completes successfully`() { + fun `in-place optimization completes successfully`(info: TestInfo) { val options = Vgo.Options( - input = listOf("src/test/resources/in-place-modify"), + input = listOf("build/test-results/inPlaceModification/${info.displayName}/"), ) val exitCode = Vgo(options).run() @@ -52,49 +47,49 @@ class InPlaceModificationTest { } @Test - fun `individual file statistics are reported with a directory input`() { + fun `individual file statistics are reported with a directory input`(info: TestInfo) { val options = Vgo.Options( printStats = true, - input = listOf("src/test/resources/in-place-modify"), + input = listOf("build/test-results/inPlaceModification/${info.displayName}/"), ) Vgo(options).run() assertThat(systemOutput.toString()) - .contains(Paths.get("src/test/resources/in-place-modify/avocado_example.xml").toString()) + .contains(Paths.get("build/test-results/inPlaceModification/${info.displayName}/avocado_example.xml").toString()) } @Test - fun `non-vector files are not mentioned in statistics reporting with a directory input`() { + fun `non-vector files are not mentioned in statistics reporting with a directory input`(info: TestInfo) { val options = Vgo.Options( printStats = true, - input = listOf("src/test/resources/in-place-modify"), + input = listOf("build/test-results/inPlaceModification/${info.displayName}"), ) Vgo(options).run() assertThat(systemOutput.toString()) - .doesNotContain("src/test/resources/in-place-modify/non_vector.xml") + .doesNotContain("build/test-results/inPlaceModification/${info.displayName}/non_vector.xml") } @Test - fun `only modified files appear in statistics reporting`() { + fun `only modified files appear in statistics reporting`(info: TestInfo) { val options = Vgo.Options( printStats = true, - input = listOf("src/test/resources/in-place-modify"), + input = listOf("build/test-results/inPlaceModification/${info.displayName}"), ) Vgo(options).run() assertThat(systemOutput.toString()) - .doesNotContain("src/test/resources/in-place-modify/avocado_example_optimized.xml") + .doesNotContain("build/test-results/inPlaceModification/${info.displayName}/avocado_example_optimized.xml") } @Test - fun `non-vector files are not modified`() { - val input = File("src/test/resources/in-place-modify/non_vector.xml") + fun `non-vector files are not modified`(info: TestInfo) { + val input = File("build/test-results/inPlaceModification/${info.displayName}/non_vector.xml") val before = input.readText() val options = @@ -106,4 +101,17 @@ class InPlaceModificationTest { val after = input.readText() assertThat(after).isEqualTo(before) } + + @Test + fun `format option results in new file extension`(info: TestInfo) { + val options = + Vgo.Options( + format = "svg", + input = listOf("build/test-results/inPlaceModification/${info.displayName}"), + ) + + Vgo(options).run() + + assertThat(File("build/test-results/inPlaceModification/${info.displayName}/avocado_example.svg")).exists() + } } From a0b8795a41ab296f6355c5efdfca9f8e3f9b2963 Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Sat, 12 Oct 2024 09:52:01 -0400 Subject: [PATCH 7/8] Add a changelog entry --- changelog.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/changelog.md b/changelog.md index 1c67a963..b168b9dd 100644 --- a/changelog.md +++ b/changelog.md @@ -15,6 +15,8 @@ ### Fixed +- Conversions without a specified output file will write a file the file extension corresponding to the format. + ### Security ## 2.4.0 - 2024-10-02 From eb07ebf3b390547286de46375ef95e4638749c5c Mon Sep 17 00:00:00 2001 From: Justin Brooks Date: Sat, 12 Oct 2024 15:11:30 -0400 Subject: [PATCH 8/8] Do not override extension for explicit file paths --- vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt | 13 +++++--- ...InPlaceModificationTest.kt => VgoTests.kt} | 31 +++++++++++++++---- 2 files changed, 34 insertions(+), 10 deletions(-) rename vgo/src/test/kotlin/com/jzbrooks/vgo/{InPlaceModificationTest.kt => VgoTests.kt} (70%) diff --git a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt index f2a0d9e4..c9c519cf 100644 --- a/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt +++ b/vgo/src/main/kotlin/com/jzbrooks/vgo/Vgo.kt @@ -24,6 +24,7 @@ import kotlin.io.path.isDirectory import kotlin.io.path.isRegularFile import kotlin.io.path.isSameFileAs import kotlin.io.path.nameWithoutExtension +import kotlin.io.path.pathString import kotlin.math.absoluteValue import kotlin.math.roundToInt @@ -154,10 +155,14 @@ class Vgo( } val output = - when (this.options.format) { - "vd" -> outputPath.resolveSibling("${outputPath.nameWithoutExtension}.xml") - "svg" -> outputPath.resolveSibling("${outputPath.nameWithoutExtension}.svg") - else -> outputPath + if (input.path == outputPath.pathString) { + when (this.options.format) { + "vd" -> outputPath.resolveSibling("${outputPath.nameWithoutExtension}.xml") + "svg" -> outputPath.resolveSibling("${outputPath.nameWithoutExtension}.svg") + else -> outputPath + } + } else { + outputPath }.toFile() if (output.parentFile?.exists() == false) output.parentFile.mkdirs() diff --git a/vgo/src/test/kotlin/com/jzbrooks/vgo/InPlaceModificationTest.kt b/vgo/src/test/kotlin/com/jzbrooks/vgo/VgoTests.kt similarity index 70% rename from vgo/src/test/kotlin/com/jzbrooks/vgo/InPlaceModificationTest.kt rename to vgo/src/test/kotlin/com/jzbrooks/vgo/VgoTests.kt index 984d566e..6c7443ad 100644 --- a/vgo/src/test/kotlin/com/jzbrooks/vgo/InPlaceModificationTest.kt +++ b/vgo/src/test/kotlin/com/jzbrooks/vgo/VgoTests.kt @@ -5,6 +5,7 @@ import assertk.assertions.contains import assertk.assertions.doesNotContain import assertk.assertions.exists import assertk.assertions.isEqualTo +import assertk.assertions.isFalse import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -14,7 +15,7 @@ import java.io.File import java.io.PrintStream import java.nio.file.Paths -class InPlaceModificationTest { +class VgoTests { private lateinit var systemOutput: ByteArrayOutputStream @BeforeEach @@ -47,7 +48,7 @@ class InPlaceModificationTest { } @Test - fun `individual file statistics are reported with a directory input`(info: TestInfo) { + fun `in-place individual file statistics are reported with a directory input`(info: TestInfo) { val options = Vgo.Options( printStats = true, @@ -60,7 +61,7 @@ class InPlaceModificationTest { } @Test - fun `non-vector files are not mentioned in statistics reporting with a directory input`(info: TestInfo) { + fun `in-place non-vector files are not mentioned in statistics reporting with a directory input`(info: TestInfo) { val options = Vgo.Options( printStats = true, @@ -74,7 +75,7 @@ class InPlaceModificationTest { } @Test - fun `only modified files appear in statistics reporting`(info: TestInfo) { + fun `in-place only modified files appear in statistics reporting`(info: TestInfo) { val options = Vgo.Options( printStats = true, @@ -88,7 +89,7 @@ class InPlaceModificationTest { } @Test - fun `non-vector files are not modified`(info: TestInfo) { + fun `in-place non-vector files are not modified`(info: TestInfo) { val input = File("build/test-results/inPlaceModification/${info.displayName}/non_vector.xml") val before = input.readText() @@ -103,7 +104,7 @@ class InPlaceModificationTest { } @Test - fun `format option results in new file extension`(info: TestInfo) { + fun `in-place format option results in new file extension`(info: TestInfo) { val options = Vgo.Options( format = "svg", @@ -114,4 +115,22 @@ class InPlaceModificationTest { assertThat(File("build/test-results/inPlaceModification/${info.displayName}/avocado_example.svg")).exists() } + + @Test + fun `output-specified file extension is not overwritten by conversion format`(info: TestInfo) { + val options = + Vgo.Options( + format = "svg", + input = listOf("build/test-results/inPlaceModification/${info.displayName}/avocado_example.xml"), + output = listOf("build/test-results/inPlaceModification/${info.displayName}/avocado_example.vec"), + ) + + Vgo(options).run() + + assertThat(File("build/test-results/inPlaceModification/${info.displayName}/avocado_example.vec")).exists() + assertThat(File("build/test-results/inPlaceModification/${info.displayName}/avocado_example.svg")) + .transform("exists") { + it.exists() + }.isFalse() // todo: replace this with doesNotExists() when assertk is updated + } }