feat(template): add customizable code generation template system#249
Merged
rafaeltonholo merged 34 commits intomainfrom Apr 4, 2026
Merged
feat(template): add customizable code generation template system#249rafaeltonholo merged 34 commits intomainfrom
rafaeltonholo merged 34 commits intomainfrom
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughA comprehensive template system is introduced enabling customization of generated Kotlin icon code through Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI/Gradle
participant Processor
participant TemplateConfigReader
participant TemplateConfigParser
participant Converter
participant CodeEmitterFactory
participant TemplateEmitter
participant PlaceholderResolver
participant FileOutput
User->>CLI/Gradle: Provide SVG + template path (--template or DSL)
CLI/Gradle->>Processor: ParserConfig with template config
Processor->>TemplateConfigReader: discover/resolve(templatePath)
TemplateConfigReader->>TemplateConfigParser: parse(tomlContent)
TemplateConfigParser-->>TemplateConfigReader: TemplateEmitterConfig
TemplateConfigReader-->>Processor: TemplateEmitterConfig
Processor->>Converter: convert(content, config, templateEmitterConfig)
Converter->>CodeEmitterFactory: create(templateEmitterConfig)
CodeEmitterFactory->>TemplateEmitter: new TemplateEmitter(templateEmitterConfig, fallback)
CodeEmitterFactory-->>Converter: TemplateEmitter
Converter->>TemplateEmitter: emit(contents)
TemplateEmitter->>PlaceholderResolver: resolve(template, context)
PlaceholderResolver-->>TemplateEmitter: resolved output
TemplateEmitter-->>Converter: emitted Kotlin code
Converter-->>Processor: Flow<ConversionStep>
Processor->>FileOutput: Write generated .kt file
sequenceDiagram
participant TemplateString
participant TemplateLexer
participant Tokens
participant PlaceholderResolver
participant TemplateContext
participant ResolvedTokens
participant NullParamTrimmer
participant FinalOutput
TemplateString->>TemplateLexer: tokenize(template)
TemplateLexer->>Tokens: Extract ${namespace:key}
Tokens->>PlaceholderResolver: resolve(tokens, context)
PlaceholderResolver->>TemplateContext: icon/path/group/def variables
TemplateContext-->>PlaceholderResolver: resolved values
PlaceholderResolver->>ResolvedTokens: Text(value) or Null
ResolvedTokens->>NullParamTrimmer: assemble(tokens)
NullParamTrimmer->>FinalOutput: Remove null-param lines
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a TOML-driven template system to customize generated Kotlin output, integrating it across the CLI, Gradle plugin, and website playground, with accompanying docs and CI integrity checks.
Changes:
- Introduces a template engine (TOML parsing, placeholder lexing/resolution, template emitter) and wires it into code generation.
- Adds website playground support (template editor, validation, autocomplete/tooltips, persistence) and docs navigation/docs pages.
- Expands testing/CI with template golden outputs and a dedicated template integrity check workflow.
Reviewed changes
Copilot reviewed 106 out of 111 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/inject/WorkerGraph.kt | Updates worker DI graph documentation for template support. |
| website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/IconConvertWorkerFactory.kt | Parses optional template TOML in the worker and forwards config into conversion. |
| website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/ConversionTypes.kt | Adds templateToml to worker conversion input payload. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/validator/playground/template/TemplateTomlValidator.kt | Adds schema-aware TOML validation for the playground template editor. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/theme/SiteDimensions.kt | Adds docs layout bottom padding token. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/reducer/TemplateReducer.kt | Adds reducer for template editor actions/state. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/reducer/PlaygroundReducer.kt | Wires template reducer into main playground reducer. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplateTomlExtensions.kt | Adds helpers to detect “structural only” template TOML and gate sending/persisting. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplatePersistenceHelper.kt | Adds debounced template validation and localStorage persistence helper. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundViewModel.kt | Integrates template validation/persistence and blocks conversion on template errors. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundState.kt | Adds template editor state (content, errors, expanded state). |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundPersistence.kt | Adds versioned localStorage persistence for options + template TOML. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundOptions.kt | Marks options as @Serializable for persistence. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundAction.kt | Adds template-related actions to the playground action set. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/ConversionInputFactory.kt | Includes usable template TOML in conversion inputs (single + batch). |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt | Adds Kotlin/JS wrappers + custom Shiki transformer for template editor decorations. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiApi.kt | Allows passing transformers to Shiki highlight options. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/Shiki.kt | Loads @shikijs/transformers at runtime alongside Shiki. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Templates.kt | Adds a dedicated “Template System” docs page route. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Index.kt | Adds a docs card entry for the template system. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TooltipInfo.kt | Adds tooltip model for template editor UX. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TomlSectionInfo.kt | Adds model for TOML section metadata (schema). |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TomlKeyInfo.kt | Adds model for TOML key metadata (autocomplete). |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TemplateValidationError.kt | Adds validation error model for template editor. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/TemplateEditorHelpers.kt | Adds highlighting/annotations, info-icon wiring, and editor key handling utilities. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/PlaygroundToolbar.kt | Disables conversion when template errors exist; improves UX hints. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/PlaygroundSection.kt | Adds the template editor section and batches template TOML into conversions. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/InputPanel.kt | Extracts scroll sync to a shared effect component. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/BatchPhaseHeader.kt | Removes “Start conversion” from batch header (handled by toolbar). |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/BatchPanel.kt | Removes start conversion hook (handled by toolbar). |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.kt | Documents Gradle templateFile(...) DSL option and links to template docs. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliOption.kt | Documents new CLI template flags in website docs. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliDocsContent.kt | Adds a callout linking to template system docs. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/NavHeader.kt | Adds template docs link to navigation. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/CapabilitiesSection.kt | Lists “Template-based customization” among capabilities. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/TomlTooltip.kt | Adds hover tooltip effect + tooltip UI for template editor. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/ScrollSyncEffect.kt | Adds reusable scroll synchronization effect component. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/AutocompleteDropdown.kt | Adds autocomplete dropdown + cursor positioning utilities for TOML editor. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/DocNavDropdown.kt | Adds template docs link to dropdown; adjusts click behavior. |
| website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/layouts/DocsLayout.kt | Updates docs layout padding to include bottom spacing and adjusted left/right. |
| website/site/build.gradle.kts | Adds serialization plugin + JSON dependency for persistence. |
| svg-to-compose/src/nativeMain/kotlin/Main.kt | Adds CLI flags --template and --no-template; passes template config into parser config. |
| svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolverTest.kt | Adds unit tests for placeholder resolution + null elision behavior. |
| svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexerTest.kt | Adds unit tests for template lexer tokenization. |
| svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfigParserTest.kt | Adds unit tests for TOML parsing/validation of template config. |
| svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.kt | Adds unit tests for template config reading and auto-discovery. |
| svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitterTest.kt | Adds tests for per-node fragment emission and formatting behavior. |
| svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/NodeChunkerTest.kt | Adds tests for shared chunking logic and thresholds. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/method/MethodSizeAccountable.kt | Extracts shared base method-size constant for chunking estimation. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/config/TemplateConfig.kt | Adds parser-level template configuration (path + discovery toggle). |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ParserConfig.kt | Adds optional template config to parser config. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/ResolvedToken.kt | Introduces resolved-token model + column-aware multiline indentation helper. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolver.kt | Implements placeholder resolution (variables, fragments, def/imports) + trimming pipeline. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.kt | Implements null parameter stripping and line elision for resolved templates. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateToken.kt | Adds lexer token model. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.kt | Implements placeholder lexer/tokenizer for template strings. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfig.kt | Adds serializable template schema model (definitions/templates/fragments/color mapping). |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReader.kt | Adds explicit-path + auto-discovery reader for s2c.template.toml. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.kt | Adds TOML parsing + sanitization of empty sections + ${def:*} validation. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt | Adds per-node fragment emission for paths/groups with fallback behavior. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt | Adds template-based file emitter that integrates chunking, imports, file header, and preview. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.kt | Adds resolution context, import accumulation, and color-mapping substitution. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.kt | Defines placeholder namespaces, fragment names, and variable keys contract. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorNodeEmitter.kt | Adds helper to emit path commands only (for template path fragments). |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorEmitter.kt | Extracts shared chunking into NodeChunker; exposes preview snippet builder and preview imports. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/NodeChunker.kt | Adds shared node chunking utility used by both default and template emitters. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/CodeEmitterFactory.kt | Adds optional template config to emitter factory and wraps with TemplateEmitter when set. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/Processor.kt | Resolves template config (explicit/discovery) and passes it into emitter creation. |
| svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/Converter.kt | Plumbs template emitter config through converter API and default converter. |
| svg-to-compose/build.gradle.kts | Adds TOML dependency and updates serialization plugin alias usage. |
| svg-to-compose/README.md | Updates CLI help output to include new template flags. |
| svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingWorkAction.kt | Adds template configuration into parser config for worker execution. |
| svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingParameters.kt | Adds templateFilePath input parameter. |
| svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/ParseSvgToComposeIconTask.kt | Wires template file DSL into worker parameters. |
| svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.kt | Adds templateFile property, DSL implementation, and cache key contribution. |
| svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/dsl/parser/IconParserConfiguration.kt | Adds templateFile(RegularFile) DSL entry and docs. |
| svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt | Adds functional tests + golden comparisons for template-based output. |
| svg-to-compose-gradle-plugin/build.gradle.kts | Adds template TOML file as functional test input for cache correctness. |
| svg-to-compose-gradle-plugin/README.md | Documents template file option and links to template docs. |
| playground/s2c.template.toml | Adds a canonical template example used by integrity checks and functional tests. |
| playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/Icons.kt | Adds a receiver namespace object used by template-generated icons. |
| playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/IconBuilders.kt | Adds sample icon/path builder helpers referenced by template config. |
| playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/Colors.kt | Adds sample color constant for template color mapping. |
| playground/app/samples/template/smiley.svg | Adds sample SVG for template demo/testing. |
| playground/app/samples/template/shield-halved-solid.xml | Adds sample AVG for template demo/testing. |
| playground/app/samples/template/brasil.svg | Adds sample SVG for template demo/testing. |
| playground/app/samples/template/attention-filled.svg | Adds sample SVG for template demo/testing. |
| playground/app/build.gradle.kts | Adds a template-based conversion configuration in the demo app. |
| playground/.gitignore | Keeps template demo sources in git while ignoring other generated icon output. |
| integrity-check/expected/template/StrokeGradient.xml.template.kt | Adds golden output for template mode (AVG). |
| integrity-check/expected/template/ShieldHalvedSolid.xml.template.kt | Adds golden output for template mode (AVG). |
| integrity-check/expected/template/MaskWithGroup.svg.template.kt | Adds golden output for template mode (SVG). |
| integrity-check/expected/template/LinearGradient01.svg.template.kt | Adds golden output for template mode (SVG). |
| integrity-check/expected/template/AttentionFilled.svg.template.kt | Adds golden output for template mode (SVG). |
| gradle/libs.versions.toml | Adds ktoml dependency and renames serialization plugin alias. |
| docs/ARCHITECTURE.md | Documents optional template resolution stage in the pipeline. |
| config/detekt.yml | Updates complexity configuration to ignore private functions. |
| buildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.kt | Adjusts JS/WASM target config and disables tests where unsupported. |
| app.properties | Bumps project version to 3.0.0. |
| .github/workflows/pull_request.yml | Splits build/test from integrity checks; adds template integrity checks and artifact reuse. |
| .github/actions/template-integrity-check/action.yml | Adds composite action to run template integrity script. |
| .github/actions/cli-integrity-check/template-script.sh | Adds CLI script to generate/compare template-mode outputs. |
Comments suppressed due to low confidence (2)
gradle/libs.versions.toml:52
- The plugin alias was renamed from
kotlinx-serializationtoorg-jetbrains-kotlin-serialization, but there are still build scripts referencinglibs.plugins.kotlinx.serialization(e.g.website/site/build.gradle.kts,website/worker/build.gradle.kts,website/editor-wasm/build.gradle.kts,website/build.gradle.kts). Either update all usages to the new alias or keep a compatibility alias to avoid breaking builds.
[plugins]
app-cash-burst = { id = "app.cash.burst", version.ref = "burst" }
dev-zacsweers-metro = { id = "dev.zacsweers.metro", version.ref = "metro" }
org-jetbrains-kotlin-serialization = { id = "org.jetbrains.kotlin.plugin.serialization", version.ref = "kotlin" }
website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/DocNavDropdown.kt:167
- The trigger click handler now always navigates to
/docs, which makes the dropdown effectively unusable on touch devices (no hover) because users can’t open the menu to reach nested doc pages. Consider keeping click-to-toggle (and optionally providing a separate “Docs home” link inside the panel) so both mouse and touch users can access the menu.
...ompose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.kt
Show resolved
Hide resolved
...e-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingWorkAction.kt
Show resolved
Hide resolved
...jsMain/kotlin/dev/tonholo/s2c/website/validator/playground/template/TemplateTomlValidator.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.kt (1)
42-86:⚠️ Potential issue | 🟠 MajorModel template resolution as more than “file or fallback”.
This configuration can currently express “use this file” or “anything else”, but downstream template resolution now also has a disabled mode and
mergealways inherits the common file when unset. That leaves Gradle with no way to opt a child config out of auto-discovery or clear an inherited template once one is configured. A tri-state model (AUTO/DISABLED/EXPLICIT_FILE) or a separatenoTemplate()flag would make the DSL predictable again.Also applies to: 146-148
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.kt` around lines 42 - 86, The current templateFile model only supports “explicit file or fallback” and cannot represent disabled or auto modes; change the model to a tri-state (e.g., enum TemplateMode { AUTO, DISABLED, EXPLICIT_FILE }) or add a separate noTemplate() flag so child configs can opt-out or clear inherited templates. Replace the RegularFileProperty usage (templateFile: RegularFileProperty and templateFile(path: RegularFile)) with a Property<TemplateMode> plus an optional RegularFileProperty for the EXPLICIT_FILE path (or implement noTemplate() that sets a distinct disabled state), add a public noTemplate() method, and update merge/inheritance logic (including the related code around lines referenced 146-148) so unset means AUTO, EXPLICIT_FILE carries the file, and DISABLED clears inheritance rather than inheriting the common file. Ensure downstream resolution reads the new tri-state and handles AUTO/DISABLED/EXPLICIT_FILE accordingly.
🧹 Nitpick comments (8)
buildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.kt (1)
61-70: Consider adding a comment explaining why browser tests are disabled.The
useWasmJs()function has a helpful TODO comment for its disabled tests referencing issue#243. For consistency and maintainability, consider adding a similar comment here explaining why browser tests are disabled, so future maintainers understand the rationale.📝 Optional: Add documentation for disabled browser tests
fun KotlinMultiplatformExtension.useJs() { js { browser { testTask { + // TODO: Add explanation for why browser tests are disabled enabled = false } } nodejs() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@buildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.kt` around lines 61 - 70, Add a short comment inside the KotlinMultiplatformExtension.useJs() block explaining why browser tests are disabled (similar to the TODO in useWasmJs()), e.g., a TODO referencing the related issue number (for example `#243`) and a one-line rationale so future maintainers understand why testTask.enabled = false is set in the browser { testTask { ... } } section.website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt (2)
87-90: Defensive check forchildrencould also handle null.For maximum robustness against edge cases in HAST nodes, consider checking for both null and undefined, consistent with the pattern in
addClassToNode.♻️ Optional: More defensive null/undefined check
val children = node.children - if (children != js("undefined")) { + if (children != null && children != undefined) { node.children = children.concat(arrayOf(iconNode)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt` around lines 87 - 90, The existing check only guards against undefined for node.children; update the condition in the transformer (the block that assigns node.children = children.concat(arrayOf(iconNode))) to guard against both null and undefined (consistent with addClassToNode). Locate the occurrence referencing node.children in ShikiTransformers (the same function that appends iconNode) and change the if to only run when children is neither null nor undefined so the concat call is safe.
35-39: Consider extracting the undefined-check pattern for consistency.The check for JS
undefinedis done differently here (node.properties == js("undefined")) versus line 88 (children != js("undefined")). Additionally, the null-check on line 35 could be simplified since in Kotlin/JS,nullandundefinedare both handled by the==operator.♻️ Optional: Simplify the null/undefined check
private fun addClassToNode(node: dynamic, className: String) { - if (node.properties == null || node.properties == js("undefined")) { + if (node.properties == undefined) { node.properties = js("({})") } val existing: String? = node.properties["class"]?.toString() node.properties["class"] = if (existing.isNullOrEmpty()) className else "$existing $className" }In Kotlin/JS,
undefinedis a predefined constant that can be used directly, and the==operator handles bothnullandundefinedcases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt` around lines 35 - 39, Unify and simplify the null/undefined checks around node.properties in ShikiTransformers.kt: replace the current mixed checks (node.properties == js("undefined") and null check) with a single Kotlin/JS‑style comparison using == undefined (or a small helper like isNullOrUndefined(value)) so both null and JS undefined are handled consistently when initializing node.properties and when checking children; update the logic that sets node.properties = js("({})") and the retrieval of existing class (node.properties["class"]) to use this unified check (refer to node.properties and className in the shown block and the children checks elsewhere) to keep the pattern consistent across the file.svg-to-compose-gradle-plugin/build.gradle.kts (1)
19-21: Keep the template fixture with the plugin's functional-test resources.Using
playground/s2c.template.tomlhere couples plugin verification to another module's assets, so unrelated playground edits can invalidate or break this task. Please move the fixture into the plugin module's own test resources and declare that local file as the input instead. Based on learnings: Integration tests should use Gradle TestKit and place test fixtures insrc/test/resources/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose-gradle-plugin/build.gradle.kts` around lines 19 - 21, The inputs.file(...) declaration currently points to playground/s2c.template.toml which couples the plugin's functional test to another module; move the template fixture into this plugin module's test resources (e.g. src/test/resources/s2c.template.toml) and update the Gradle task input used by copyTemplateConfigToProject() to reference the local resource file instead (replace rootProject.layout.projectDirectory.file("playground/s2c.template.toml") with the plugin module's resource file reference), ensuring the task uses the new local path as its input so Gradle cache invalidation is driven by the module-local fixture.svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolverTest.kt (1)
76-85: Collapse the five inline null-elision tests into a single parameterized@Bursttest.Lines 140–213 contain five separate test methods (
null at start,null at end,null in middle,all params null,multiple nulls) that all exercise the same resolver rule with different fixture inputs and already use exactassertEqualsassertions. Moving them into a single@Burstmatrix withburstValues()would eliminate duplication and align with the codebase pattern established inSvgViewBoxTest,CssSpecificityTest, and other test files.(Note: Lines 76–85 and 216–239 test different scenarios and can remain as separate tests, but consider adding exact assertions if whitespace handling is critical.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolverTest.kt` around lines 76 - 85, The five nearly identical null-elision tests should be collapsed into a single parameterized `@Burst` test: replace the individual test methods that exercise PlaceholderResolver.resolve with different fixtures by creating a `@Burst-annotated` test that supplies a matrix of input templates and expected outputs (use burstValues() to enumerate the cases), call createContext and PlaceholderResolver.resolve for each case, and assert the exact expected output with assertEquals; keep the existing separate tests at the start and end (lines you noted) as-is and ensure the new burst test covers the previous five scenarios (names like "null at start", "null at end", "null in middle", "all params null", "multiple nulls") so duplication is removed while preserving exact whitespace/assertions..github/actions/cli-integrity-check/template-script.sh (1)
107-109: Use-eqfor numeric comparison in[test brackets.Using
==for numeric comparison inside[ ]is a string comparison. While it works in this case, using-eqis the idiomatic approach for numeric comparisons in shell scripts and avoids potential issues.🔧 Suggested fix
-if [ "${`#errors`[@]}" == 0 ]; then +if [ "${`#errors`[@]}" -eq 0 ]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/cli-integrity-check/template-script.sh around lines 107 - 109, The numeric comparison in the conditional using the errors array length ("${`#errors`[@]}") is using string equality operator "==" inside [ ], so change the test in the if condition to use the numeric comparison operator -eq (i.e., update the if statement that checks "${`#errors`[@]}" to use -eq) to correctly and idiomatically compare the number of errors.svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt (1)
104-130: Keep the template fixtures inside the plugin test resources.These helpers now pull inputs from
samples/andplayground/, so the functional tests depend on repo-root files outside the module and no longer follow the classpath-fixture pattern already used bycopyIconsToProject. Moving the SVG/AVG samples ands2c.template.tomlinto the module test resources would make the suite more stable and self-contained.As per coding guidelines, "Integration tests should use Gradle TestKit and place test fixtures in src/test/resources/".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt` around lines 104 - 130, The functional test currently reads fixtures from repo-root paths via templateSvgFiles, templateAvgFiles and copyTemplateConfigToProject (pulling from "samples/" and "playground/"), making tests depend on files outside the module; move those SVG/AVG sample files and s2c.template.toml into this module's src/test/resources and update copyTemplateIconsToProject and copyTemplateConfigToProject (and any callers like copyIconsToProject) to load fixtures via the classpath (e.g., use ClassLoader.getResource or javaClass.getResourceAsStream) instead of projectRoot.resolve, then copy the resource streams into the temporary projectDir icons/config files so tests use module-local resources and conform to Gradle TestKit fixture guidelines.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.kt (1)
22-28:icon_builderis still missing from the fragment contract.The template docs and examples treat
icon_builderas a first-class fragment, but callers still have to hard-code"icon_builder"becauseFragmentdoes not expose it here. Adding the constant would keep the contract centralized.♻️ Minimal fix
object Fragment { + const val ICON_BUILDER = "icon_builder" const val PATH_BUILDER = "path_builder" const val GROUP_BUILDER = "group_builder" const val CHUNK_FUNCTION_NAME = "chunk_function_name" const val CHUNK_FUNCTION_DEFINITION = "chunk_function_definition" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.kt` around lines 22 - 28, The Fragment object in TemplateConstants.kt is missing the icon_builder constant used by docs/examples; add a new constant const val ICON_BUILDER = "icon_builder" to the object Fragment (alongside PATH_BUILDER, GROUP_BUILDER, CHUNK_FUNCTION_NAME, CHUNK_FUNCTION_DEFINITION) so callers can reference Fragment.ICON_BUILDER instead of hard-coding the string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/template-integrity-check/action.yml:
- Around line 8-9: The CI step currently executes
"./.github/actions/cli-integrity-check/template-script.sh ." directly which can
fail when executable permissions aren’t preserved; update the run command in
action.yml to invoke the script with an explicit shell (e.g., "bash
./.github/actions/cli-integrity-check/template-script.sh .") so the
template-script.sh is executed via bash for portability and reliability.
In `@docs/TEMPLATES.md`:
- Around line 345-346: The Gradle guidance is inconsistent: decide and document
the correct behavior (omitting templateFile() either disables or enables
auto-discovery) and make both places consistent; if the intended behavior is
that omission enables auto-discovery/inheritance, change the earlier sentence to
say "Disable auto-discovery with --no-template (CLI) or by explicitly setting
templateFile(null)/templateFile('') in the Gradle DSL; omitting templateFile()
enables auto-discovery and inherits common config" and update the later "filled"
example text to match; alternatively, if omission should disable discovery,
change the "filled" example to state that omitting templateFile() disables
discovery—ensure both occurrences referencing templateFile() and the
--no-template flag are aligned.
In
`@svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.kt`:
- Around line 120-121: The calculateHash() currently appends only
templateFile.orNull?.asFile?.absolutePath which means edits to the template
won't change the task hash; update calculateHash() to read the templateFile
content (if templateFile is present) and include a content digest (e.g., SHA-256
of templateFile.readBytes()) in the returned hash string, or if you
intentionally want external fingerprinting, add a comment and assert that
CacheManager and workerExecutor.noIsolation() provide separate fingerprinting
for templateFilePath; reference calculateHash(), templateFile/templateFilePath,
CacheManager, outputs.upToDateWhen, and workerExecutor.noIsolation() when making
the change.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfig.kt`:
- Around line 87-88: The data class declaration ColorMappingDefinition currently
exceeds the 120-char Detekt limit; break the primary constructor across multiple
lines so each constructor parameter is on its own line and keep the
`@Serializable` annotation and `@SerialName`("import") annotation intact (e.g.,
split after the class name and list val name: String, `@SerialName`("import") val
importPackage: String, and val value: String each on separate lines) to satisfy
the rule without changing identifiers or annotations.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.kt`:
- Around line 33-46: When tryParsePlaceholder sees a `${` but returns null,
don't append only the `$` and continue; instead scan forward from the opening
`{` until the matching `}` (or end of input) and append the entire `${...}` span
to the current literal so malformed placeholders are consumed as one literal.
Update the branch in TemplateLexer where tryParsePlaceholder(input, cursor + 2)
is called: on result == null, locate the endIndex of the next `}` (or
input.length if missing), append input.substring(cursor, endIndex) to literal
(or build it char-by-char), and advance cursor to endIndex so the lexer won't
re-interpret the inner text as nested placeholders; keep using
TemplateToken.Literal and the existing tokens/literal handling.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.kt`:
- Around line 161-177: cleanupCommaArtifacts currently applies global
Regex.replace calls which can alter unrelated string literals; change it to only
normalize commas local to the removed null-param region. Update the call site in
NullParamTrimmer to pass the removal index or surrounding substring into
cleanupCommaArtifacts (or change cleanupCommaArtifacts signature to accept a
start/end index), then perform localized fixes using replaceFirst (or a single
replace on the extracted region) with the precompiled patterns LEADING_COMMA,
TRAILING_COMMA, and DOUBLE_COMMA and reinsert the cleaned region into the full
line; ensure symbols referenced are cleanupCommaArtifacts, LEADING_COMMA,
TRAILING_COMMA, DOUBLE_COMMA and the NullParamTrimmer code that computes the
removal bounds.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/ResolvedToken.kt`:
- Around line 25-60: The indentation pass in indentMultiLineValues uses column
positions that include ResolvedToken.Null entries that NullParamTrimmer later
removes, causing over-indentation; modify indentMultiLineValues to compute
column and apply indentation based on the post-trim token stream by skipping or
removing ResolvedToken.Null tokens before calculating column (or perform null
trimming up-front inside this function), e.g., operate on tokens.filter { it !is
ResolvedToken.Null } (or call the existing NullParamTrimmer) so the column
arithmetic and subsequent indenting of ResolvedToken.Text (fromPlaceholder)
reflect the final assembled output.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.kt`:
- Around line 69-113: Normalize the receiver source in forIcon by resolving a
single receiverResolved value (prefer contents.receiverType if non-empty,
otherwise use config.definitions.receiver?.name), strip any trailing '.' from
contents.receiverType (e.g., removeSuffix(".")), and then use that same
receiverResolved for computing iconPropertyName, setting IconVar.RECEIVER, and
deciding imports; when adding imports, only call ctx.addImport(...) for the
definition receiver's package/name when the resolved receiver actually came from
config.definitions.receiver (not when an explicit contents.receiverType was
provided).
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt`:
- Around line 131-154: buildChunkFunctionsContent currently emits chunk bodies
via nodeEmitter.emit and calls nodeEmitter.emitChunkFunctionDefinition for the
default branch, which bypasses template-aware emission and loses fragment/def
imports; change it to emit chunk bodies using templateNodeEmitter.emit (so
fragments, color mappings and path/group fragments are applied) and, when
emitting each chunk collect/merge any definition imports produced during
template emission back into the outer definitions set before building
TemplateContext; also update the default-branch fallback to call
templateNodeEmitter.emitChunkFunctionDefinition (or wrap its result to capture
imports) instead of nodeEmitter.emitChunkFunctionDefinition so chunk emissions
remain template-aware. Ensure references: buildChunkFunctionsContent,
TemplateContext, ChunkVar.NAME, ChunkVar.BODY,
nodeEmitter.emitChunkFunctionDefinition/nodeEmitter.emit, templateNodeEmitter,
templateEmitterConfig.fragments and templateEmitterConfig.definitions.imports
are used to locate and update the code.
- Around line 102-110: The current buildPreview treats a present preview config
with a null template as suppression; instead, update buildPreview (use symbols:
buildPreview, templateEmitterConfig.templates.preview, previewConfig.template,
contents.noPreview, buildDefaultPreview, PlaceholderResolver.resolve) so that
when previewConfig exists but previewConfig.template is null you fall through to
the existing fallback logic: return contents.noPreview ? "" :
buildDefaultPreview(contents, context); keep the current behavior that an
explicitly blank template (isBlank) returns "", otherwise resolve the trimmed
template via PlaceholderResolver.resolve; also add a regression test covering an
empty [templates.preview] section to assert fallback to default preview
behavior.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt`:
- Around line 99-123: The wrapMultiLineCall function currently splits parameters
by top-level commas and only tracks parentheses, which corrupts arguments
containing string literals, generics, lambdas or other nested delimiters (e.g.
name = "Foo, Bar" or "Group (Alt)"). Update wrapMultiLineCall (and the related
splitTopLevelParams/findMatchingCloseParen usage) to either skip reformatting
when the argument list contains string/char literals, angle brackets, braces or
bracketed expressions, or replace splitTopLevelParams with a token-aware
splitter: implement a simple state machine that scans innerContent and respects
quoted strings (with escapes), char literals, nested (), <>, {}, [] and ignores
commas inside those tokens; if implementing the state machine is too risky,
conservatively detect quotes or other delimiters in innerContent and return the
original resolved string to avoid corrupting valid Kotlin code.
- Around line 61-68: The current emitGroup in TemplateNodeEmitter returns
fallbackEmitter.emit(group) when Fragment.GROUP_BUILDER is missing, which causes
the fallback to emit the entire subtree and skips template-aware recursion for
descendants; instead, when the group fragment is missing, emit only the group's
wrapper using the fallback logic but still iterate group.commands and call
emit(it, context) for each child so nested path_builder/group fragments and
color-mapping collection are preserved; locate emitGroup, the
fallbackEmitter.emit(group) call, and the group.commands handling and replace
the early return with logic that uses the fallback only for the group's
header/footer (or build a wrapper string via fallback helper) while keeping
recursive emission of children via emit.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/DocNavDropdown.kt`:
- Around line 153-157: The onClick on the Row using DocNavDropdownTriggerStyle
currently calls context.router.navigateTo("/docs") which causes taps to
immediately navigate and prevents touch users from opening the submenu; change
the behavior so clicks/taps toggle/open the dropdown instead of navigating
directly (e.g., call the same open/toggle handler used for hover/keyboard), and
provide a distinct, accessible action for navigating to "/docs" (for example a
separate "All docs" menu item or a link inside the opened menu). Update
rememberPageContext()/Row usage so onClick triggers the dropdown open/toggle
logic rather than direct navigation via context.router.navigateTo("/docs").
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.kt`:
- Around line 367-370: The docs option text is inconsistent with the DSL: in
GradlePluginDocsContent update the OptionItem entry for
"templateFile(RegularFileProperty)" to match the API signature used in
IconParserConfiguration by changing its name to "templateFile(path:
RegularFile)"; locate the OptionItem call and adjust the displayed name string
so the docs reflect the actual method parameter signature used by
IconParserConfiguration.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt`:
- Around line 200-206: The docs show contradictory guidance about
templateFile(): update the text so both the inline example and the
warning/callout agree — either state that omitting templateFile() will inherit
from the common config / enable Gradle auto-discovery, or change the example to
explicitly call templateFile() if omission disables auto-discovery; modify the
callout or the example text around the icons {
theme("com.example.theme.AppTheme") } / templateFile() mention in
TemplateDocsContent.kt (and the similar block repeated later) so the page
consistently says the same behavior for templateFile().
---
Outside diff comments:
In
`@svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.kt`:
- Around line 42-86: The current templateFile model only supports “explicit file
or fallback” and cannot represent disabled or auto modes; change the model to a
tri-state (e.g., enum TemplateMode { AUTO, DISABLED, EXPLICIT_FILE }) or add a
separate noTemplate() flag so child configs can opt-out or clear inherited
templates. Replace the RegularFileProperty usage (templateFile:
RegularFileProperty and templateFile(path: RegularFile)) with a
Property<TemplateMode> plus an optional RegularFileProperty for the
EXPLICIT_FILE path (or implement noTemplate() that sets a distinct disabled
state), add a public noTemplate() method, and update merge/inheritance logic
(including the related code around lines referenced 146-148) so unset means
AUTO, EXPLICIT_FILE carries the file, and DISABLED clears inheritance rather
than inheriting the common file. Ensure downstream resolution reads the new
tri-state and handles AUTO/DISABLED/EXPLICIT_FILE accordingly.
---
Nitpick comments:
In @.github/actions/cli-integrity-check/template-script.sh:
- Around line 107-109: The numeric comparison in the conditional using the
errors array length ("${`#errors`[@]}") is using string equality operator "=="
inside [ ], so change the test in the if condition to use the numeric comparison
operator -eq (i.e., update the if statement that checks "${`#errors`[@]}" to use
-eq) to correctly and idiomatically compare the number of errors.
In
`@buildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.kt`:
- Around line 61-70: Add a short comment inside the
KotlinMultiplatformExtension.useJs() block explaining why browser tests are
disabled (similar to the TODO in useWasmJs()), e.g., a TODO referencing the
related issue number (for example `#243`) and a one-line rationale so future
maintainers understand why testTask.enabled = false is set in the browser {
testTask { ... } } section.
In `@svg-to-compose-gradle-plugin/build.gradle.kts`:
- Around line 19-21: The inputs.file(...) declaration currently points to
playground/s2c.template.toml which couples the plugin's functional test to
another module; move the template fixture into this plugin module's test
resources (e.g. src/test/resources/s2c.template.toml) and update the Gradle task
input used by copyTemplateConfigToProject() to reference the local resource file
instead (replace
rootProject.layout.projectDirectory.file("playground/s2c.template.toml") with
the plugin module's resource file reference), ensuring the task uses the new
local path as its input so Gradle cache invalidation is driven by the
module-local fixture.
In
`@svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt`:
- Around line 104-130: The functional test currently reads fixtures from
repo-root paths via templateSvgFiles, templateAvgFiles and
copyTemplateConfigToProject (pulling from "samples/" and "playground/"), making
tests depend on files outside the module; move those SVG/AVG sample files and
s2c.template.toml into this module's src/test/resources and update
copyTemplateIconsToProject and copyTemplateConfigToProject (and any callers like
copyIconsToProject) to load fixtures via the classpath (e.g., use
ClassLoader.getResource or javaClass.getResourceAsStream) instead of
projectRoot.resolve, then copy the resource streams into the temporary
projectDir icons/config files so tests use module-local resources and conform to
Gradle TestKit fixture guidelines.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.kt`:
- Around line 22-28: The Fragment object in TemplateConstants.kt is missing the
icon_builder constant used by docs/examples; add a new constant const val
ICON_BUILDER = "icon_builder" to the object Fragment (alongside PATH_BUILDER,
GROUP_BUILDER, CHUNK_FUNCTION_NAME, CHUNK_FUNCTION_DEFINITION) so callers can
reference Fragment.ICON_BUILDER instead of hard-coding the string.
In
`@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolverTest.kt`:
- Around line 76-85: The five nearly identical null-elision tests should be
collapsed into a single parameterized `@Burst` test: replace the individual test
methods that exercise PlaceholderResolver.resolve with different fixtures by
creating a `@Burst-annotated` test that supplies a matrix of input templates and
expected outputs (use burstValues() to enumerate the cases), call createContext
and PlaceholderResolver.resolve for each case, and assert the exact expected
output with assertEquals; keep the existing separate tests at the start and end
(lines you noted) as-is and ensure the new burst test covers the previous five
scenarios (names like "null at start", "null at end", "null in middle", "all
params null", "multiple nulls") so duplication is removed while preserving exact
whitespace/assertions.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt`:
- Around line 87-90: The existing check only guards against undefined for
node.children; update the condition in the transformer (the block that assigns
node.children = children.concat(arrayOf(iconNode))) to guard against both null
and undefined (consistent with addClassToNode). Locate the occurrence
referencing node.children in ShikiTransformers (the same function that appends
iconNode) and change the if to only run when children is neither null nor
undefined so the concat call is safe.
- Around line 35-39: Unify and simplify the null/undefined checks around
node.properties in ShikiTransformers.kt: replace the current mixed checks
(node.properties == js("undefined") and null check) with a single
Kotlin/JS‑style comparison using == undefined (or a small helper like
isNullOrUndefined(value)) so both null and JS undefined are handled consistently
when initializing node.properties and when checking children; update the logic
that sets node.properties = js("({})") and the retrieval of existing class
(node.properties["class"]) to use this unified check (refer to node.properties
and className in the shown block and the children checks elsewhere) to keep the
pattern consistent across the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1f8d8efe-2ba8-40d0-bedc-6d7a8ab3000e
⛔ Files ignored due to path filters (46)
config/detekt.ymlis excluded by!**/config/detekt.ymlintegrity-check/expected/template/Android.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/Android.xml.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/AttentionFilled.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/Brasil.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/LinearGradient01.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/MaskWithGroup.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/ShieldHalvedSolid.xml.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/StrokeGradient.xml.template.ktis excluded by!**/integrity-check/expected/**kotlin-js-store/yarn.lockis excluded by!**/yarn.lock,!**/*.lockplayground/.gitignoreis excluded by!**/playground/**playground/app/build.gradle.ktsis excluded by!**/playground/**playground/app/samples/template/attention-filled.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/samples/template/brasil.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/samples/template/shield-halved-solid.xmlis excluded by!**/samples/**,!**/playground/**playground/app/samples/template/smiley.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/Colors.ktis excluded by!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/IconBuilders.ktis excluded by!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/Icons.ktis excluded by!**/playground/**playground/s2c.template.tomlis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/AutocompleteDropdown.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/ScrollSyncEffect.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/TomlTooltip.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/BatchPanel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/BatchPhaseHeader.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/InputPanel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/PlaygroundSection.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/PlaygroundToolbar.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/TemplateEditorHelpers.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/TemplateEditorSection.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/template/TemplateEditorSchema.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TemplateValidationError.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TomlKeyInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TomlSectionInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TooltipInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/ConversionInputFactory.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundAction.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundOptions.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundPersistence.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundState.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundViewModel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplatePersistenceHelper.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplateTomlExtensions.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/reducer/PlaygroundReducer.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/reducer/TemplateReducer.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/validator/playground/template/TemplateTomlValidator.ktis excluded by!**/playground/**
📒 Files selected for processing (65)
.github/actions/cli-integrity-check/template-script.sh.github/actions/template-integrity-check/action.yml.github/workflows/pull_request.ymlapp.propertiesbuildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.ktdocs/ARCHITECTURE.mddocs/TEMPLATES.mdgradle/libs.versions.tomlsvg-to-compose-gradle-plugin/README.mdsvg-to-compose-gradle-plugin/build.gradle.ktssvg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/dsl/parser/IconParserConfiguration.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/ParseSvgToComposeIconTask.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingParameters.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingWorkAction.ktsvg-to-compose/README.mdsvg-to-compose/build.gradle.ktssvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/Converter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/Processor.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/CodeEmitterFactory.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/NodeChunker.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorNodeEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReader.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateToken.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolver.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/ResolvedToken.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ParserConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/config/TemplateConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/method/MethodSizeAccountable.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/NodeChunkerTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitterTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitterTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfigParserTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexerTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolverTest.ktsvg-to-compose/src/nativeMain/kotlin/Main.ktwebsite/site/build.gradle.ktswebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/layouts/DocsLayout.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/DocNavDropdown.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/CapabilitiesSection.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/NavHeader.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliOption.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Index.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Templates.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/Shiki.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiApi.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/theme/SiteDimensions.ktwebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/ConversionTypes.ktwebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/IconConvertWorkerFactory.ktwebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/inject/WorkerGraph.kt
...plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.kt
Outdated
Show resolved
Hide resolved
...mpose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfig.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt
Show resolved
Hide resolved
website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/DocNavDropdown.kt
Outdated
Show resolved
Hide resolved
...c/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.kt
Show resolved
Hide resolved
...e/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt (1)
148-151:⚠️ Potential issue | 🟠 MajorFallback branch still bypasses template-aware emission for chunk bodies.
The custom-fragment branch (lines 153-173) has been fixed to use
templateNodeEmitter.emit()and merge imports. However, the fallback branch (lines 148-151) still delegates entirely tonodeEmitter.emitChunkFunctionDefinition(it), which emits the entire chunk function including its body without template-aware emission.This means when no
CHUNK_FUNCTION_DEFINITIONfragment is configured but other fragments (likepath_builderorgroup_builder) are, paths/groups inside chunks won't receive fragment resolution or color mappings—creating inconsistency between the main icon body and chunk contents.🛠️ Proposed fix: Emit chunk bodies template-aware even in fallback
val defFragment = templateEmitterConfig.fragments[Fragment.CHUNK_FUNCTION_DEFINITION] - ?: return chunkFunctions.joinToString("\n\n") { - nodeEmitter.emitChunkFunctionDefinition(it) - } + ?: return chunkFunctions.joinToString("\n\n") { chunk -> + // Even without a custom fragment, emit body template-aware for path/group fragments + val chunkNodeContext = buildNodeContext(parentContext) + val body = chunk.nodes.joinToString("\n") { + templateNodeEmitter.emit(it, chunkNodeContext).trimEnd() + }.prependIndent(formatConfig.indentUnit) + parentContext.addImports(chunkNodeContext.collectedImports) + nodeEmitter.emitChunkFunctionDefinitionWithBody(chunk, body) + }This would require adding an
emitChunkFunctionDefinitionWithBody(chunk, body)method toImageVectorNodeEmitterthat accepts a pre-built body string, or restructuring to separate definition header from body emission.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt` around lines 148 - 151, The fallback branch currently calls nodeEmitter.emitChunkFunctionDefinition(it) which emits the full chunk including its body and bypasses template-aware fragment resolution; change this so the header is produced by a definition-only method and the body is emitted via templateNodeEmitter.emit(...) with merged imports (or add a new ImageVectorNodeEmitter.emitChunkFunctionDefinitionWithBody(chunk, body) that accepts the pre-built body); specifically update the code path that uses templateEmitterConfig.fragments[Fragment.CHUNK_FUNCTION_DEFINITION] and chunkFunctions.joinToString(...) to build the body with templateNodeEmitter.emit(...) (so path_builder/group_builder fragments and color mappings are applied) and then pass that body into the definition-emitter (or call the new emitChunkFunctionDefinitionWithBody) and ensure imports from the body are merged into the overall imports map.
🧹 Nitpick comments (4)
docs/TEMPLATES.md (3)
196-198: Consider clarifying the preview suppression example.The commented-out TOML format might confuse users. Consider either:
- Showing an actual uncommented example with
template = "", or- Adding explanatory text like "To suppress preview entirely, include the section with an empty template string:"
📝 Suggested clarification
-# Suppress preview entirely -# [templates.preview] -# template = "" +# To suppress preview entirely, include the section with an empty template: +[templates.preview] +template = ""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/TEMPLATES.md` around lines 196 - 198, Update the "Suppress preview entirely" snippet so it's unambiguous: replace the commented-out TOML block with an actual example section showing templates.preview and template = "" (e.g., [templates.preview] followed by template = ""), or add a single-line explanatory sentence above the commented block like "To suppress preview entirely, include the section with an empty template string:". Ensure the example references the exact symbol names templates.preview and template = "" so readers know where to set an empty template.
345-348: Clarify auto-discovery behavior when templateFile() is omitted everywhere.The documentation explains that omitting
templateFile()in a specific configuration means it inherits from common config, but doesn't clearly state what happens when both common and the specific configuration omittemplateFile(). Does this:
- Enable auto-discovery (walk up from output directory)?
- Disable auto-discovery entirely (no template used)?
Consider adding a sentence to clarify the behavior when templateFile() is omitted at all levels.
📝 Suggested clarification
Disable auto-discovery with `--no-template` (CLI). In the Gradle DSL, omitting `templateFile()` disables auto-discovery for that configuration; -set `templateFile()` in `common` to share a template across all -configurations via inheritance. +set `templateFile()` in `common` to share a template across all +configurations via inheritance. If `templateFile()` is omitted in both +common and specific configurations, auto-discovery is disabled and no +template is used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/TEMPLATES.md` around lines 345 - 348, Clarify that omitting templateFile() at all levels enables auto-discovery: update the docs near the existing paragraph (mentioning --no-template and templateFile()) to add a sentence stating that if templateFile() is not provided in both the common config and a specific configuration, the system falls back to auto-discovery (it walks up from the output directory to find a template); also note that passing --no-template on the CLI disables this auto-discovery. Reference templateFile(), auto-discovery, and --no-template when adding the single-sentence clarification.
305-311: Add language specifier to fenced code block.The code block should specify a language for proper syntax highlighting and to satisfy markdownlint.
🔧 Proposed fix
-``` +```kotlin path( fill = SolidColor(Color.Black), fillAlpha = ${path:fill_alpha}, <- this line is removed pathFillType = EvenOdd, )</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/TEMPLATES.mdaround lines 305 - 311, Update the fenced code block to
include a language specifier so markdownlint and syntax highlighting work:
change the opening triple-backticks for the snippet containing the path(...)
call (the block that shows fill = SolidColor(Color.Black), fillAlpha =
${path:fill_alpha}, pathFillType = EvenOdd) to use ```kotlin instead of plainsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt (1)
61-88: Good fix for template-aware child emission.Children are now correctly emitted with template-aware recursion (line 65) regardless of whether a group fragment is configured, ensuring nested paths/groups get color mappings and fragment resolution. The fallback at line 69 now properly uses only
emitGroupHeaderfor the wrapper while preserving template-aware children.One minor inconsistency:
wrapMultiLineCallis applied to path fragments (line 57) but not to group fragments (line 86). If group templates can contain multi-line parameter values (e.g., complex clip paths), they may not format consistently.💡 Optional: Apply wrapMultiLineCall to group fragments for consistency
- "$resolved {\n${childBody.prependIndent(formatConfig.indentUnit)}\n}" + val formatted = wrapMultiLineCall(resolved) + "$formatted {\n${childBody.prependIndent(formatConfig.indentUnit)}\n}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt` around lines 61 - 88, In emitGroup, wrap the resolved group fragment invocation with wrapMultiLineCall like you do for path fragments so multi-line parameter values format consistently; locate the Fragment.GROUP_BUILDER branch in emitGroup, take the resolved variable (created from context.fragments.getValue(Fragment.GROUP_BUILDER) and PlaceholderResolver.resolve) and pass it through wrapMultiLineCall before composing the final "$resolved {\n...}" string, keeping the existing childBody indentation and fallback behavior (emitGroupHeader) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.kt`:
- Line 393: Update the inline comment in GradlePluginDocsContent.kt (the comment
containing "// TIP: You can choose whatever name for the template, as soon as it
is a toml file.") to use the correct idiom and capitalization: replace "as soon
as it is a toml file" with "as long as it is a TOML file" so the comment reads
e.g. "// TIP: You can choose whatever name for the template, as long as it is a
TOML file." to fix grammar and acronym casing.
---
Duplicate comments:
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt`:
- Around line 148-151: The fallback branch currently calls
nodeEmitter.emitChunkFunctionDefinition(it) which emits the full chunk including
its body and bypasses template-aware fragment resolution; change this so the
header is produced by a definition-only method and the body is emitted via
templateNodeEmitter.emit(...) with merged imports (or add a new
ImageVectorNodeEmitter.emitChunkFunctionDefinitionWithBody(chunk, body) that
accepts the pre-built body); specifically update the code path that uses
templateEmitterConfig.fragments[Fragment.CHUNK_FUNCTION_DEFINITION] and
chunkFunctions.joinToString(...) to build the body with
templateNodeEmitter.emit(...) (so path_builder/group_builder fragments and color
mappings are applied) and then pass that body into the definition-emitter (or
call the new emitChunkFunctionDefinitionWithBody) and ensure imports from the
body are merged into the overall imports map.
---
Nitpick comments:
In `@docs/TEMPLATES.md`:
- Around line 196-198: Update the "Suppress preview entirely" snippet so it's
unambiguous: replace the commented-out TOML block with an actual example section
showing templates.preview and template = "" (e.g., [templates.preview] followed
by template = ""), or add a single-line explanatory sentence above the commented
block like "To suppress preview entirely, include the section with an empty
template string:". Ensure the example references the exact symbol names
templates.preview and template = "" so readers know where to set an empty
template.
- Around line 345-348: Clarify that omitting templateFile() at all levels
enables auto-discovery: update the docs near the existing paragraph (mentioning
--no-template and templateFile()) to add a sentence stating that if
templateFile() is not provided in both the common config and a specific
configuration, the system falls back to auto-discovery (it walks up from the
output directory to find a template); also note that passing --no-template on
the CLI disables this auto-discovery. Reference templateFile(), auto-discovery,
and --no-template when adding the single-sentence clarification.
- Around line 305-311: Update the fenced code block to include a language
specifier so markdownlint and syntax highlighting work: change the opening
triple-backticks for the snippet containing the path(...) call (the block that
shows fill = SolidColor(Color.Black), fillAlpha = ${path:fill_alpha},
pathFillType = EvenOdd) to use ```kotlin instead of plain ```; no other edits to
the snippet content are required.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt`:
- Around line 61-88: In emitGroup, wrap the resolved group fragment invocation
with wrapMultiLineCall like you do for path fragments so multi-line parameter
values format consistently; locate the Fragment.GROUP_BUILDER branch in
emitGroup, take the resolved variable (created from
context.fragments.getValue(Fragment.GROUP_BUILDER) and
PlaceholderResolver.resolve) and pass it through wrapMultiLineCall before
composing the final "$resolved {\n...}" string, keeping the existing childBody
indentation and fallback behavior (emitGroupHeader) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4a55bfe4-73a4-4506-b9d3-e71e90286969
⛔ Files ignored due to path filters (2)
playground/s2c.template.tomlis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplateTomlExtensions.ktis excluded by!**/playground/**
📒 Files selected for processing (10)
.github/actions/cli-integrity-check/template-script.sh.github/actions/template-integrity-check/action.ymldocs/TEMPLATES.mdsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorNodeEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt
✅ Files skipped from review due to trivial changes (2)
- .github/actions/template-integrity-check/action.yml
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorNodeEmitter.kt
- .github/actions/cli-integrity-check/template-script.sh
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt
...c/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.kt
Show resolved
Hide resolved
This was referenced Mar 28, 2026
rafaeltonholo
force-pushed
the
feat/template-system
branch
from
b9b5e9e to
84b34e6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt (1)
148-151:⚠️ Potential issue | 🟡 MinorDefault chunk function emission bypasses template-aware node emission.
When no
chunk_function_definitionfragment is configured, the fallback usesnodeEmitter.emitChunkFunctionDefinition(it)which emits chunk bodies through the non-template-awareImageVectorNodeEmitter. This means path/group fragments and color mappings won't apply inside chunks for users who only define apath_builderfragment without achunk_function_definition.Consider emitting the chunk body through
templateNodeEmittereven in the fallback case, then wrapping it with the default function signature.💡 Suggested approach
val defFragment = templateEmitterConfig.fragments[Fragment.CHUNK_FUNCTION_DEFINITION] - ?: return chunkFunctions.joinToString("\n\n") { - nodeEmitter.emitChunkFunctionDefinition(it) - } + ?: return chunkFunctions.joinToString("\n\n") { chunk -> + val chunkNodeContext = buildNodeContext(parentContext) + val body = chunk.nodes.joinToString("\n") { + templateNodeEmitter.emit(it, chunkNodeContext).trimEnd() + }.prependIndent(formatConfig.indentUnit) + parentContext.addImports(chunkNodeContext.collectedImports) + // Use fallback's chunk function wrapper with template-aware body + "private fun ImageVector.Builder.${chunk.functionName}() {\n$body\n}" + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt` around lines 148 - 151, The fallback currently calls nodeEmitter.emitChunkFunctionDefinition inside chunkFunctions.joinToString when templateEmitterConfig.fragments[Fragment.CHUNK_FUNCTION_DEFINITION] is missing, which bypasses template-aware emission; change the fallback to generate the chunk body via templateNodeEmitter (e.g., use templateNodeEmitter to emit the chunk body or a template-aware chunk emitter) and then wrap that body with the default chunk function signature so path/group fragments and color mappings from templateNodeEmitter apply; update the code path where Fragment.CHUNK_FUNCTION_DEFINITION is absent (the chunkFunctions.joinToString branch and the call to nodeEmitter.emitChunkFunctionDefinition) to call the template emitter for the body and concatenate the default function definition around it.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.kt (1)
99-100:⚠️ Potential issue | 🟠 MajorKeep comma cleanup scoped to the removed argument.
Line 99 still runs
cleanupCommaArtifacts()over the full assembled line. That can rewrite unrelated literals on the same line—e.g.label = ",)"or",,"—when a different placeholder resolves to null. Please normalize only the slice around the removed parameter instead of post-processing the entire string.Also applies to: 169-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.kt` around lines 99 - 100, The code currently calls cleanupCommaArtifacts(result.toString()) on the entire assembled line in NullParamTrimmer, which can alter unrelated literals; instead, detect the exact substring boundaries corresponding to the removed parameter (use the placeholder start/end indices you already compute when you strip the null param), run cleanupCommaArtifacts only on that slice, and splice the cleaned slice back into the StringBuilder (or result) so only the neighborhood of the removed argument is normalized; apply the same localized-slice cleanup change to the other occurrence handled in the same class (the block around the lines referenced 169-177) so both code paths trim commas only within the removed-parameter region.website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt (1)
205-206:⚠️ Potential issue | 🟡 MinorThe Gradle docs still contradict each other about omitted
templateFile().The per-config example says leaving
templateFile()out inherits the common template, but the auto-discovery callout says omission disables discovery. Please make both sections describe the same Gradle behavior before publishing.Also applies to: 747-754
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt` around lines 205 - 206, Update the two inconsistent documentation passages in TemplateDocsContent.kt that mention templateFile(): ensure both the "per-config example" paragraph and the "auto-discovery callout" use the same Gradle behavior text — e.g., state that omitting templateFile() causes the config to inherit the common template (or, if you prefer the opposite behavior, state that omission disables discovery) — and make the wording identical for both occurrences of "templateFile()" so the docs are consistent.
🧹 Nitpick comments (9)
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.kt (1)
14-111: Consider consolidating discovery cases into Burst-parameterized tests and tightening one scenario.The discovery tests are a good matrix, but they’re repetitive. A parameterized Burst table would reduce duplication and make intent clearer. Also, the case on Lines 74-87 can pass even if traversal skips
/project/output; seed the template at/project/output/s2c.template.tomlto verify parent handling explicitly.As per coding guidelines:
**/*Test.kt: Create and update Kotlin unit tests following KMP testing conventions, including Burst parameterized patterns and targeted Gradle execution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.kt` around lines 14 - 111, Consolidate the repetitive discovery tests in TemplateConfigReaderTest into a single Burst-parameterized table-driven test that calls TemplateConfigReader.discover(...) with different starting paths, directory sets and file maps (use the existing fakeFileManager helper) to cover: found in output dir, walking up to parent, no template, treating file-path by using parent, and preferring closest template; specifically update the "discover treats file path by using its parent" scenario to place the template at "/project/output/s2c.template.toml" (instead of only "/project/s2c.template.toml") so the test verifies parent handling explicitly; refactor test functions (the current `@Test` methods in this file) into a Burst `@BurstTest` with a `@Table` or data provider and assert via reader.discover and config.templates/iconTemplate or config.definitions.imports as before.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt (1)
201-202: Angle bracket tracking may misinterpret comparison operators.The
<and>characters are tracked for generics, but in Kotlin code they also appear as comparison operators (e.g.,if (x < 10)). While this is unlikely to cause issues in function parameter contexts (the primary use case), be aware that expressions likelistOf(1, 2).filter { it > 0 }could confuse the tracker if they appear in resolved template values.Since this is used for reformatting multi-line calls where such expressions are uncommon, this is a minor concern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt` around lines 201 - 202, The angle bracket tracker in TemplateNodeEmitter (the when branch that updates angleDepth for '<' and '>') can mistake comparison operators for generics; update that logic so '<' and '>' only modify angleDepth when they appear to be part of generic/angle-bracket syntax (not binary comparison). Concretely, inspect surrounding characters (previous and next non-space chars) before changing angleDepth: treat '<' as an opening angle only if the following non-space char is an identifier/letter/digit/'(' or if the previous non-space char is an identifier/letter/digit/')' (similar heuristics), and treat '>' as a closing angle only when the previous non-space char looks like an identifier/letter/digit/')" or when the next non-space char starts a punctuation typical of generics; otherwise ignore them as comparison operators. Keep the change localized to the existing when branch that references angleDepth so behavior elsewhere is unchanged.website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt (2)
14-16: Add a defensive initialization check for runtime transformers.If
globalThis.shikiTransformersis unavailable, downstream calls fail with opaque JS errors. A guard here would fail fast with a clear message.Proposed patch
private val transformersModule: dynamic - get() = js("globalThis.shikiTransformers") + get() { + val module = js("globalThis.shikiTransformers") + check(module != null && module != js("undefined")) { + "Shiki transformers are not initialized. Call Shiki.initialize() before using ShikiTransformers." + } + return module + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt` around lines 14 - 16, The current getter for transformersModule (in ShikiTransformers.kt) directly returns js("globalThis.shikiTransformers") and can produce obscure JS errors when that global is missing; change the getter to read the global into a local (e.g., val value = js("globalThis.shikiTransformers")), check for null/undefined, and if absent throw a clear Kotlin/IllegalStateException (or IllegalArgumentException) with a descriptive message like "shikiTransformers runtime not found; ensure shiki transformers are loaded", otherwise return the value; this ensures downstream callers of transformersModule fail fast with a helpful error.
87-90: Handle missingchildrenwhen appending the info icon.If
node.childrenis undefined, the icon is silently dropped. Initializing the array keeps behavior stable across node shapes.Proposed patch
val children = node.children if (children != js("undefined")) { node.children = children.concat(arrayOf(iconNode)) + } else { + node.children = arrayOf(iconNode) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt` around lines 87 - 90, When appending the info icon the code currently drops the icon if node.children is undefined; change the block so that if children is undefined you initialize node.children to an array containing iconNode, otherwise concat as before. Update the snippet that references node.children and iconNode to: read children, if children == js("undefined") then set node.children = arrayOf(iconNode), else node.children = children.concat(arrayOf(iconNode)); this ensures the icon is always appended regardless of the original node shape..github/actions/template-integrity-check/script.sh (4)
107-112: Diff argument order is reversed from convention.The diff compares
$tmp_output(actual) against$expected_file(expected). The conventional order isdiff expected actual, so that additions represent new content and deletions represent removed content. The current order inverts this interpretation in the output.🔧 Suggested fix
echo "Verifying $stem (template) against expected file." - if ! $differ "$tmp_output" "$expected_file"; then + if ! $differ "$expected_file" "$tmp_output"; then errors+=("$stem.$ext") else echo "✅ $stem.$ext (template) pass"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/template-integrity-check/script.sh around lines 107 - 112, The diff invocation uses the wrong argument order: change the call to $differ so it compares expected then actual (swap "$tmp_output" and "$expected_file" to "$expected_file" "$tmp_output") in the if-statement that currently reads if ! $differ "$tmp_output" "$expected_file"; then so the diff output follows the conventional expected→actual interpretation (keep existing variable names like $differ, $tmp_output, $expected_file, and the errors+=("$stem.$ext") handling).
73-74: Consider handlingmktempfailure.If
mktemp -dfails,tmp_dirwill be empty and subsequent operations could write to unexpected locations or fail silently.🛡️ Suggested defensive check
tmp_dir="$(mktemp -d)" + if [ -z "$tmp_dir" ] || [ ! -d "$tmp_dir" ]; then + echo "Failed to create temporary directory for $stem." + errors+=("$stem.$ext") + return + fi tmp_output="${tmp_dir}/${icon_name}.kt"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/template-integrity-check/script.sh around lines 73 - 74, The script currently assigns tmp_dir="$(mktemp -d)" and then uses tmp_output="${tmp_dir}/${icon_name}.kt" without checking mktemp success; update the logic around tmp_dir (and tmp_output) to verify mktemp succeeded (e.g., test if tmp_dir is non-empty and is a directory or check mktemp exit status) and if it failed, emit a clear error (including the attempted icon_name) to stderr and exit non‑zero; also ensure any later cleanup logic (trap or rm) handles the case where tmp_dir was never created to avoid accidental deletions.
39-42: Minor: Inconsistent indentation in function body.The
to_pascal_casefunction has extra leading whitespace on lines 41-42 compared to the rest of the script.🔧 Suggested fix
# Convert kebab-case or snake_case filename to PascalCase. to_pascal_case() { - echo "$1" | awk -F'[-_]' '{ for(i=1;i<=NF;i++) $i=toupper(substr($i,1,1)) substr($i,2); print }' OFS='' - } + echo "$1" | awk -F'[-_]' '{ for(i=1;i<=NF;i++) $i=toupper(substr($i,1,1)) substr($i,2); print }' OFS='' +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/template-integrity-check/script.sh around lines 39 - 42, The to_pascal_case function body has inconsistent leading whitespace; remove the extra indentation before the awk command and the closing brace so the function lines align with the rest of the script (normalize indentation of the to_pascal_case() { ... } block), keeping the awk command unchanged and preserving OFS='' and the function name to_pascal_case for easy location.
70-71: Consider suppressing version output on failure.If
s2c --versionfails,s2c_versionwill be empty or contain an error message. This won't break the script but could produce confusing output. Low priority since version fetch failing likely indicates bigger issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/template-integrity-check/script.sh around lines 70 - 71, The script currently assigns s2c_version via s2c_version=$(command "$root_directory/s2c" --version) which can produce empty or noisy output on failure; update this to suppress stderr and provide a safe default so the echo remains meaningful: run the version command redirecting stderr to /dev/null and fall back to a default like "unknown" (or "version unavailable") when the command fails, ensuring variables referenced (s2c_version, root_directory, and the command invocation) still work if s2c --version errors.website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt (1)
831-840: Use row headers for the first column in these tables.You already mark column headers with
scope="col", but the first body cell in each row is still aTd. Rendering that cell asTh(scope="row")would improve screen-reader navigation for the namespace, variable, and field tables.Also applies to: 873-881, 918-929
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt` around lines 831 - 840, The first cell of each data row is currently rendered as a Td (see usages around TemplateTableCellStyle and InlineCode in TemplateDocsContent.kt); change those first-column cells to use Th with scope="row" (preserving the same attrs: TemplateTableCellStyle.toModifier().fontWeight(...).color(...).toAttrs()) so screen readers treat the namespace/variable/field as a row header—apply the same change to the other occurrences that render the first column (the blocks that currently create Td with InlineCode and the subsequent Td for the row body).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/cli-integrity-check/action.yml:
- Around line 14-18: The run lines interpolate inputs.type directly which can
cause shell injection; instead export the action input via env and call the
script with a quoted variable. Update the two steps that call
./.github/actions/cli-integrity-check/script.sh to set an environment variable
(e.g., EXTENSION: ${{ inputs.type }}) and invoke the script as
./.github/actions/cli-integrity-check/script.sh . --extension "$EXTENSION" (and
the optimized variant with --optimize), referencing the inputs via EXTENSION
rather than ${ inputs.type } in the run block.
In @.github/actions/cli-integrity-check/script.sh:
- Around line 20-23: The --optimize branch sets optimize="true" but never
updates the suffix variable, so later path construction that uses suffix (used
to locate expected fixtures like *.${suffix}.kt) still points to "nonoptimized";
update the suffix variable inside the --optimize) case (alongside setting
optimize="true") to "optimized" so functions that build expected file paths use
the correct suffix; search for the optimize flag handling (the --optimize) case
and the suffix variable and make them consistent.
- Around line 13-18: The script currently assigns and later possibly overrides
the ext variable via the --extension case but then silently maps non-'xml'
values to 'svg'; add a post-parsing validation step (after the flag-parsing
loop) that inspects ext and rejects anything other than "xml" or "svg" by
printing a clear error to stderr and exiting non-zero; if ext is empty or still
contains an unexpected token (e.g., a leftover flag name), treat that as invalid
and fail fast instead of falling back to "svg".
In `@docs/TEMPLATES.md`:
- Around line 305-311: The fenced code block showing the path(...) snippet needs
a language identifier for proper syntax highlighting: add "kotlin" immediately
after the opening triple backticks of the block that contains the path(...)
example (the snippet including fill = SolidColor(Color.Black), fillAlpha =
${path:fill_alpha}, pathFillType = EvenOdd), so the block becomes ```kotlin ...
``` to enable Kotlin highlighting.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/avg/AvgGradientNode.kt`:
- Around line 126-133: The current stops branch for shorthandColors.size == 2
always returns [0f, 1f], which drops the intended CENTER_COLOR_STOP_OFFSET when
the two colors are (startColor + centerColor) or (centerColor + endColor);
update the when branch in AvgGradientNode (the shorthandColors.size == 2 case)
to detect which colors are present and return [0f, CENTER_COLOR_STOP_OFFSET]
when startColor and centerColor are provided, [CENTER_COLOR_STOP_OFFSET, 1f]
when centerColor and endColor are provided, and only default to [0f, 1f] for the
pure startColor+endColor case (or alternatively add a comment documenting that
only start+end is expected). Ensure you reference the existing symbols stops,
shorthandColors, startColor, centerColor, endColor, and CENTER_COLOR_STOP_OFFSET
when making the change.
In
`@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.kt`:
- Line 120: The fake FileManager implementation's exists(path: Path) currently
returns true only when the exact path string is in files, violating
FileManager.exists which should return true for directories as well; update the
fake's exists method to return true if files contains the exact path OR if any
entry in files is a descendant of the path (e.g., files.any {
it.startsWith(path.toString().trimEnd('/') + '/') }), so directory paths are
treated as existing by the fake and tests mirror production discovery logic.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt`:
- Around line 157-163: The icon scope row currently only lists icon_template and
icon_builder but must include all sources that support icon placeholders
(notably file_header) so readers don’t think `${icon:name}` is out of scope;
update the scope row in TemplateDocsContent (the place that renders the icon
scope list) to add file_header (and any other template kinds that accept
`icon:`), and ensure examples/preview functions such as the
`${icon:name}Preview` and usage of `${icon:name}` in chunk-name examples remain
consistent with the expanded scope.
- Around line 705-722: The example contradicts the described elision rule by
annotating the elided parameter line inside the CodeBlock; remove the inline
annotation so the sample mirrors the rule. In TemplateDocsContent.kt update the
CodeBlock that uses ${path:fill_alpha} to delete the trailing comment ("// <-
this line is removed") from the fillAlpha line (and any other inline annotation
referencing elision), keeping the surrounding code and the InlineCode reference
(${path:fill_alpha}) unchanged so the prose and snippet match.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt`:
- Around line 34-40: The helper addClassToNode currently reads/writes
node.properties["class"] as a string and calls toString() which mangles HAST
class tokens; change it to use node.properties["className"] which is a string[]:
ensure node.properties exists, read node.properties["className"] into a mutable
list (safely casting/handling undefined), append the new className only if not
already present, and write it back as an array (e.g., toTypedArray()) to
node.properties["className"] so class tokens are preserved and deduplicated.
---
Duplicate comments:
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.kt`:
- Around line 99-100: The code currently calls
cleanupCommaArtifacts(result.toString()) on the entire assembled line in
NullParamTrimmer, which can alter unrelated literals; instead, detect the exact
substring boundaries corresponding to the removed parameter (use the placeholder
start/end indices you already compute when you strip the null param), run
cleanupCommaArtifacts only on that slice, and splice the cleaned slice back into
the StringBuilder (or result) so only the neighborhood of the removed argument
is normalized; apply the same localized-slice cleanup change to the other
occurrence handled in the same class (the block around the lines referenced
169-177) so both code paths trim commas only within the removed-parameter
region.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt`:
- Around line 148-151: The fallback currently calls
nodeEmitter.emitChunkFunctionDefinition inside chunkFunctions.joinToString when
templateEmitterConfig.fragments[Fragment.CHUNK_FUNCTION_DEFINITION] is missing,
which bypasses template-aware emission; change the fallback to generate the
chunk body via templateNodeEmitter (e.g., use templateNodeEmitter to emit the
chunk body or a template-aware chunk emitter) and then wrap that body with the
default chunk function signature so path/group fragments and color mappings from
templateNodeEmitter apply; update the code path where
Fragment.CHUNK_FUNCTION_DEFINITION is absent (the chunkFunctions.joinToString
branch and the call to nodeEmitter.emitChunkFunctionDefinition) to call the
template emitter for the body and concatenate the default function definition
around it.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt`:
- Around line 205-206: Update the two inconsistent documentation passages in
TemplateDocsContent.kt that mention templateFile(): ensure both the "per-config
example" paragraph and the "auto-discovery callout" use the same Gradle behavior
text — e.g., state that omitting templateFile() causes the config to inherit the
common template (or, if you prefer the opposite behavior, state that omission
disables discovery) — and make the wording identical for both occurrences of
"templateFile()" so the docs are consistent.
---
Nitpick comments:
In @.github/actions/template-integrity-check/script.sh:
- Around line 107-112: The diff invocation uses the wrong argument order: change
the call to $differ so it compares expected then actual (swap "$tmp_output" and
"$expected_file" to "$expected_file" "$tmp_output") in the if-statement that
currently reads if ! $differ "$tmp_output" "$expected_file"; then so the diff
output follows the conventional expected→actual interpretation (keep existing
variable names like $differ, $tmp_output, $expected_file, and the
errors+=("$stem.$ext") handling).
- Around line 73-74: The script currently assigns tmp_dir="$(mktemp -d)" and
then uses tmp_output="${tmp_dir}/${icon_name}.kt" without checking mktemp
success; update the logic around tmp_dir (and tmp_output) to verify mktemp
succeeded (e.g., test if tmp_dir is non-empty and is a directory or check mktemp
exit status) and if it failed, emit a clear error (including the attempted
icon_name) to stderr and exit non‑zero; also ensure any later cleanup logic
(trap or rm) handles the case where tmp_dir was never created to avoid
accidental deletions.
- Around line 39-42: The to_pascal_case function body has inconsistent leading
whitespace; remove the extra indentation before the awk command and the closing
brace so the function lines align with the rest of the script (normalize
indentation of the to_pascal_case() { ... } block), keeping the awk command
unchanged and preserving OFS='' and the function name to_pascal_case for easy
location.
- Around line 70-71: The script currently assigns s2c_version via
s2c_version=$(command "$root_directory/s2c" --version) which can produce empty
or noisy output on failure; update this to suppress stderr and provide a safe
default so the echo remains meaningful: run the version command redirecting
stderr to /dev/null and fall back to a default like "unknown" (or "version
unavailable") when the command fails, ensuring variables referenced
(s2c_version, root_directory, and the command invocation) still work if s2c
--version errors.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt`:
- Around line 201-202: The angle bracket tracker in TemplateNodeEmitter (the
when branch that updates angleDepth for '<' and '>') can mistake comparison
operators for generics; update that logic so '<' and '>' only modify angleDepth
when they appear to be part of generic/angle-bracket syntax (not binary
comparison). Concretely, inspect surrounding characters (previous and next
non-space chars) before changing angleDepth: treat '<' as an opening angle only
if the following non-space char is an identifier/letter/digit/'(' or if the
previous non-space char is an identifier/letter/digit/')' (similar heuristics),
and treat '>' as a closing angle only when the previous non-space char looks
like an identifier/letter/digit/')" or when the next non-space char starts a
punctuation typical of generics; otherwise ignore them as comparison operators.
Keep the change localized to the existing when branch that references angleDepth
so behavior elsewhere is unchanged.
In
`@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.kt`:
- Around line 14-111: Consolidate the repetitive discovery tests in
TemplateConfigReaderTest into a single Burst-parameterized table-driven test
that calls TemplateConfigReader.discover(...) with different starting paths,
directory sets and file maps (use the existing fakeFileManager helper) to cover:
found in output dir, walking up to parent, no template, treating file-path by
using parent, and preferring closest template; specifically update the "discover
treats file path by using its parent" scenario to place the template at
"/project/output/s2c.template.toml" (instead of only
"/project/s2c.template.toml") so the test verifies parent handling explicitly;
refactor test functions (the current `@Test` methods in this file) into a Burst
`@BurstTest` with a `@Table` or data provider and assert via reader.discover and
config.templates/iconTemplate or config.definitions.imports as before.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt`:
- Around line 831-840: The first cell of each data row is currently rendered as
a Td (see usages around TemplateTableCellStyle and InlineCode in
TemplateDocsContent.kt); change those first-column cells to use Th with
scope="row" (preserving the same attrs:
TemplateTableCellStyle.toModifier().fontWeight(...).color(...).toAttrs()) so
screen readers treat the namespace/variable/field as a row header—apply the same
change to the other occurrences that render the first column (the blocks that
currently create Td with InlineCode and the subsequent Td for the row body).
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt`:
- Around line 14-16: The current getter for transformersModule (in
ShikiTransformers.kt) directly returns js("globalThis.shikiTransformers") and
can produce obscure JS errors when that global is missing; change the getter to
read the global into a local (e.g., val value =
js("globalThis.shikiTransformers")), check for null/undefined, and if absent
throw a clear Kotlin/IllegalStateException (or IllegalArgumentException) with a
descriptive message like "shikiTransformers runtime not found; ensure shiki
transformers are loaded", otherwise return the value; this ensures downstream
callers of transformersModule fail fast with a helpful error.
- Around line 87-90: When appending the info icon the code currently drops the
icon if node.children is undefined; change the block so that if children is
undefined you initialize node.children to an array containing iconNode,
otherwise concat as before. Update the snippet that references node.children and
iconNode to: read children, if children == js("undefined") then set
node.children = arrayOf(iconNode), else node.children =
children.concat(arrayOf(iconNode)); this ensures the icon is always appended
regardless of the original node shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72ee5f78-0840-45b2-9ce2-b71f0990811b
⛔ Files ignored due to path filters (46)
config/detekt.ymlis excluded by!**/config/detekt.ymlintegrity-check/expected/template/Android.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/Android.xml.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/AttentionFilled.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/Brasil.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/LinearGradient01.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/MaskWithGroup.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/ShieldHalvedSolid.xml.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/StrokeGradient.xml.template.ktis excluded by!**/integrity-check/expected/**kotlin-js-store/yarn.lockis excluded by!**/yarn.lock,!**/*.lockplayground/.gitignoreis excluded by!**/playground/**playground/app/build.gradle.ktsis excluded by!**/playground/**playground/app/samples/template/attention-filled.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/samples/template/brasil.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/samples/template/shield-halved-solid.xmlis excluded by!**/samples/**,!**/playground/**playground/app/samples/template/smiley.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/Colors.ktis excluded by!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/IconBuilders.ktis excluded by!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/Icons.ktis excluded by!**/playground/**playground/s2c.template.tomlis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/AutocompleteDropdown.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/ScrollSyncEffect.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/TomlTooltip.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/BatchPanel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/BatchPhaseHeader.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/InputPanel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/PlaygroundSection.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/PlaygroundToolbar.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/TemplateEditorHelpers.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/TemplateEditorSection.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/template/TemplateEditorSchema.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TemplateValidationError.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TomlKeyInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TomlSectionInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TooltipInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/ConversionInputFactory.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundAction.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundOptions.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundPersistence.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundState.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundViewModel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplatePersistenceHelper.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplateTomlExtensions.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/reducer/PlaygroundReducer.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/reducer/TemplateReducer.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/validator/playground/template/TemplateTomlValidator.ktis excluded by!**/playground/**
📒 Files selected for processing (74)
.github/actions/cli-integrity-check/action.yml.github/actions/cli-integrity-check/script.sh.github/actions/template-integrity-check/action.yml.github/actions/template-integrity-check/script.sh.github/workflows/pull_request.ymlapp.propertiesbuildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.ktdocs/ARCHITECTURE.mddocs/TEMPLATES.mdgradle/libs.playground-kmp.versions.tomlgradle/libs.playground.versions.tomlgradle/libs.versions.tomlgradle/libs.website.versions.tomlsvg-to-compose-gradle-plugin/README.mdsvg-to-compose-gradle-plugin/build.gradle.ktssvg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/dsl/parser/IconParserConfiguration.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/ParseSvgToComposeIconTask.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingParameters.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingWorkAction.ktsvg-to-compose/README.mdsvg-to-compose/build.gradle.ktssvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/Converter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/avg/AvgGradientNode.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/CodeEmitterFactory.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/NodeChunker.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorNodeEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReader.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateToken.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolver.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/ResolvedToken.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/error/ErrorCode.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ParserConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/config/TemplateConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/method/MethodSizeAccountable.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/avg/AvgGradientNodeTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/NodeChunkerTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitterTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitterTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfigParserTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexerTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolverTest.ktwebsite/build.gradle.ktswebsite/editor-wasm/build.gradle.ktswebsite/site/build.gradle.ktswebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/layouts/DocsLayout.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/DocNavDropdown.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/CapabilitiesSection.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/NavHeader.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliOption.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Index.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Templates.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/Shiki.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiApi.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/theme/SiteDimensions.ktwebsite/worker/build.gradle.ktswebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/ConversionTypes.ktwebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/IconConvertWorkerFactory.ktwebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/inject/WorkerGraph.kt
✅ Files skipped from review due to trivial changes (25)
- website/site/build.gradle.kts
- website/worker/build.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/error/ErrorCode.kt
- app.properties
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/NavHeader.kt
- website/editor-wasm/build.gradle.kts
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/avg/AvgGradientNodeTest.kt
- website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/inject/WorkerGraph.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/Shiki.kt
- svg-to-compose-gradle-plugin/build.gradle.kts
- .github/actions/template-integrity-check/action.yml
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliOption.kt
- website/build.gradle.kts
- svg-to-compose-gradle-plugin/README.md
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateToken.kt
- svg-to-compose/build.gradle.kts
- website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/ConversionTypes.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/config/TemplateConfig.kt
- gradle/libs.playground.versions.toml
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.kt
- .github/workflows/pull_request.yml
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfigParserTest.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitterTest.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfig.kt
🚧 Files skipped from review as they are similar to previous changes (23)
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/CapabilitiesSection.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/method/MethodSizeAccountable.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ParserConfig.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/layouts/DocsLayout.kt
- buildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.kt
- website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/IconConvertWorkerFactory.kt
- svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/dsl/parser/IconParserConfiguration.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiApi.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Templates.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/theme/SiteDimensions.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliDocsContent.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/CodeEmitterFactory.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReader.kt
- svg-to-compose/README.md
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexerTest.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorNodeEmitter.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/NodeChunkerTest.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/Converter.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/ResolvedToken.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolver.kt
Comment on lines
+14
to
+18
| ./.github/actions/cli-integrity-check/script.sh . --extension ${{ inputs.type }} | ||
| - name: Verify ${{ inputs.type }} conversion with optimization | ||
| shell: bash | ||
| run: | | ||
| ./.github/actions/cli-integrity-check/script.sh . ${{ inputs.type }} optimize | ||
| ./.github/actions/cli-integrity-check/script.sh . --extension ${{ inputs.type }} --optimize |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/actions/cli-integrity-check/action.ymlRepository: rafaeltonholo/svg-to-compose
Length of output: 806
🏁 Script executed:
# Check if there are any input constraints or validation for the type input
rg "inputs:" -A 20 .github/actions/cli-integrity-check/action.ymlRepository: rafaeltonholo/svg-to-compose
Length of output: 581
🏁 Script executed:
# Find workflow files that use this action
rg "cli-integrity-check" --type yaml -B 5 -A 5 .github/workflows/Repository: rafaeltonholo/svg-to-compose
Length of output: 3235
🏁 Script executed:
# Also search for any uses of this action in test or other files
rg "cli-integrity-check" -g "*.yml" -g "*.yaml"Repository: rafaeltonholo/svg-to-compose
Length of output: 54
Pass the action input through env instead of interpolating it directly into run.
GitHub resolves ${{ ... }} before Bash parses these lines. Without input validation, if type ever contains shell syntax (;, $(), backticks), the command can be re-tokenized or execute unintended code. Export it via env and pass "$EXTENSION" to the script instead. This is especially important since this is a reusable action that could be invoked with dynamic input in the future.
Suggested fix
- name: Verify ${{ inputs.type }} conversion without optimization
shell: bash
+ env:
+ EXTENSION: ${{ inputs.type }}
run: |
- ./.github/actions/cli-integrity-check/script.sh . --extension ${{ inputs.type }}
+ ./.github/actions/cli-integrity-check/script.sh . --extension "$EXTENSION"
- name: Verify ${{ inputs.type }} conversion with optimization
shell: bash
+ env:
+ EXTENSION: ${{ inputs.type }}
run: |
- ./.github/actions/cli-integrity-check/script.sh . --extension ${{ inputs.type }} --optimize
+ ./.github/actions/cli-integrity-check/script.sh . --extension "$EXTENSION" --optimize📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ./.github/actions/cli-integrity-check/script.sh . --extension ${{ inputs.type }} | |
| - name: Verify ${{ inputs.type }} conversion with optimization | |
| shell: bash | |
| run: | | |
| ./.github/actions/cli-integrity-check/script.sh . ${{ inputs.type }} optimize | |
| ./.github/actions/cli-integrity-check/script.sh . --extension ${{ inputs.type }} --optimize | |
| - name: Verify ${{ inputs.type }} conversion without optimization | |
| shell: bash | |
| env: | |
| EXTENSION: ${{ inputs.type }} | |
| run: | | |
| ./.github/actions/cli-integrity-check/script.sh . --extension "$EXTENSION" | |
| - name: Verify ${{ inputs.type }} conversion with optimization | |
| shell: bash | |
| env: | |
| EXTENSION: ${{ inputs.type }} | |
| run: | | |
| ./.github/actions/cli-integrity-check/script.sh . --extension "$EXTENSION" --optimize |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/cli-integrity-check/action.yml around lines 14 - 18, The run
lines interpolate inputs.type directly which can cause shell injection; instead
export the action input via env and call the script with a quoted variable.
Update the two steps that call ./.github/actions/cli-integrity-check/script.sh
to set an environment variable (e.g., EXTENSION: ${{ inputs.type }}) and invoke
the script as ./.github/actions/cli-integrity-check/script.sh . --extension
"$EXTENSION" (and the optimized variant with --optimize), referencing the inputs
via EXTENSION rather than ${ inputs.type } in the run block.
Comment on lines
+13
to
+18
| shift # consume root_directory ($1), leaving only flags | ||
| while [ $# -gt 0 ]; do | ||
| case "$1" in | ||
| --extension) | ||
| ext="${2:?Error: --extension flag requires a value}" | ||
| shift 2 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/actions/cli-integrity-check/script.sh | head -80Repository: rafaeltonholo/svg-to-compose
Length of output: 2708
🏁 Script executed:
rg -r "cli-integrity-check" --type yml --type yaml -A 3 | head -40Repository: rafaeltonholo/svg-to-compose
Length of output: 468
🏁 Script executed:
rg "cli-integrity-check" -t yml -t yaml -A 3 2>/dev/null | head -60Repository: rafaeltonholo/svg-to-compose
Length of output: 54
🏁 Script executed:
find .github -name "*.yml" -o -name "*.yaml" | xargs grep -l "cli-integrity-check" 2>/dev/nullRepository: rafaeltonholo/svg-to-compose
Length of output: 216
🏁 Script executed:
cat .github/actions/cli-integrity-check/action.ymlRepository: rafaeltonholo/svg-to-compose
Length of output: 680
🏁 Script executed:
grep -r "script.sh" .github/workflows/ --include="*.yml" --include="*.yaml" -B 2 -A 2Repository: rafaeltonholo/svg-to-compose
Length of output: 54
🏁 Script executed:
grep -r "cli-integrity-check" .github/workflows/ --include="*.yml" -A 2 -B 2Repository: rafaeltonholo/svg-to-compose
Length of output: 1946
Add validation to reject invalid extensions after flag parsing.
The script initializes ext from the second positional parameter (line 4), then the flag loop may override it with --extension. However, lines 40-45 silently convert any extension other than xml to svg. If the --extension flag is omitted during direct script invocation, ext retains an invalid value (e.g., a flag name) and is silently converted to svg instead of failing fast.
While the GitHub Actions workflow always provides explicit --extension values, adding post-parsing validation ensures the script safely rejects invalid extensions:
Suggested fix
shift # consume root_directory ($1), leaving only flags
while [ $# -gt 0 ]; do
case "$1" in
--extension)
ext="${2:?Error: --extension flag requires a value}"
shift 2
@@
esac
done
+
+case "$ext" in
+ svg|xml) ;;
+ *)
+ echo "Error: --extension must be either 'svg' or 'xml'" >&2
+ exit 2
+ ;;
+esac🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/cli-integrity-check/script.sh around lines 13 - 18, The
script currently assigns and later possibly overrides the ext variable via the
--extension case but then silently maps non-'xml' values to 'svg'; add a
post-parsing validation step (after the flag-parsing loop) that inspects ext and
rejects anything other than "xml" or "svg" by printing a clear error to stderr
and exiting non-zero; if ext is empty or still contains an unexpected token
(e.g., a leftover flag name), treat that as invalid and fail fast instead of
falling back to "svg".
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/avg/AvgGradientNode.kt
Show resolved
Hide resolved
| override fun createDirectory(dir: Path) = Unit | ||
| override fun createDirectories(dir: Path, mustCreate: Boolean) = Unit | ||
|
|
||
| override fun exists(path: Path): Boolean = path.toString() in files |
Contributor
There was a problem hiding this comment.
exists in the fake file manager breaks the interface contract.
On Line 120, exists returns true only for files, but FileManager.exists is defined for any existing filesystem object (including directories). This can make test behavior diverge from production discovery logic.
Suggested fix
- override fun exists(path: Path): Boolean = path.toString() in files
+ override fun exists(path: Path): Boolean {
+ val value = path.toString()
+ return value in files || value in directories
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.kt`
at line 120, The fake FileManager implementation's exists(path: Path) currently
returns true only when the exact path string is in files, violating
FileManager.exists which should return true for directories as well; update the
fake's exists method to return true if files contains the exact path OR if any
entry in files is a descendant of the path (e.g., files.any {
it.startsWith(path.toString().trimEnd('/') + '/') }), so directory paths are
treated as existing by the fake and tests mirror production discovery logic.
...e/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt
Show resolved
Hide resolved
Comment on lines
+705
to
+722
| text = "When a variable resolves to null, the entire line containing only that parameter " + | ||
| "assignment is elided from the output. This keeps generated code clean without " + | ||
| "requiring conditional logic in your template.", | ||
| modifier = DocsBodyTextStyle.toModifier(), | ||
| ) | ||
| Span(attrs = DocsBodyTextStyle.toAttrs()) { | ||
| Text("For example, if ") | ||
| InlineCode($$"${path:fill_alpha}") | ||
| Text(" is null:") | ||
| } | ||
| CodeBlock( | ||
| code = $$""" | ||
| |path( | ||
| | fill = SolidColor(Color.Black), | ||
| | fillAlpha = ${path:fill_alpha}, // <- this line is removed | ||
| | pathFillType = EvenOdd, | ||
| |) | ||
| """.trimMargin(), |
Contributor
There was a problem hiding this comment.
Don’t annotate the elided line inline in the code sample.
The prose says elision applies when the line contains only that parameter assignment, but the example adds a trailing comment to that exact line. That makes the copy-paste snippet stop matching the rule you’re describing.
✏️ Suggested tweak
- | fillAlpha = ${path:fill_alpha}, // <- this line is removed
+ | fillAlpha = ${path:fill_alpha},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt`
around lines 705 - 722, The example contradicts the described elision rule by
annotating the elided parameter line inside the CodeBlock; remove the inline
annotation so the sample mirrors the rule. In TemplateDocsContent.kt update the
CodeBlock that uses ${path:fill_alpha} to delete the trailing comment ("// <-
this line is removed") from the fillAlpha line (and any other inline annotation
referencing elision), keeping the surrounding code and the InlineCode reference
(${path:fill_alpha}) unchanged so the prose and snippet match.
Comment on lines
+34
to
+40
| private fun addClassToNode(node: dynamic, className: String) { | ||
| if (node.properties == null || node.properties == js("undefined")) { | ||
| node.properties = js("({})") | ||
| } | ||
| val existing: String? = node.properties["class"]?.toString() | ||
| node.properties["class"] = if (existing.isNullOrEmpty()) className else "$existing $className" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For @shikijs/transformersv4 (line transformer hook), what is the expected HAST class property shape on nodes:properties.class, properties.className, string, or string[]?
💡 Result:
In @shikijs/transformers v4 transformer hooks, the nodes you receive are HAST Element nodes, where classes live on:
node.properties.className- Expected shape:
string[](array of class tokens)
This follows the HAST convention that the HTML class attribute is represented as the className property (and HAST properties values can be arrays). (app.unpkg.com)
Citations:
Fix class property name and handle string[] type correctly.
The code uses node.properties["class"] but HAST Element nodes store classes on node.properties.className as a string[] array, not a string. On line 38, calling toString() on an array will produce "foo,bar" instead of properly handling class tokens. Refactor to read/write className as an array and append values appropriately:
private fun addClassToNode(node: dynamic, className: String) {
if (node.properties == null || node.properties == js("undefined")) {
node.properties = js("({})")
}
val existing = (node.properties["className"] as? Array<String>)?.toMutableList() ?: mutableListOf()
if (!existing.contains(className)) {
existing.add(className)
}
node.properties["className"] = existing.toTypedArray()
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.kt`
around lines 34 - 40, The helper addClassToNode currently reads/writes
node.properties["class"] as a string and calls toString() which mangles HAST
class tokens; change it to use node.properties["className"] which is a string[]:
ensure node.properties exists, read node.properties["className"] into a mutable
list (safely casting/handling undefined), append the new className only if not
already present, and write it back as an array (e.g., toTypedArray()) to
node.properties["className"] so class tokens are preserved and deduplicated.
…ate script - Invoke template-script.sh via `bash` for portability when executable permissions aren't preserved. - Use `-eq` instead of `==` for numeric array-length comparison.
…n lexer
When tryParsePlaceholder returns null for a `${...}` sequence, consume the entire span up to the closing `}` as literal text instead of just the `$` character. This prevents inner content from being re-interpreted as nested placeholders.
- When [templates.preview] exists but the template key is null (e.g. empty section), fall through to default preview behavior instead of suppressing preview generation. - Refactor buildPreview to use a when expression.
…nk functions - Extract emitGroupHeader from ImageVectorNodeEmitter so the fallback path in TemplateNodeEmitter still recurses children template-aware. - Make splitTopLevelParams and findMatchingCloseParen respect string literals, angle brackets, braces, and brackets via a shared NestingTracker.
Add config.templates.fileHeader to allTemplateTexts so that ${def:...} references in file headers are validated against defined import keys.
…ment - Align docs, website, and playground on consistent templateFile() behavior: omitting it in Gradle disables auto-discovery; use common config for inheritance. - Fix method signature to templateFile(path: RegularFile). Correct chunk:index comment from 0-based to 1-based.
…ty scripts Refactor both integrity check scripts to parse flags via a while/case loop instead of positional arguments. Add --default-differ flag to skip delta and use plain diff for machine-readable output.
… handling in group parameters
…andling in SVG templates
Replaces RichSpanText with a more descriptive name. Renders text that may contain inline code segments using backtick syntax. Adds typed CssStyleVariant support with per-CalloutVariant variants (Tip, Warning, Important) for proper InlineCode colour theming inside callouts.
… English
Replace Span { Text() InlineCode() } patterns across all five docs
files with CodeAwareSpanText using backtick syntax. Callout blocks
now use typed variants instead of manually resolving InlineCodeVars.
Fix US English spellings in copy text: color to colour in section
titles, descriptions, and variable labels. Code identifiers (Color,
SolidColor, color_mapping) are left unchanged.
Add KDoc to WebRoute class, its properties, and companion entries. Add KDoc to CodeAwareSpanText composable, ComponentKind marker, and all three callout style variants.
… backtick The previous logic dropped leading empty parts and reset the index, which inverted the even/odd check and rendered code as text and vice versa. Now iterates all parts using the original split index and skips empty segments instead.
…and tests Update ToC entry in Templates.kt and test names in PlaceholderResolverTest.kt to use "trim" instead of "elide".
…SpanText Convert TemplateSystemCallout in CliDocsContent and --max-workers Li in GradlePluginDocsContent to use CodeAwareSpanText with backtick syntax. Remove now-unused resolveInlineCodeColors from DocsStyles.
…ion" Replace all US English "customize/customization/customizing" with "customise/customisation/customising" in website copy across docs, playground, and landing page.
… filename casing in documentation
rafaeltonholo
force-pushed
the
feat/template-system
branch
from
84b34e6 to
f5272c0
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt (3)
573-574:⚠️ Potential issue | 🟡 MinorExpand the
iconnamespace scope list to match the rest of the page.At Line 573, the scope is too narrow and conflicts with other sections that show
icon:placeholders in additional template contexts (for example file header and preview docs).✏️ Suggested update
- scope = "`icon_template`, `icon_builder`", + scope = "`file_header`, `icon_template`, `templates.preview.template`, `icon_builder`, `chunk_function_name`",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt` around lines 573 - 574, The scope list for the icon namespace is too narrow—update the assignment where scope is set (currently scope = "`icon_template`, `icon_builder`") to include the additional icon contexts used elsewhere on the page (add the file header and preview template scopes, e.g. include `icon_file_header` and `icon_preview` alongside `icon_template` and `icon_builder`) so the icon placeholders shown in file header and preview docs are covered by the same namespace.
755-756:⚠️ Potential issue | 🟡 MinorResolve conflicting Gradle
templateFile()behavior guidance.Line 209 says omitting
templateFile()inherits common config, but Lines 755-756 say omission disables auto-discovery. These instructions conflict and should be unified.✏️ Suggested update (align with the example at Line 209)
- text = "Disable auto-discovery with `--no-template` (CLI) or by omitting " + - "`templateFile()` from the Gradle DSL.", + text = "Disable auto-discovery with `--no-template` (CLI). In Gradle, omitting " + + "`templateFile()` falls back to common config / auto-discovery.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt` around lines 755 - 756, Update the conflicting guidance in TemplateDocsContent by changing the text assignment that currently states "Disable auto-discovery with `--no-template` (CLI) or by omitting `templateFile()` from the Gradle DSL." to match the earlier example (line ~209) that omitting `templateFile()` causes the module to inherit the common config rather than disabling auto-discovery; keep the CLI note about `--no-template` if accurate, but reword the Gradle part to indicate omission = inherit common config (or explicitly call out how to disable if different), so the message in the text property and the earlier documentation are consistent.
734-734:⚠️ Potential issue | 🟡 MinorRemove inline annotation from the elision example line.
Line 734 contradicts the stated rule by keeping commentary on the line that is described as removed.
✏️ Suggested update
-| fillAlpha = ${path:fill_alpha}, // <- this line is removed +| fillAlpha = ${path:fill_alpha},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt` at line 734, In TemplateDocsContent (the elision example block) remove the inline annotation comment on the line containing "fillAlpha = ${path:fill_alpha}," so the line no longer includes the trailing comment marker (e.g., "// <- this line is removed"); leave the code snippet content consistent with the stated rule by deleting only the inline commentary and not the code line itself.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.kt (1)
169-176:⚠️ Potential issue | 🟠 MajorKeep comma cleanup local to the removed null parameter.
cleanupCommaArtifacts()still rewrites every"(,",",)", and",,"sequence in the assembled line, so unrelated literals or comments can be mutated whenever a null placeholder is trimmed. Limit the normalization to the slice around the removed placeholder instead of running global replacements over the whole line.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.kt` around lines 169 - 176, The cleanupCommaArtifacts function currently performs global replacements on the whole line (in cleanupCommaArtifacts) which can mutate unrelated text; change it to only normalize the region around the removed null parameter: modify the caller (e.g., wherever trimNullParam/removeNullPlaceholder is implemented) to pass the substring/range surrounding the removed placeholder to a new localized cleanup function (or extend cleanupCommaArtifacts to accept a start/end index), perform the three replacement rules only on that slice, then reassemble the full line by concatenating prefix + cleanedSlice + suffix; keep the replacement rules (LEADING_COMMA, TRAILING_COMMA, DOUBLE_COMMA) but apply them only to the slice to avoid touching other literals or comments.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt (1)
148-172:⚠️ Potential issue | 🟠 MajorChunked icons still bypass template-aware emission and import merging.
The no-fragment branch still calls
nodeEmitter.emitChunkFunctionDefinition(...), so once chunking kicks in large icons losepath_builder/group_builder/ color-mapping customizations. In the custom-fragment branch, imports added while resolving${def:*}insidechunk_function_definitionstay on the temporarycontextand never get merged back intoparentContext, so the emitted file can miss required imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt` around lines 148 - 172, The chunked-icon path currently bypasses template-aware emission and import merging: replace the direct call to nodeEmitter.emitChunkFunctionDefinition(...) in the no-fragment branch so chunk functions are emitted via the same template flow (use templateNodeEmitter.emit with a TemplateContext) and ensure after resolving the defFragment (via PlaceholderResolver.resolve) you always merge chunkNodeContext.collectedImports into parentContext by calling parentContext.addImports(chunkNodeContext.collectedImports); in other words, remove the special-case emitChunkFunctionDefinition path and make both branches build a chunkNodeContext, emit with templateNodeEmitter/PlaceholderResolver.resolve using the defFragment or a generated default, then add collected imports from chunkNodeContext into parentContext so path_builder/group_builder/color mappings and imports are preserved.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.kt (1)
69-113:⚠️ Potential issue | 🟠 MajorNormalize the resolved receiver once and reuse it everywhere.
iconPropertyNamestrips a trailing.and falls back toconfig.definitions.receiver, butIconVar.RECEIVERstill gets the rawcontents.receiverTypeand the receiver import is always added from the template definition. An explicit empty/Icons.receiver can therefore make${icon:receiver},${icon:property_name}, and the collected imports disagree.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.kt` around lines 69 - 113, The forIcon function should normalize the resolved receiver once and reuse it for iconPropertyName, IconVar.RECEIVER, and import handling: compute a single resolvedReceiver (strip trailing '.' from contents.receiverType if present, otherwise use config.definitions.receiver?.name or empty/Icons marker), use that resolvedReceiver when building IconVar.RECEIVER and when constructing iconPropertyName, and only call ctx.addImport(...) from the TemplateContext.also block if the receiver actually comes from config.definitions (i.e., ensure you reference the definition's receiver.packageName/receiver.name rather than always adding an import for the resolved string); update the logic in forIcon to derive and reuse these values consistently.
🧹 Nitpick comments (6)
.github/actions/cli-integrity-check/script.sh (1)
132-132: Consider using numeric comparison for array length.While string comparison
==works here,-eqis more idiomatic for numeric comparisons in bash and signals intent more clearly.Proposed fix
-if [ "${`#errors`[@]}" == 0 ]; then +if [ "${`#errors`[@]}" -eq 0 ]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/cli-integrity-check/script.sh at line 132, The conditional uses string equality for a numeric check: change the test in the if that inspects the errors array length (the expression "${`#errors`[@]}") to use numeric comparison (-eq) instead of == so the if [ "${`#errors`[@]}" -eq 0 ]; reads as an intented numeric comparison; update the condition where errors array length is checked to use -eq.website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/navigation/WebRoute.kt (1)
10-14: Consider makingalla lazy property to avoid repeated Set creation.The
allproperty creates a new Set on every access. For navigation routes accessed frequently, this could be optimized withlazy.💡 Optional optimization
data class WebRoute(val path: String, val label: String, val subRoutes: Set<WebRoute> = emptySet()) { /** * All routes in this hierarchy, including this route and its [subRoutes]. */ - val all = setOf(this) + subRoutes + val all: Set<WebRoute> by lazy { setOf(this) + subRoutes }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/navigation/WebRoute.kt` around lines 10 - 14, The `all` property on data class WebRoute constructs a new Set each time it's accessed; change it to a cached lazy property to avoid repeated allocations by replacing the current val all = setOf(this) + subRoutes with a lazy-initialized property (e.g. val all by lazy { setOf(this) + subRoutes }) so the set is created once per instance; locate WebRoute and update the all property accordingly.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/NodeChunker.kt (2)
55-70: Consider usingmapIndexedfor cleaner indexing.Using a mutable
var ithat's incremented inside thechunkedlambda works but is less idiomatic than usingmapIndexed. This is a minor style suggestion.♻️ Suggested refactor
- var i = 1 val chunks = ceil(byteSize.toFloat() / MethodSizeAccountable.METHOD_SIZE_THRESHOLD) .roundToInt() val chunkSize = max(1, contents.nodes.size / chunks) logger.warn( "Potential large icon detected. Splitting icon's content in $chunks chunks to avoid " + "compilation issues. However, that won't affect the performance of displaying this icon.", ) - val chunkFunctions = contents.nodes.chunked(chunkSize) { chunk -> + val chunkFunctions = contents.nodes.chunked(chunkSize).mapIndexed { index, chunk -> ImageVectorNode.ChunkFunction( - functionName = nameResolver(contents, i++), - nodes = chunk.toList(), + functionName = nameResolver(contents, index + 1), + nodes = chunk, ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/NodeChunker.kt` around lines 55 - 70, The code uses a mutable counter var i incremented inside the chunked lambda to build ImageVectorNode.ChunkFunction(functionName = nameResolver(contents, i++), ...); replace this pattern with a functional approach by using an indexed mapping (e.g., chunked(...).mapIndexed) so you can remove var i and call nameResolver(contents, index + 1) (or appropriate offset) inside mapIndexed; update the expression that builds chunkFunctions from contents.nodes.chunked(chunkSize) { ... } to a chunked(...).mapIndexed { index, chunk -> ImageVectorNode.ChunkFunction(functionName = nameResolver(contents, index + 1), nodes = chunk.toList()) } to make the code idiomatic and thread-safe.
43-73: Verify behavior when all nodes exceed threshold individually.The chunk size calculation
max(1, contents.nodes.size / chunks)ensures at least 1 node per chunk. However, if a single node'sapproximateByteSizeexceedsMETHOD_SIZE_THRESHOLD, it will still be placed in its own chunk but the warning message may be misleading about the effectiveness of chunking. This is an edge case worth documenting or handling explicitly.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReader.kt (1)
26-29: Consider wrappingreadContentin error handling.The
resolvemethod directly callsfileManager.readContent()which can throwIOExceptionif the file doesn't exist or is inaccessible. While callers may handle this, documenting the throwing behavior in the KDoc would improve API clarity.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReader.kt` around lines 26 - 29, Update the resolve function to make its error behavior explicit: either add KDoc above resolve including an `@throws` (or `@exception`) entry noting that fileManager.readContent(templatePath) can throw IOException, or wrap the readContent call in a try/catch that catches IOException and rethrows a more specific exception (or a RuntimeException) with contextual message (including templatePath) before returning TemplateConfigParser.parse(content); reference resolve, fileManager.readContent, TemplateConfigParser.parse and TemplateEmitterConfig when making the change.svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt (1)
230-252: Please add a configuration-cache case fortemplateFile(...).The existing cache coverage only exercises the non-template path. A functional test that runs these template-based builds twice with
--configuration-cachewould protect the new DSL from eager file/provider resolution regressions.As per coding guidelines,
svg-to-compose-gradle-plugin/**/*.kt: Gradle plugin development for the svg-to-compose-gradle-plugin module must use lazy Provider-based configuration, typed DSL/extensions, and TestKit coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt` around lines 230 - 252, Add a configuration-cache functional test that exercises the template path by running the template-based builds twice with Gradle's "--configuration-cache" flag; specifically, create a test (next to `svg with template matches expected output` / `avg with template matches expected output`) that sets up the same project (uses `buildTemplateGradleContent`, `copyTemplateIconsToProject`, `copyTemplateConfigToProject`) then calls `runGradle("parseSvgToComposeIcon", "--configuration-cache")` twice (second run should also pass the same flag) and asserts success with `assertTaskSuccess` and output validity with `assertTemplateOutputMatchesExpected` (use `fileType = "svg"` and `"xml"` respectively) to ensure `templateFile(...)` is safe for the configuration cache.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/cli-integrity-check/script.sh:
- Line 13: The script mistakenly performs only one shift after consuming
root_directory, leaving the extension value (variable ext assigned from $2) to
become $1 and be treated as a flag by the while-loop; fix by performing a second
shift (i.e., shift again after assigning ext) so that ext is removed from
positional parameters before entering the flag-parsing while loop (reference
variable ext and the existing shift in the script.sh).
In
`@svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt`:
- Around line 99-101: Replace the external repo dependency in
copyTemplateConfigToProject: add a dedicated s2c.template.toml fixture under the
plugin test resources (per guidelines put fixtures in src/test/resources/), then
change copyTemplateConfigToProject() to load that resource via the test class
loader (e.g., javaClass.classLoader.getResourceAsStream("s2c.template.toml"))
and copy the stream into projectDir.resolve("s2c.template.toml") instead of
reading from repoRoot; this keeps the fixture versioned with the functional test
and removes the cross-module dependency.
- Around line 161-165: The test currently auto-bootstraps missing golden files
by copying generatedFile to expectedFile when expectedFile.exists() is false;
change this so missing expectedFile causes the test to fail instead of mutating
templateExpectedDir. Remove or disable the copyTo and println in the if
(!expectedFile.exists()) branch and replace it with a clear test failure (e.g.,
throw an assertion or fail) that mentions expectedFileName and the missing
expectedFile path so CI/maintainers must add the reviewed golden into
integrity-check/expected before the test can pass; keep the rest of the
comparison logic unchanged.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt`:
- Around line 124-129: The default preview fallback in
TemplateEmitter.buildDefaultPreview calls
fallbackEmitter.buildPreviewSnippet(contents) which lacks TemplateContext, so
fallback previews cannot use template-resolved property/receiver names and
produce invalid references (e.g., Icons.TestIcon). Modify the fallback API and
call so the fallback emitter receives the TemplateContext: update
fallbackEmitter.buildPreviewSnippet to accept (contents, context) (or add an
overload) and change TemplateEmitter.buildDefaultPreview to pass the context
through; ensure the fallback implementation uses context's resolved property
names/receiver info and still lets TemplateEmitter register PREVIEW_IMPORTS via
context.addImports.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt`:
- Around line 77-85: The current guard in TemplateNodeEmitter (variables
hasClipPath and fragmentReferencesClipPath) incorrectly searches only the raw
Fragment.GROUP_BUILDER text for the literal ${group:clip_path_data}; update the
check to either (A) resolve referenced fragments/templates before validating by
following any ${template:*} or fragment includes referenced from the
GROUP_BUILDER and search their expanded text for
${Namespace.GROUP}:${GroupVar.CLIP_PATH_DATA}, or (B) move this validation to
run after fragment expansion (post-expansion phase) so the presence of clip path
placeholders injected via templates is detected; ensure the logic still throws
ParserException(ErrorCode.TemplateEmissionError) only when, after following
references or after expansion, the clip_path_data placeholder is truly absent.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/FaqContent.kt`:
- Around line 105-118: The shell example in the CodeBlock call contains trailing
spaces after the line-continuation backslashes causing potential copy-paste
issues; locate the CodeBlock(...) invocation in FaqContent.kt and update its
code string so each continuation line ends with a backslash immediately followed
by the newline (no spaces after "\"), preserving the trimMargin usage and the
displayed lines/indentation.
---
Duplicate comments:
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.kt`:
- Around line 169-176: The cleanupCommaArtifacts function currently performs
global replacements on the whole line (in cleanupCommaArtifacts) which can
mutate unrelated text; change it to only normalize the region around the removed
null parameter: modify the caller (e.g., wherever
trimNullParam/removeNullPlaceholder is implemented) to pass the substring/range
surrounding the removed placeholder to a new localized cleanup function (or
extend cleanupCommaArtifacts to accept a start/end index), perform the three
replacement rules only on that slice, then reassemble the full line by
concatenating prefix + cleanedSlice + suffix; keep the replacement rules
(LEADING_COMMA, TRAILING_COMMA, DOUBLE_COMMA) but apply them only to the slice
to avoid touching other literals or comments.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.kt`:
- Around line 69-113: The forIcon function should normalize the resolved
receiver once and reuse it for iconPropertyName, IconVar.RECEIVER, and import
handling: compute a single resolvedReceiver (strip trailing '.' from
contents.receiverType if present, otherwise use
config.definitions.receiver?.name or empty/Icons marker), use that
resolvedReceiver when building IconVar.RECEIVER and when constructing
iconPropertyName, and only call ctx.addImport(...) from the TemplateContext.also
block if the receiver actually comes from config.definitions (i.e., ensure you
reference the definition's receiver.packageName/receiver.name rather than always
adding an import for the resolved string); update the logic in forIcon to derive
and reuse these values consistently.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt`:
- Around line 148-172: The chunked-icon path currently bypasses template-aware
emission and import merging: replace the direct call to
nodeEmitter.emitChunkFunctionDefinition(...) in the no-fragment branch so chunk
functions are emitted via the same template flow (use templateNodeEmitter.emit
with a TemplateContext) and ensure after resolving the defFragment (via
PlaceholderResolver.resolve) you always merge chunkNodeContext.collectedImports
into parentContext by calling
parentContext.addImports(chunkNodeContext.collectedImports); in other words,
remove the special-case emitChunkFunctionDefinition path and make both branches
build a chunkNodeContext, emit with
templateNodeEmitter/PlaceholderResolver.resolve using the defFragment or a
generated default, then add collected imports from chunkNodeContext into
parentContext so path_builder/group_builder/color mappings and imports are
preserved.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.kt`:
- Around line 573-574: The scope list for the icon namespace is too
narrow—update the assignment where scope is set (currently scope =
"`icon_template`, `icon_builder`") to include the additional icon contexts used
elsewhere on the page (add the file header and preview template scopes, e.g.
include `icon_file_header` and `icon_preview` alongside `icon_template` and
`icon_builder`) so the icon placeholders shown in file header and preview docs
are covered by the same namespace.
- Around line 755-756: Update the conflicting guidance in TemplateDocsContent by
changing the text assignment that currently states "Disable auto-discovery with
`--no-template` (CLI) or by omitting `templateFile()` from the Gradle DSL." to
match the earlier example (line ~209) that omitting `templateFile()` causes the
module to inherit the common config rather than disabling auto-discovery; keep
the CLI note about `--no-template` if accurate, but reword the Gradle part to
indicate omission = inherit common config (or explicitly call out how to disable
if different), so the message in the text property and the earlier documentation
are consistent.
- Line 734: In TemplateDocsContent (the elision example block) remove the inline
annotation comment on the line containing "fillAlpha = ${path:fill_alpha}," so
the line no longer includes the trailing comment marker (e.g., "// <- this line
is removed"); leave the code snippet content consistent with the stated rule by
deleting only the inline commentary and not the code line itself.
---
Nitpick comments:
In @.github/actions/cli-integrity-check/script.sh:
- Line 132: The conditional uses string equality for a numeric check: change the
test in the if that inspects the errors array length (the expression
"${`#errors`[@]}") to use numeric comparison (-eq) instead of == so the if [
"${`#errors`[@]}" -eq 0 ]; reads as an intented numeric comparison; update the
condition where errors array length is checked to use -eq.
In
`@svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt`:
- Around line 230-252: Add a configuration-cache functional test that exercises
the template path by running the template-based builds twice with Gradle's
"--configuration-cache" flag; specifically, create a test (next to `svg with
template matches expected output` / `avg with template matches expected output`)
that sets up the same project (uses `buildTemplateGradleContent`,
`copyTemplateIconsToProject`, `copyTemplateConfigToProject`) then calls
`runGradle("parseSvgToComposeIcon", "--configuration-cache")` twice (second run
should also pass the same flag) and asserts success with `assertTaskSuccess` and
output validity with `assertTemplateOutputMatchesExpected` (use `fileType =
"svg"` and `"xml"` respectively) to ensure `templateFile(...)` is safe for the
configuration cache.
In `@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/NodeChunker.kt`:
- Around line 55-70: The code uses a mutable counter var i incremented inside
the chunked lambda to build ImageVectorNode.ChunkFunction(functionName =
nameResolver(contents, i++), ...); replace this pattern with a functional
approach by using an indexed mapping (e.g., chunked(...).mapIndexed) so you can
remove var i and call nameResolver(contents, index + 1) (or appropriate offset)
inside mapIndexed; update the expression that builds chunkFunctions from
contents.nodes.chunked(chunkSize) { ... } to a chunked(...).mapIndexed { index,
chunk -> ImageVectorNode.ChunkFunction(functionName = nameResolver(contents,
index + 1), nodes = chunk.toList()) } to make the code idiomatic and
thread-safe.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReader.kt`:
- Around line 26-29: Update the resolve function to make its error behavior
explicit: either add KDoc above resolve including an `@throws` (or `@exception`)
entry noting that fileManager.readContent(templatePath) can throw IOException,
or wrap the readContent call in a try/catch that catches IOException and
rethrows a more specific exception (or a RuntimeException) with contextual
message (including templatePath) before returning
TemplateConfigParser.parse(content); reference resolve, fileManager.readContent,
TemplateConfigParser.parse and TemplateEmitterConfig when making the change.
In
`@website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/navigation/WebRoute.kt`:
- Around line 10-14: The `all` property on data class WebRoute constructs a new
Set each time it's accessed; change it to a cached lazy property to avoid
repeated allocations by replacing the current val all = setOf(this) + subRoutes
with a lazy-initialized property (e.g. val all by lazy { setOf(this) + subRoutes
}) so the set is created once per instance; locate WebRoute and update the all
property accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0c112916-5fc9-441b-be98-6f143a9789fb
⛔ Files ignored due to path filters (46)
config/detekt.ymlis excluded by!**/config/detekt.ymlintegrity-check/expected/template/Android.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/Android.xml.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/AttentionFilled.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/Brasil.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/LinearGradient01.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/MaskWithGroup.svg.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/ShieldHalvedSolid.xml.template.ktis excluded by!**/integrity-check/expected/**integrity-check/expected/template/StrokeGradient.xml.template.ktis excluded by!**/integrity-check/expected/**kotlin-js-store/yarn.lockis excluded by!**/yarn.lock,!**/*.lockplayground/.gitignoreis excluded by!**/playground/**playground/app/build.gradle.ktsis excluded by!**/playground/**playground/app/samples/template/attention-filled.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/samples/template/brasil.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/samples/template/shield-halved-solid.xmlis excluded by!**/samples/**,!**/playground/**playground/app/samples/template/smiley.svgis excluded by!**/*.svg,!**/samples/**,!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/Colors.ktis excluded by!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/IconBuilders.ktis excluded by!**/playground/**playground/app/src/main/kotlin/dev/tonholo/svg_to_compose/playground/ui/icon/template/Icons.ktis excluded by!**/playground/**playground/s2c.template.tomlis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/AutocompleteDropdown.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/ScrollSyncEffect.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/playground/TomlTooltip.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/BatchPanel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/BatchPhaseHeader.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/InputPanel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/PlaygroundSection.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/PlaygroundToolbar.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/TemplateEditorHelpers.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/TemplateEditorSection.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/playground/template/TemplateEditorSchema.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TemplateValidationError.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TomlKeyInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TomlSectionInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/domain/model/playground/template/TooltipInfo.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/ConversionInputFactory.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundAction.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundOptions.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundPersistence.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundState.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/PlaygroundViewModel.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplatePersistenceHelper.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/TemplateTomlExtensions.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/reducer/PlaygroundReducer.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/state/playground/reducer/TemplateReducer.ktis excluded by!**/playground/**website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/validator/playground/template/TemplateTomlValidator.ktis excluded by!**/playground/**
📒 Files selected for processing (80)
.github/actions/cli-integrity-check/action.yml.github/actions/cli-integrity-check/script.sh.github/actions/template-integrity-check/action.yml.github/actions/template-integrity-check/script.sh.github/workflows/pull_request.ymlapp.propertiesbuildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.ktdocs/ARCHITECTURE.mddocs/TEMPLATES.mdgradle/libs.playground-kmp.versions.tomlgradle/libs.playground.versions.tomlgradle/libs.versions.tomlgradle/libs.website.versions.tomlsvg-to-compose-gradle-plugin/README.mdsvg-to-compose-gradle-plugin/build.gradle.ktssvg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/dsl/parser/IconParserConfiguration.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/ParseSvgToComposeIconTask.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingParameters.ktsvg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingWorkAction.ktsvg-to-compose/README.mdsvg-to-compose/build.gradle.ktssvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/Converter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/CodeEmitterFactory.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/NodeChunker.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorNodeEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateContext.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReader.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateToken.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/NullParamTrimmer.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolver.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/resolver/ResolvedToken.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/error/ErrorCode.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ParserConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/config/TemplateConfig.ktsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/method/MethodSizeAccountable.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/NodeChunkerTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitterTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitterTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigReaderTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfigParserTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexerTest.ktsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolverTest.ktwebsite/build.gradle.ktswebsite/editor-wasm/build.gradle.ktswebsite/site/build.gradle.ktswebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/layouts/DocsLayout.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/CodeAwareSpanText.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/DocNavDropdown.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/OptionRow.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/footer/Links.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/CapabilitiesSection.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/InstallSection.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/NavHeader.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/AlternativesContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliOption.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/DocsStyles.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/FaqContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/TemplateDocsContent.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/navigation/WebRoute.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Index.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Templates.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/Shiki.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiApi.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiTransformers.ktwebsite/site/src/jsMain/kotlin/dev/tonholo/s2c/website/theme/SiteDimensions.ktwebsite/worker/build.gradle.ktswebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/ConversionTypes.ktwebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/IconConvertWorkerFactory.ktwebsite/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/inject/WorkerGraph.kt
💤 Files with no reviewable changes (1)
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/DocsStyles.kt
✅ Files skipped from review due to trivial changes (27)
- .github/actions/template-integrity-check/action.yml
- website/worker/build.gradle.kts
- website/editor-wasm/build.gradle.kts
- svg-to-compose-gradle-plugin/build.gradle.kts
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/CapabilitiesSection.kt
- app.properties
- website/site/build.gradle.kts
- website/build.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/method/MethodSizeAccountable.kt
- gradle/libs.playground-kmp.versions.toml
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateToken.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/molecules/DocNavDropdown.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/theme/SiteDimensions.kt
- website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/inject/WorkerGraph.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfigParserTest.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/Shiki.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateConfigParser.kt
- gradle/libs.versions.toml
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/resolver/PlaceholderResolverTest.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitterTest.kt
- svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/ParseSvgToComposeIconTask.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateConstants.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/imagevector/ImageVectorNodeEmitter.kt
- docs/ARCHITECTURE.md
- svg-to-compose-gradle-plugin/README.md
- docs/TEMPLATES.md
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/NavHeader.kt
🚧 Files skipped from review as they are similar to previous changes (23)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/error/ErrorCode.kt
- .github/actions/cli-integrity-check/action.yml
- svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingParameters.kt
- website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/ConversionTypes.kt
- svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/tasks/worker/IconParsingWorkAction.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/shiki/ShikiApi.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/config/TemplateConfig.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/pages/docs/Templates.kt
- gradle/libs.playground.versions.toml
- .github/workflows/pull_request.yml
- svg-to-compose/README.md
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexerTest.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/emitter/NodeChunkerTest.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/Converter.kt
- .github/actions/template-integrity-check/script.sh
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/CodeEmitterFactory.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/GradlePluginDocsContent.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/config/TemplateEmitterConfig.kt
- website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/CliOption.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/lexer/TemplateLexer.kt
- website/worker/src/jsMain/kotlin/dev/tonholo/s2c/website/worker/IconConvertWorkerFactory.kt
- svg-to-compose-gradle-plugin/src/main/kotlin/dev/tonholo/s2c/gradle/internal/parser/IconParserConfigurationImpl.kt
- buildSrc/src/main/kotlin/dev/tonholo/s2c/conventions/kmp/targets/KmpTargets.kt
Comment on lines
+99
to
+101
| private fun copyTemplateConfigToProject() { | ||
| val templateSource = repoRoot.resolve("playground/s2c.template.toml") | ||
| templateSource.copyTo(projectDir.resolve("s2c.template.toml")) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Keep the template fixture with the Gradle plugin tests.
copyTemplateConfigToProject() currently pulls playground/s2c.template.toml from another module, so website/demo edits can break this functional test and the fixture is no longer versioned with the plugin test itself. Please add a dedicated TOML fixture under the plugin test resources and copy that into the temp project instead.
As per coding guidelines, svg-to-compose-gradle-plugin/src/{test,functionalTest}/**/*.kt: Integration tests should use Gradle TestKit and place test fixtures in src/test/resources/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@svg-to-compose-gradle-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt`
around lines 99 - 101, Replace the external repo dependency in
copyTemplateConfigToProject: add a dedicated s2c.template.toml fixture under the
plugin test resources (per guidelines put fixtures in src/test/resources/), then
change copyTemplateConfigToProject() to load that resource via the test class
loader (e.g., javaClass.classLoader.getResourceAsStream("s2c.template.toml"))
and copy the stream into projectDir.resolve("s2c.template.toml") instead of
reading from repoRoot; this keeps the fixture versioned with the functional test
and removes the cross-module dependency.
...e-plugin/src/functionalTest/kotlin/dev/tonholo/s2c/gradle/Svg2ComposePluginFunctionalTest.kt
Outdated
Show resolved
Hide resolved
Comment on lines
+124
to
+129
| private fun buildDefaultPreview(contents: IconFileContents, context: TemplateContext): String { | ||
| val preview = fallbackEmitter.buildPreviewSnippet(contents) | ||
| if (preview.isEmpty()) return "" | ||
| // Register preview-related imports that the fallback emitter would have added | ||
| context.addImports(PREVIEW_IMPORTS) | ||
| return preview |
Contributor
There was a problem hiding this comment.
Default preview fallback can't see template-derived property names.
buildPreviewSnippet(contents) only receives IconFileContents, so it cannot reference TemplateContext's resolved ${icon:property_name} / receiver information. A template that emits something like Icons.TestIcon but omits a custom preview template will still get the default preview for TestIcon, and the generated file will not compile.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateEmitter.kt`
around lines 124 - 129, The default preview fallback in
TemplateEmitter.buildDefaultPreview calls
fallbackEmitter.buildPreviewSnippet(contents) which lacks TemplateContext, so
fallback previews cannot use template-resolved property/receiver names and
produce invalid references (e.g., Icons.TestIcon). Modify the fallback API and
call so the fallback emitter receives the TemplateContext: update
fallbackEmitter.buildPreviewSnippet to accept (contents, context) (or add an
overload) and change TemplateEmitter.buildDefaultPreview to pass the context
through; ensure the fallback implementation uses context's resolved property
names/receiver info and still lets TemplateEmitter register PREVIEW_IMPORTS via
context.addImports.
Comment on lines
+77
to
+85
| val hasClipPath = group.params.clipPath != null | ||
| val fragmentReferencesClipPath = "\${${Namespace.GROUP}:${GroupVar.CLIP_PATH_DATA}}" in fragment | ||
| if (hasClipPath && !fragmentReferencesClipPath) { | ||
| throw ParserException( | ||
| errorCode = ErrorCode.TemplateEmissionError, | ||
| message = "Icon group has clipPathData but the '${Fragment.GROUP_BUILDER}' " + | ||
| "fragment does not reference \${${Namespace.GROUP}:${GroupVar.CLIP_PATH_DATA}}. " + | ||
| "Add it to the fragment to avoid silently dropping the clip path.", | ||
| ) |
Contributor
There was a problem hiding this comment.
Don't require clip_path_data to appear literally in group_builder.
This guard only searches the raw group_builder text. A valid template that pulls ${group:clip_path_data} in through ${template:*} will still throw TemplateEmissionError here even though the clip path is preserved after fragment expansion. The check needs to follow referenced fragments, or run after expansion, before rejecting the template.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/emitter/template/TemplateNodeEmitter.kt`
around lines 77 - 85, The current guard in TemplateNodeEmitter (variables
hasClipPath and fragmentReferencesClipPath) incorrectly searches only the raw
Fragment.GROUP_BUILDER text for the literal ${group:clip_path_data}; update the
check to either (A) resolve referenced fragments/templates before validating by
following any ${template:*} or fragment includes referenced from the
GROUP_BUILDER and search their expanded text for
${Namespace.GROUP}:${GroupVar.CLIP_PATH_DATA}, or (B) move this validation to
run after fragment expansion (post-expansion phase) so the presence of clip path
placeholders injected via templates is detected; ensure the logic still throws
ParserException(ErrorCode.TemplateEmissionError) only when, after following
references or after expansion, the clip_path_data placeholder is truly absent.
website/site/src/jsMain/kotlin/dev/tonholo/s2c/website/components/organisms/docs/FaqContent.kt
Show resolved
Hide resolved
- fix(ci): clean up cli-integrity-check script parameter parsing, remove vestigial $2 assignment, add --extension required validation, fix suffix not being set to "optimized" when --optimize flag is passed, use POSIX = for string comparisons and -eq for numeric comparisons - fix(gradle-plugin): replace auto-bootstrap with assertion failure for missing template golden files in functional tests - fix(website): remove trailing spaces after line-continuation backslashes in FaqContent shell code block - fix(website): expand icon namespace scope to include file_header and preview contexts in TemplateDocsContent - fix(website): remove editorial inline comment from null-handling code example in TemplateDocsContent - fix(website): correct contradictory auto-discovery guidance in TemplateDocsContent - omitting templateFile() inherits common config - refactor(website): use lazy delegate for WebRoute.all property
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
s2c.template.toml)templateFileDSL option), and the website playground (TOML editor with validation, autocomplete, and syntax highlighting)Summary by CodeRabbit
Version 3.0.0 Release Notes
New Features
s2c.template.tomlconfiguration files--templateand--no-templateflagstemplateFile()configuration optionDocumentation