diff --git a/components/parser/unified/src/commonTest/kotlin/io/github/composegears/valkyrie/parser/unified/util/IconNameFormatterTest.kt b/components/parser/unified/src/commonTest/kotlin/io/github/composegears/valkyrie/parser/unified/util/IconNameFormatterTest.kt index c4b18002d..215a9259d 100644 --- a/components/parser/unified/src/commonTest/kotlin/io/github/composegears/valkyrie/parser/unified/util/IconNameFormatterTest.kt +++ b/components/parser/unified/src/commonTest/kotlin/io/github/composegears/valkyrie/parser/unified/util/IconNameFormatterTest.kt @@ -49,4 +49,18 @@ class IconNameFormatterTest { assertThat(iconName).isEqualTo(it.expected) } } + + @Test + fun `check case-insensitive collision detection`() { + // These files produce different icon names that collide on case-insensitive file systems + val testCases = listOf( + IconTest(fileName = "test-icon.svg", expected = "TestIcon"), + IconTest(fileName = "testicon.svg", expected = "Testicon"), + ) + + testCases.forEach { + val iconName = IconNameFormatter.format(it.fileName) + assertThat(iconName).isEqualTo(it.expected) + } + } } diff --git a/tools/cli/CHANGELOG.md b/tools/cli/CHANGELOG.md index 8eaab3c15..7505055c2 100644 --- a/tools/cli/CHANGELOG.md +++ b/tools/cli/CHANGELOG.md @@ -5,6 +5,8 @@ ### Added - Add `--use-path-data-string` option to generate addPath with pathData strings instead of path builder calls +- Add validation for exact duplicate icon names (e.g., `test-icon.svg` and `test_icon.svg` both produce `TestIcon.kt`) +- Add validation for case-insensitive duplicate icon names to prevent file overwrites on macOS/Windows ### Removed diff --git a/tools/cli/src/main/kotlin/io/github/composegears/valkyrie/cli/command/SvgXmlToImageVectorCommand.kt b/tools/cli/src/main/kotlin/io/github/composegears/valkyrie/cli/command/SvgXmlToImageVectorCommand.kt index 193da6295..2956447f2 100644 --- a/tools/cli/src/main/kotlin/io/github/composegears/valkyrie/cli/command/SvgXmlToImageVectorCommand.kt +++ b/tools/cli/src/main/kotlin/io/github/composegears/valkyrie/cli/command/SvgXmlToImageVectorCommand.kt @@ -207,9 +207,9 @@ private fun svgXml2ImageVector( outputInfo("Start processing...") - val fullQualifiedNames = iconPaths - .map { IconNameFormatter.format(name = it.name) } - .filter { reservedComposeQualifiers.contains(it) } + val iconNames = iconPaths.map { IconNameFormatter.format(name = it.name) } + + val fullQualifiedNames = iconNames.filter { reservedComposeQualifiers.contains(it) } if (fullQualifiedNames.isNotEmpty()) { outputInfo( @@ -218,6 +218,40 @@ private fun svgXml2ImageVector( ) } + // Check for exact duplicates + val exactDuplicates = iconNames + .groupBy { it } + .filter { it.value.size > 1 } + .keys + .toList() + .sorted() + + if (exactDuplicates.isNotEmpty()) { + outputError( + "Found duplicate icon names: ${exactDuplicates.joinToString(", ")}. " + + "Each icon must have a unique name. " + + "Please rename the source files to avoid duplicates.", + ) + } + + // Check for case-insensitive duplicates that would cause file overwrites on case-insensitive file systems + val caseInsensitiveDuplicates = iconNames + .groupBy { it.lowercase() } + .filter { it.value.size > 1 && it.value.distinct().size > 1 } + .values + .flatten() + .distinct() + .sorted() + + if (caseInsensitiveDuplicates.isNotEmpty()) { + outputError( + "Found icon names that would collide on case-insensitive file systems (macOS/Windows): " + + "${caseInsensitiveDuplicates.joinToString(", ")}. " + + "These icons would overwrite each other during generation. " + + "Please rename the source files to avoid case-insensitive duplicates.", + ) + } + val config = ImageVectorGeneratorConfig( packageName = packageName, iconPackPackage = packageName, diff --git a/tools/cli/src/test/kotlin/io/github/composegears/valkyrie/cli/command/SvgXmlToImageVectorCommandTest.kt b/tools/cli/src/test/kotlin/io/github/composegears/valkyrie/cli/command/SvgXmlToImageVectorCommandTest.kt index e9ce115d1..456c2e74f 100644 --- a/tools/cli/src/test/kotlin/io/github/composegears/valkyrie/cli/command/SvgXmlToImageVectorCommandTest.kt +++ b/tools/cli/src/test/kotlin/io/github/composegears/valkyrie/cli/command/SvgXmlToImageVectorCommandTest.kt @@ -135,4 +135,82 @@ class SvgXmlToImageVectorCommandTest { verify { outputInfo("process = $path") } } } + + @Test + fun `should throw error for case-insensitive duplicate icon names`() { + val mockMessage = "Found icon names that would collide on case-insensitive file systems (macOS/Windows): " + + "TestIcon, Testicon. " + + "These icons would overwrite each other during generation. " + + "Please rename the source files to avoid case-insensitive duplicates." + + mockkStatic(::outputError) + every { outputError(mockMessage) } answers { error(mockMessage) } + + val tempDir = createTempDirectory() + + // Create two SVG files that produce case-insensitive duplicates: + // test-icon.svg -> TestIcon.kt + // testicon.svg -> Testicon.kt + // On case-insensitive file systems, these would collide + val svgContent = """ + + + + """.trimIndent() + + tempDir.resolve("test-icon.svg").toFile().writeText(svgContent) + tempDir.resolve("testicon.svg").toFile().writeText(svgContent) + + val exception = assertFailsWith { + SvgXmlToImageVectorCommand().test( + "--input-path", + tempDir.absolutePathString(), + "--output-path", + createTempDirectory().absolutePathString(), + "--package-name", + "com.example", + ) + } + + assertThat(exception.message).isEqualTo(mockMessage) + verify { outputError(mockMessage) } + } + + @Test + fun `should throw error for exact duplicate icon names`() { + val mockMessage = "Found duplicate icon names: TestIcon. " + + "Each icon must have a unique name. " + + "Please rename the source files to avoid duplicates." + + mockkStatic(::outputError) + every { outputError(mockMessage) } answers { error(mockMessage) } + + val tempDir = createTempDirectory() + + val svgContent = """ + + + + """.trimIndent() + + // Create two SVG files that produce the exact same icon name: + // test-icon.svg -> TestIcon.kt + // test_icon.svg -> TestIcon.kt (same result!) + tempDir.resolve("test-icon.svg").toFile().writeText(svgContent) + tempDir.resolve("test_icon.svg").toFile().writeText(svgContent) + + val exception = assertFailsWith { + SvgXmlToImageVectorCommand().test( + "--input-path", + tempDir.absolutePathString(), + "--output-path", + createTempDirectory().absolutePathString(), + "--package-name", + "com.example", + ) + } + + assertThat(exception.message).isEqualTo(mockMessage) + verify { outputError(mockMessage) } + } } diff --git a/tools/gradle-plugin/CHANGELOG.md b/tools/gradle-plugin/CHANGELOG.md index 5e744f3d5..37ff6c032 100644 --- a/tools/gradle-plugin/CHANGELOG.md +++ b/tools/gradle-plugin/CHANGELOG.md @@ -6,6 +6,10 @@ - Add `usePathDataString` configuration option in `imageVector` block to generate addPath with pathData strings instead of path builder calls +- Add validation for exact duplicate icon names (e.g., `test-icon.svg` and `test_icon.svg` both produce `TestIcon.kt`) +- Add validation for case-insensitive duplicate icon names to prevent file overwrites on macOS/Windows +- Add nested pack aware validation that correctly handles `useFlatPackage` mode - when enabled, duplicates are detected + across all nested packs since they write to the same output folder ### Removed diff --git a/tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt b/tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt index 874ccc374..cabf02c8f 100644 --- a/tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt +++ b/tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt @@ -105,9 +105,9 @@ internal abstract class GenerateImageVectorsTask : DefaultTask() { outputDirectory.mkdirs() // Detect icons with names conflicting with reserved Compose qualifiers - val fullQualifiedNames = iconFiles.files - .map { IconNameFormatter.format(name = it.name) } - .filter { reservedComposeQualifiers.contains(it) } + val iconNames = iconFiles.files.map { IconNameFormatter.format(name = it.name) } + + val fullQualifiedNames = iconNames.filter { reservedComposeQualifiers.contains(it) } if (fullQualifiedNames.isNotEmpty()) { logger.lifecycle( @@ -116,6 +116,9 @@ internal abstract class GenerateImageVectorsTask : DefaultTask() { ) } + // Check for duplicates with nested pack awareness + validateDuplicates(iconFiles.files.toList(), iconNames) + if (iconPack.isPresent && iconPack.get().targetSourceSet.get() == sourceSet.get()) { generateIconPack(outputDirectory = outputDirectory) } @@ -391,4 +394,119 @@ internal abstract class GenerateImageVectorsTask : DefaultTask() { return null } + + private fun validateDuplicates(files: List, iconNames: List) { + if (iconPack.isPresent) { + val pack = iconPack.get() + val nestedPacks = pack.nestedPacks.get() + val useFlatPackage = pack.useFlatPackage.get() + + if (nestedPacks.isNotEmpty()) { + // Build map: file -> nested pack name + val sourceFolderToNestedPack = nestedPacks.associateBy { it.sourceFolder.get() } + + // When useFlatPackage is false, only validate files that are in configured nested pack folders + // (matching the behavior of generateIconsForNestedPacks) + val filesToValidate = if (useFlatPackage) { + files + } else { + files.filter { file -> sourceFolderToNestedPack.containsKey(file.parentFile.name) } + } + + val fileToNestedPack = filesToValidate.associateWith { file -> + sourceFolderToNestedPack[file.parentFile.name]?.name?.get() + } + + // Group by nested pack (or single group if useFlatPackage) + val iconsByPack = filesToValidate.groupBy { file -> + if (useFlatPackage) { + pack.name.get() // All in same pack when flat + } else { + val nestedPackName = fileToNestedPack[file] + if (nestedPackName != null) "${pack.name.get()}.$nestedPackName" else pack.name.get() + } + } + + iconsByPack.forEach { (packIdentifier, filesInPack) -> + val names = filesInPack.map { IconNameFormatter.format(name = it.name) } + + // Check exact duplicates within this pack/group + val exactDuplicates = names + .groupBy { it } + .filter { it.value.size > 1 } + .keys + .toList() + .sorted() + + if (exactDuplicates.isNotEmpty()) { + throw GradleException( + "Found duplicate icon names in \"$packIdentifier\": ${exactDuplicates.joinToString(", ")}. " + + "Each icon must have a unique name. " + + "Please rename the source files to avoid duplicates.", + ) + } + + // Check case-insensitive duplicates within this pack/group + val caseInsensitiveDuplicates = names + .groupBy { it.lowercase() } + .filter { it.value.size > 1 && it.value.distinct().size > 1 } + .values + .flatten() + .distinct() + .sorted() + + if (caseInsensitiveDuplicates.isNotEmpty()) { + throw GradleException( + "Found icon names that would collide on case-insensitive file systems (macOS/Windows) in \"$packIdentifier\": " + + "${caseInsensitiveDuplicates.joinToString(", ")}. " + + "These icons would overwrite each other during generation. " + + "Please rename the source files to avoid case-insensitive duplicates.", + ) + } + } + } else { + // Single pack - check all files together + checkDuplicatesInIconNames(iconNames, "icon pack \"${pack.name.get()}\"") + } + } else { + // No icon pack - check all files together + checkDuplicatesInIconNames(iconNames, "package \"${packageName.get()}\"") + } + } + + private fun checkDuplicatesInIconNames(names: List, context: String) { + // Check exact duplicates + val exactDuplicates = names + .groupBy { it } + .filter { it.value.size > 1 } + .keys + .toList() + .sorted() + + if (exactDuplicates.isNotEmpty()) { + throw GradleException( + "Found duplicate icon names in $context: ${exactDuplicates.joinToString(", ")}. " + + "Each icon must have a unique name. " + + "Please rename the source files to avoid duplicates.", + ) + } + + // Check case-insensitive duplicates + val caseInsensitiveDuplicates = names + .groupBy { it.lowercase() } + .filter { it.value.size > 1 && it.value.distinct().size > 1 } + .values + .flatten() + .distinct() + .sorted() + + if (caseInsensitiveDuplicates.isNotEmpty()) { + throw GradleException( + "Found icon names that would collide on case-insensitive file systems (macOS/Windows) in $context: " + + "${caseInsensitiveDuplicates.joinToString(", ")}. " + + "These icons would overwrite each other during generation. " + + "Please rename the source files to avoid case-insensitive duplicates.", + ) + } + } } diff --git a/tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/FailedConfigurationTest.kt b/tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/FailedConfigurationTest.kt index 5335941b1..e7b8345c4 100644 --- a/tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/FailedConfigurationTest.kt +++ b/tools/gradle-plugin/src/test/kotlin/io/github/composegears/valkyrie/gradle/FailedConfigurationTest.kt @@ -3,8 +3,10 @@ package io.github.composegears.valkyrie.gradle import assertk.assertThat import assertk.assertions.contains import io.github.composegears.valkyrie.gradle.common.CommonGradleTest +import io.github.composegears.valkyrie.gradle.internal.DEFAULT_RESOURCE_DIRECTORY import io.github.composegears.valkyrie.gradle.internal.TASK_NAME import java.nio.file.Path +import kotlin.io.path.createDirectories import kotlin.io.path.writeText import org.junit.jupiter.api.Test import org.junit.jupiter.api.io.TempDir @@ -313,4 +315,274 @@ class FailedConfigurationTest : CommonGradleTest() { val result = failTask(root, TASK_NAME) assertThat(result.output).contains("Duplicate sourceFolder found: \"outlined\"") } + + @Test + fun `case-insensitive duplicate icon names`(@TempDir root: Path) { + root.writeSettingsFile() + root.resolve("build.gradle.kts").writeText( + """ + plugins { + kotlin("jvm") + id("io.github.composegears.valkyrie") + } + + valkyrie { + packageName = "x.y.z" + } + """.trimIndent(), + ) + + val svgDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY").createDirectories() + + val svgContent = """ + + + + """.trimIndent() + + // test-icon.svg -> TestIcon.kt + svgDir.resolve("test-icon.svg").writeText(svgContent) + // testicon.svg -> Testicon.kt + // On case-insensitive file systems (macOS/Windows), TestIcon.kt and Testicon.kt collide + svgDir.resolve("testicon.svg").writeText(svgContent) + + val result = failTask(root, TASK_NAME) + assertThat(result.output).contains("Found icon names that would collide on case-insensitive file systems") + assertThat(result.output).contains("TestIcon") + assertThat(result.output).contains("Testicon") + } + + @Test + fun `nested packs with flatPackage - exact duplicates across different nested packs`(@TempDir root: Path) { + root.writeSettingsFile() + root.resolve("build.gradle.kts").writeText( + """ + plugins { + kotlin("jvm") + id("io.github.composegears.valkyrie") + } + + valkyrie { + packageName = "x.y.z" + + iconPack { + name = "ValkyrieIcons" + targetSourceSet = "main" + useFlatPackage = true + + nested { + name = "Filled" + sourceFolder = "filled" + } + nested { + name = "Outlined" + sourceFolder = "outlined" + } + } + } + """.trimIndent(), + ) + + val svgContent = """ + + + + """.trimIndent() + + // Create same icon name in different nested packs + val filledDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY/filled").createDirectories() + filledDir.resolve("test-icon.svg").writeText(svgContent) + + val outlinedDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY/outlined").createDirectories() + outlinedDir.resolve("test-icon.svg").writeText(svgContent) + + val result = failTask(root, TASK_NAME) + assertThat(result.output).contains("Found duplicate icon names in \"ValkyrieIcons\": TestIcon") + } + + @Test + fun `nested packs with flatPackage - case-insensitive duplicates across different nested packs`(@TempDir root: Path) { + root.writeSettingsFile() + root.resolve("build.gradle.kts").writeText( + """ + plugins { + kotlin("jvm") + id("io.github.composegears.valkyrie") + } + + valkyrie { + packageName = "x.y.z" + + iconPack { + name = "ValkyrieIcons" + targetSourceSet = "main" + useFlatPackage = true + + nested { + name = "Filled" + sourceFolder = "filled" + } + nested { + name = "Outlined" + sourceFolder = "outlined" + } + } + } + """.trimIndent(), + ) + + val svgContent = """ + + + + """.trimIndent() + + // Create case-insensitive duplicates in different nested packs + val filledDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY/filled").createDirectories() + filledDir.resolve("test-icon.svg").writeText(svgContent) + + val outlinedDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY/outlined").createDirectories() + outlinedDir.resolve("testicon.svg").writeText(svgContent) + + val result = failTask(root, TASK_NAME) + assertThat(result.output).contains("Found icon names that would collide on case-insensitive file systems") + assertThat(result.output).contains("ValkyrieIcons") + assertThat(result.output).contains("TestIcon") + assertThat(result.output).contains("Testicon") + } + + @Test + fun `nested packs without flatPackage - allow duplicates in different nested packs`(@TempDir root: Path) { + root.writeSettingsFile() + root.resolve("build.gradle.kts").writeText( + """ + plugins { + kotlin("jvm") + id("io.github.composegears.valkyrie") + } + + valkyrie { + packageName = "x.y.z" + + iconPack { + name = "ValkyrieIcons" + targetSourceSet = "main" + useFlatPackage = false + + nested { + name = "Filled" + sourceFolder = "filled" + } + nested { + name = "Outlined" + sourceFolder = "outlined" + } + } + } + """.trimIndent(), + ) + + val svgContent = """ + + + + """.trimIndent() + + // Create same icon name in different nested packs - should be OK without flatPackage + val filledDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY/filled").createDirectories() + filledDir.resolve("test-icon.svg").writeText(svgContent) + + val outlinedDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY/outlined").createDirectories() + outlinedDir.resolve("test-icon.svg").writeText(svgContent) + + val result = runTask(root, TASK_NAME) + assertThat(result).taskWasSuccessful(":$TASK_NAME") + } + + @Test + fun `nested packs without flatPackage - detect duplicates within same nested pack`(@TempDir root: Path) { + root.writeSettingsFile() + root.resolve("build.gradle.kts").writeText( + """ + plugins { + kotlin("jvm") + id("io.github.composegears.valkyrie") + } + + valkyrie { + packageName = "x.y.z" + + iconPack { + name = "ValkyrieIcons" + targetSourceSet = "main" + useFlatPackage = false + + nested { + name = "Filled" + sourceFolder = "filled" + } + } + } + """.trimIndent(), + ) + + val svgContent = """ + + + + """.trimIndent() + + // Create exact duplicates within same nested pack + val filledDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY/filled").createDirectories() + filledDir.resolve("test-icon.svg").writeText(svgContent) + filledDir.resolve("test_icon.svg").writeText(svgContent) // Same name after formatting + + val result = failTask(root, TASK_NAME) + assertThat(result.output).contains("Found duplicate icon names in \"ValkyrieIcons.Filled\": TestIcon") + } + + @Test + fun `nested packs without flatPackage - detect case-insensitive duplicates within same nested pack`(@TempDir root: Path) { + root.writeSettingsFile() + root.resolve("build.gradle.kts").writeText( + """ + plugins { + kotlin("jvm") + id("io.github.composegears.valkyrie") + } + + valkyrie { + packageName = "x.y.z" + + iconPack { + name = "ValkyrieIcons" + targetSourceSet = "main" + useFlatPackage = false + + nested { + name = "Outlined" + sourceFolder = "outlined" + } + } + } + """.trimIndent(), + ) + + val svgContent = """ + + + + """.trimIndent() + + // Create case-insensitive duplicates within same nested pack + val outlinedDir = root.resolve("src/main/$DEFAULT_RESOURCE_DIRECTORY/outlined").createDirectories() + outlinedDir.resolve("test-icon.svg").writeText(svgContent) + outlinedDir.resolve("testicon.svg").writeText(svgContent) + + val result = failTask(root, TASK_NAME) + assertThat(result.output).contains("Found icon names that would collide on case-insensitive file systems") + assertThat(result.output).contains("ValkyrieIcons.Outlined") + assertThat(result.output).contains("TestIcon") + assertThat(result.output).contains("Testicon") + } } diff --git a/tools/idea-plugin/CHANGELOG.md b/tools/idea-plugin/CHANGELOG.md index 0803b0e1a..d15caeacd 100644 --- a/tools/idea-plugin/CHANGELOG.md +++ b/tools/idea-plugin/CHANGELOG.md @@ -5,6 +5,9 @@ ### Added - Add export as file action for `Simple mode` and `ImageVector to XML` tool +- Add validation for exact duplicate icon names (e.g., `test-icon.svg` and `test_icon.svg` both produce `TestIcon.kt`) +- Add validation for case-insensitive duplicate icon names to prevent file overwrites on macOS/Windows +- Add automatic re-validation when `useFlatPackage` setting changes to detect new conflicts immediately ### Fixed diff --git a/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/textfield/ConfirmTextField.kt b/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/textfield/ConfirmTextField.kt index a8b432709..4f956a1c7 100644 --- a/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/textfield/ConfirmTextField.kt +++ b/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/jewel/textfield/ConfirmTextField.kt @@ -7,6 +7,7 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.text.input.rememberTextFieldState import androidx.compose.foundation.text.input.setTextAndPlaceCursorAtEnd import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.remember @@ -41,6 +42,13 @@ fun ConfirmTextField( val state = rememberTextFieldState(text) val focusManager = LocalFocusManager.current + // Update text field state when external text changes + LaunchedEffect(text) { + if (state.text.toString() != text) { + state.setTextAndPlaceCursorAtEnd(text) + } + } + val isError by remember { derivedStateOf { state.text.isEmpty() } } var focused by rememberMutableState { false } diff --git a/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/IconPackConversionState.kt b/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/IconPackConversionState.kt index 2e39f2267..302dee749 100644 --- a/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/IconPackConversionState.kt +++ b/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/IconPackConversionState.kt @@ -75,4 +75,5 @@ enum class ValidationError { IconNameEmpty, IconNameContainsSpace, HasDuplicates, + HasCaseInsensitiveDuplicates, } diff --git a/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/IconPackConversionViewModel.kt b/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/IconPackConversionViewModel.kt index ec899f3d8..a9fa82ffe 100644 --- a/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/IconPackConversionViewModel.kt +++ b/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/IconPackConversionViewModel.kt @@ -52,6 +52,9 @@ class IconPackConversionViewModel( private val _events = MutableSharedFlow() val events = _events.asSharedFlow() + private val isFlatPackage: Boolean + get() = inMemorySettings.current.flatPackage + init { val restoredState = iconsRecord.value @@ -63,7 +66,7 @@ class IconPackConversionViewModel( _state.updateState { BatchProcessing.IconPackCreationState( icons = restoredState, - importIssues = restoredState.checkImportIssues(), + importIssues = restoredState.checkImportIssues(useFlatPackage = isFlatPackage), ) } } @@ -84,6 +87,19 @@ class IconPackConversionViewModel( } } .launchIn(viewModelScope) + + inMemorySettings.settings + .onEach { settings -> + _state.updateState { + when (this) { + is BatchProcessing.IconPackCreationState -> { + copy(importIssues = icons.checkImportIssues(useFlatPackage = settings.flatPackage)) + } + else -> this + } + } + } + .launchIn(viewModelScope) } fun pickerEvent(events: PickerEvent) { @@ -105,7 +121,7 @@ class IconPackConversionViewModel( } else { copy( icons = iconsToProcess, - importIssues = iconsToProcess.checkImportIssues(), + importIssues = iconsToProcess.checkImportIssues(useFlatPackage = isFlatPackage), ) } } @@ -132,7 +148,7 @@ class IconPackConversionViewModel( } copy( icons = updatedIcons, - importIssues = updatedIcons.checkImportIssues(), + importIssues = updatedIcons.checkImportIssues(useFlatPackage = isFlatPackage), ) } else -> this @@ -228,7 +244,7 @@ class IconPackConversionViewModel( } copy( icons = icons, - importIssues = icons.checkImportIssues(), + importIssues = icons.checkImportIssues(useFlatPackage = isFlatPackage), ) } else -> this @@ -255,35 +271,96 @@ class IconPackConversionViewModel( } } - val nameGroups = processedIcons.groupBy { it.iconName.name } - val nameCounters = mutableMapOf() - - val icons = processedIcons.map { icon -> - val originalName = icon.iconName.name - val group = nameGroups[originalName] + // Group icons by their output location (considering useFlatPackage) + val iconsByLocation = processedIcons.groupBy { icon -> + when (val pack = icon.iconPack) { + is IconPack.Single -> pack.iconPackName + is IconPack.Nested -> when { + isFlatPackage -> pack.iconPackName // All in same location when flat + else -> "${pack.iconPackName}.${pack.currentNestedPack}" // Separate locations + } + } + } - if (group != null && group.size > 1) { - val counter = nameCounters.getOrDefault(originalName, 0) + 1 - nameCounters[originalName] = counter + // Process duplicates within each location group + val resolvedIcons = iconsByLocation.flatMap { (_, iconsInLocation) -> + // Track all committed names to ensure uniqueness across both passes + val committedNames = mutableSetOf() + + // First, resolve exact duplicates + val nameGroups = iconsInLocation.groupBy { it.iconName.name } + val nameCounters = mutableMapOf() + + val iconsWithResolvedExactDuplicates = iconsInLocation.map { icon -> + val originalName = icon.iconName.name + val group = nameGroups[originalName] + + if (group != null && group.size > 1) { + val counter = nameCounters.getOrDefault(originalName, 0) + 1 + nameCounters[originalName] = counter + + if (counter > 1) { + // Generate unique name by incrementing suffix until not in committedNames + var suffix = counter - 1 + var candidateName = "$originalName$suffix" + while (committedNames.any { it.equals(candidateName, ignoreCase = true) }) { + suffix++ + candidateName = "$originalName$suffix" + } + committedNames.add(candidateName) + icon.copy(iconName = IconName(candidateName)) + } else { + committedNames.add(originalName) + icon + } + } else { + committedNames.add(originalName) + icon + } + } - if (counter > 1) { - icon.copy(iconName = IconName("$originalName${counter - 1}")) + // Then, resolve case-insensitive duplicates + val lowercaseGroups = iconsWithResolvedExactDuplicates.groupBy { it.iconName.name.lowercase() } + val lowercaseCounters = mutableMapOf() + + iconsWithResolvedExactDuplicates.map { icon -> + val currentName = icon.iconName.name + val lowercaseKey = currentName.lowercase() + val group = lowercaseGroups[lowercaseKey] + + // Only process if there are multiple icons with same lowercase name but different actual names + if (group != null && group.size > 1 && group.map { it.iconName.name }.distinct().size > 1) { + val counter = lowercaseCounters.getOrDefault(lowercaseKey, 0) + 1 + lowercaseCounters[lowercaseKey] = counter + + if (counter > 1) { + // Generate unique name by incrementing suffix until not in committedNames + var suffix = counter - 1 + var candidateName = "$currentName$suffix" + while (committedNames.any { it.equals(candidateName, ignoreCase = true) }) { + suffix++ + candidateName = "$currentName$suffix" + } + committedNames.add(candidateName) + icon.copy(iconName = IconName(candidateName)) + } else { + // First in group - name is already in committedNames from exact duplicate pass + icon + } } else { icon } - } else { - icon } } - if (icons.isEmpty()) { + if (resolvedIcons.isEmpty()) { _events.emit(ConversionEvent.NothingToImport) reset() } else { _state.updateState { creationState.copy( - icons = icons, - importIssues = icons.checkImportIssues(), + icons = resolvedIcons, + importIssues = resolvedIcons.checkImportIssues(useFlatPackage = isFlatPackage), ) } } @@ -311,7 +388,7 @@ class IconPackConversionViewModel( BatchProcessing.IconPackCreationState( icons = icons, - importIssues = icons.checkImportIssues(), + importIssues = icons.checkImportIssues(useFlatPackage = isFlatPackage), ) } } @@ -350,7 +427,7 @@ class IconPackConversionViewModel( BatchProcessing.IconPackCreationState( icons = icons, - importIssues = icons.checkImportIssues(), + importIssues = icons.checkImportIssues(useFlatPackage = isFlatPackage), ) } } diff --git a/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/util/IconValidation.kt b/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/util/IconValidation.kt index d87394842..d850528f2 100644 --- a/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/util/IconValidation.kt +++ b/tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/util/IconValidation.kt @@ -7,11 +7,12 @@ import io.github.composegears.valkyrie.ui.screen.mode.iconpack.conversion.IconSo import io.github.composegears.valkyrie.ui.screen.mode.iconpack.conversion.ValidationError import io.github.composegears.valkyrie.ui.screen.mode.iconpack.conversion.ValidationError.FailedToParseClipboard import io.github.composegears.valkyrie.ui.screen.mode.iconpack.conversion.ValidationError.FailedToParseFile +import io.github.composegears.valkyrie.ui.screen.mode.iconpack.conversion.ValidationError.HasCaseInsensitiveDuplicates import io.github.composegears.valkyrie.ui.screen.mode.iconpack.conversion.ValidationError.HasDuplicates import io.github.composegears.valkyrie.ui.screen.mode.iconpack.conversion.ValidationError.IconNameContainsSpace import io.github.composegears.valkyrie.ui.screen.mode.iconpack.conversion.ValidationError.IconNameEmpty -fun List.checkImportIssues(): Map> { +fun List.checkImportIssues(useFlatPackage: Boolean = false): Map> { return buildMap { val brokenIcons = this@checkImportIssues.filterIsInstance() @@ -43,19 +44,53 @@ fun List.checkImportIssues(): Map> { val duplicates = this@checkImportIssues .filterIsInstance() .groupBy { + // When useFlatPackage is true, all icons go to the same folder regardless of nested pack + // So we need to check for duplicates across all nested packs within the same icon pack val packIdentifier = when (val pack = it.iconPack) { is IconPack.Single -> pack.iconPackName - is IconPack.Nested -> "${pack.iconPackName}.${pack.currentNestedPack}" + is IconPack.Nested -> when { + useFlatPackage -> pack.iconPackName // Flat package: check across all nested packs + else -> "${pack.iconPackName}.${pack.currentNestedPack}" // Separate folders per nested pack + } } packIdentifier to it.iconName.name } - .filter { it.value.size > 1 && it.value.any { it.iconName.name.isNotEmpty() } } + .filter { it.value.size > 1 } .values .flatten() .map { it.iconName } .distinct() + .sortedBy { it.name } addIfNotEmpty(error = HasDuplicates, icons = duplicates) + + // Check for case-insensitive duplicates (file system collision on macOS/Windows) + val caseInsensitiveDuplicates = this@checkImportIssues + .filterIsInstance() + .groupBy { + // When useFlatPackage is true, all icons go to the same folder regardless of nested pack + // So we need to check for duplicates across all nested packs within the same icon pack + val packIdentifier = when (val pack = it.iconPack) { + is IconPack.Single -> pack.iconPackName + is IconPack.Nested -> when { + useFlatPackage -> pack.iconPackName // Flat package: check across all nested packs + else -> "${pack.iconPackName}.${pack.currentNestedPack}" // Separate folders per nested pack + } + } + packIdentifier to it.iconName.name.lowercase() + } + .filter { + // Filter groups where there are multiple icons with the same lowercase name + // but they are not already exact duplicates + it.value.size > 1 && it.value.map { icon -> icon.iconName.name }.distinct().size > 1 + } + .values + .flatten() + .map { it.iconName } + .distinct() + .sortedBy { it.name } + + addIfNotEmpty(error = HasCaseInsensitiveDuplicates, icons = caseInsensitiveDuplicates) } } @@ -79,6 +114,7 @@ fun Map>.toMessageText(): String { IconNameEmpty -> "• Contains icon${if (isPlural) "s" else ""} with empty name" IconNameContainsSpace -> "• Contains icon${if (isPlural) "s" else ""} with space in name: $value" HasDuplicates -> "• Contains duplicate icon${if (isPlural) "s" else ""}: $value" + HasCaseInsensitiveDuplicates -> "• Contains icon${if (isPlural) "s" else ""} that collide on case-insensitive file systems (macOS/Windows): $value" } } } diff --git a/tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/util/IconValidationTest.kt b/tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/util/IconValidationTest.kt index 3845f2d7b..a4fd98e08 100644 --- a/tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/util/IconValidationTest.kt +++ b/tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/ui/screen/mode/iconpack/conversion/ui/util/IconValidationTest.kt @@ -151,4 +151,79 @@ class IconValidationTest { assertThat(icons.checkImportIssues()) .isEqualTo(mapOf(ValidationError.HasDuplicates to listOf(IconName("Test")))) } + + @Test + fun `nested with flatPackage, detect exact duplicates across different nested packs`() { + // When useFlatPackage = true, Test.kt from "Filled" and Test.kt from "Outlined" + // would overwrite each other since they go to the same folder + val icons = listOf( + validIcon("Test", nestedPack.copy(currentNestedPack = "Filled")), + validIcon("Test", nestedPack.copy(currentNestedPack = "Outlined")), + ) + assertThat(icons.checkImportIssues(useFlatPackage = true)) + .isEqualTo(mapOf(ValidationError.HasDuplicates to listOf(IconName("Test")))) + } + + @Test + fun `nested without flatPackage, allow exact duplicates in different nested packs`() { + // Without useFlatPackage, Filled/Test.kt and Outlined/Test.kt are in different folders + val icons = listOf( + validIcon("Test", nestedPack.copy(currentNestedPack = "Filled")), + validIcon("Test", nestedPack.copy(currentNestedPack = "Outlined")), + ) + assertThat(icons.checkImportIssues(useFlatPackage = false)).isEqualTo(emptyMap()) + } + + @Test + fun `single, detect case-insensitive duplicates on macOS file system`() { + // On macOS with case-insensitive file system, TestIcon.kt and Testicon.kt collide + val icons = listOf( + validIcon("TestIcon", singlePack), + validIcon("Testicon", singlePack), + ) + assertThat(icons.checkImportIssues()) + .isEqualTo(mapOf(ValidationError.HasCaseInsensitiveDuplicates to listOf(IconName("TestIcon"), IconName("Testicon")))) + } + + @Test + fun `nested, detect case-insensitive duplicates on macOS file system`() { + // On macOS with case-insensitive file system, TestIcon.kt and Testicon.kt collide + val icons = listOf( + validIcon("TestIcon", nestedPack), + validIcon("Testicon", nestedPack), + ) + assertThat(icons.checkImportIssues()) + .isEqualTo(mapOf(ValidationError.HasCaseInsensitiveDuplicates to listOf(IconName("TestIcon"), IconName("Testicon")))) + } + + @Test + fun `nested, allow same case-insensitive names in different nested packs`() { + val icons = listOf( + validIcon("TestIcon", nestedPack.copy(currentNestedPack = "Filled")), + validIcon("Testicon", nestedPack.copy(currentNestedPack = "Outlined")), + ) + assertThat(icons.checkImportIssues()).isEqualTo(emptyMap()) + } + + @Test + fun `nested with flatPackage, detect case-insensitive duplicates across different nested packs`() { + // When useFlatPackage = true, all icons go to the same folder + // So TestIcon.kt and Testicon.kt would collide even if they're in different nested packs + val icons = listOf( + validIcon("TestIcon", nestedPack.copy(currentNestedPack = "Filled")), + validIcon("Testicon", nestedPack.copy(currentNestedPack = "Outlined")), + ) + assertThat(icons.checkImportIssues(useFlatPackage = true)) + .isEqualTo(mapOf(ValidationError.HasCaseInsensitiveDuplicates to listOf(IconName("TestIcon"), IconName("Testicon")))) + } + + @Test + fun `nested with flatPackage, allow case-insensitive names in different icon packs`() { + // Even with useFlatPackage = true, different icon packs have separate folders + val icons = listOf( + validIcon("TestIcon", nestedPack.copy(iconPackName = "ValkyrieIcons", currentNestedPack = "Filled")), + validIcon("Testicon", nestedPack.copy(iconPackName = "AnotherIconPack", currentNestedPack = "Outlined")), + ) + assertThat(icons.checkImportIssues(useFlatPackage = true)).isEqualTo(emptyMap()) + } }