Skip to content

Conversation

@t-regbs
Copy link
Collaborator

@t-regbs t-regbs commented Dec 7, 2025

What did I do?

PR adds a new feature that displays actual icon previews for ImageVector Kotlin files in the project view, instead of the default Kotlin file icon. This makes it easier to visually identify icons in your project without opening the files.

What did I Change?

  • Added new IconProvider
  • Extracted shared utilities for reuse between ImageVectorIconProvider, ImageVectorGutterProvider, and ImageVectorCompletionContributor.
Screen.Recording.2025-12-07.at.18.29.50.mov

@coderabbitai
Copy link

coderabbitai bot commented Dec 7, 2025

Walkthrough

Image-vector detection, parsing, and icon creation were moved into a new utility file (ImageVectorPsiUtil.kt) that provides public extensions (e.g., KtProperty.isImageVector(), KtFile.hasImageVectorProperties(), KtFile.getOrCreateCachedIcon(), KtProperty.getOrCreateGutterIcon()) and internal parsing helpers. ImageVectorCompletionContributor and ImageVectorGutterProvider now call those utilities instead of using private helpers. A new ImageVectorIconProvider class was added and registered in plugin.xml to supply file icons for Kotlin files containing image-vector properties. Public API additions include the new extensions and the icon provider registration.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main feature: displaying ImageVector Kotlin file icons in the project view instead of default icons.
Description check ✅ Passed The description is directly related to the changeset, explaining the new IconProvider feature and the extraction of shared utilities for icon handling.
Linked Issues check ✅ Passed The PR successfully implements the objective from issue #681 by adding an IconProvider to display ImageVector icon previews in the project view instead of default Kotlin file icons.
Out of Scope Changes check ✅ Passed All changes are scoped to supporting the icon preview feature: the new IconProvider, refactored utilities in ImageVectorPsiUtil, updates to existing providers, and plugin manifest registration.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d8d8357 and dc52ad1.

📒 Files selected for processing (1)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build (ubuntu-latest)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorCompletionContributor.kt (1)

50-56: Consider gating icon creation by ImageVector type

getOrCreateCachedIcon currently runs for any non‑var property; createImageVectorIcon() will then do parsing even for files that don’t actually expose ImageVector properties. Since you now have KtProperty.isImageVector() in ImageVectorPsiUtil, you could optionally short‑circuit in fillCompletionVariants:

-                ?.let { element ->
-                    if (!element.isVar) {
-                        getOrCreateCachedIcon(element.containingKtFile)
-                    } else {
-                        null
-                    }
-                }
+                ?.let { element ->
+                    if (!element.isVar && element.isImageVector()) {
+                        getOrCreateCachedIcon(element.containingKtFile)
+                    } else {
+                        null
+                    }
+                }

This keeps completion behavior the same for ImageVector properties while avoiding extra work for unrelated completions.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterProvider.kt (1)

19-19: Good reuse of shared ImageVector type detection

Switching to the shared io.github.composegears.valkyrie.util.isImageVector keeps gutter detection consistent with the new icon provider and completion paths, and removes duplicated constants/logic here. If you later decide to make isImageVector more robust (e.g., handling ImageVector? or type aliases), all three call sites will benefit immediately.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1)

12-53: Shared PSI utilities are well-factored; consider minor robustness and caching improvements

The centralization of ImageVector detection and icon creation here is a nice cleanup and makes behavior consistent across completion, gutter, and the new icon provider.

A couple of small follow‑ups you might consider:

  1. Type matching robustness in isImageVector
    Right now detection is based on typeReference?.text being exactly one of:

    IMAGE_VECTOR_TYPES = setOf(
        "ImageVector",
        "androidx.compose.ui.graphics.vector.ImageVector",
    )

    This will miss cases like ImageVector?, List<ImageVector>, or aliases. A low‑cost improvement would be to normalize simple nullable syntax:

    fun KtProperty.isImageVector(): Boolean {
        val raw = typeReference?.text ?: return false
        val typeText = raw.removeSuffix("?")
        return typeText in IMAGE_VECTOR_TYPES
    }

    If you need to support aliases or more complex type shapes in the future, this is also a good central place to switch to a resolution‑based check.

  2. Central parse caching for createImageVectorIcon
    createImageVectorIcon() is now called from multiple places (completion and IconProvider). Each call site wraps it in its own CachedValuesManager.getCachedValue invocation, so ImageVectorPsiParser.parseToIrImageVector(this) may still end up running more than once per file.

    You could move the caching down into this util, e.g.:

    fun KtFile.cachedImageVectorIcon(): Icon? =
        CachedValuesManager.getManager(project).getCachedValue(this) {
            val ir = ImageVectorPsiParser.parseToIrImageVector(this) 
            val icon = ir?.let {
                ImageVectorIcon(
                    vectorXml = it.toVectorXmlString(),
                    aspectRatio = it.aspectRatio,
                    dominantShade = it.dominantShadeColor,
                )
            }
            CachedValueProvider.Result.create(icon, this)
        }

    and then have callers use cachedImageVectorIcon() instead of wrapping createImageVectorIcon() themselves. This keeps parsing cost strictly once per file until PSI changes.

  3. Visibility of IMAGE_VECTOR_TYPES
    If IMAGE_VECTOR_TYPES isn’t meant to be part of your public API surface, consider marking it internal to make future refactors easier.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e24d7ab and 6497472.

📒 Files selected for processing (5)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorCompletionContributor.kt (2 hunks)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterProvider.kt (1 hunks)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/icon/ImageVectorIconProvider.kt (1 hunks)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1 hunks)
  • tools/idea-plugin/src/main/resources/META-INF/plugin.xml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build (ubuntu-latest)
🔇 Additional comments (2)
tools/idea-plugin/src/main/resources/META-INF/plugin.xml (1)

79-79: Icon provider registration looks correct

The iconProvider extension is wired to the right FQN and order="first" is appropriate to override the default Kotlin file icon with the ImageVector preview when available. No issues from a registration standpoint.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/icon/ImageVectorIconProvider.kt (1)

14-33: IconProvider implementation looks sound; verify dumb-mode safety and consider caching optimization

The overall flow (resolve KtFilehasImageVectorProperties() guard → cached createImageVectorIcon()) is solid and avoids heavy work for non-Kotlin files.

Two follow-ups worth considering:

  1. Dumb mode safety is critical
    The class declares DumbAware, meaning getIcon runs while indices are being built. Per IntelliJ best practices, verify that createImageVectorIcon() and ImageVectorPsiParser.parseToIrImageVector do not call index-dependent APIs (e.g., resolve, stub lookups, cross-file references). If they do, either remove DumbAware or guard with DumbService.isDumb(project) and return null during indexing. Index-dependent calls in dumb mode will throw IndexNotReadyException.

  2. Cache the hasImageVectorProperties() check
    hasImageVectorProperties() scans top-level properties on every getIcon call. Fold this check into the cached computation to avoid repeated work:

    return CachedValuesManager.getCachedValue(ktFile) {
        val icon = if (ktFile.hasImageVectorProperties()) ktFile.createImageVectorIcon() else null
        CachedValueProvider.Result.create(icon, ktFile)
    }

    This ensures both the "does this file matter?" check and icon creation recompute only when the file changes, aligning with IconProvider performance guidelines.

@t-regbs t-regbs removed the feature label Dec 7, 2025
@egorikftp
Copy link
Member

@t-regbs you can work directly in this repository, not necessary use fork anymore 🙂

…xtension functions for improved performance and code clarity
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (2)

18-21: Broaden isImageVector() detection to handle nullable / fully‑qualified types more robustly

isImageVector() compares typeReference?.text directly against IMAGE_VECTOR_TYPES, which will miss common cases like ImageVector? or fully‑qualified nullable types. That can silently skip valid ImageVector properties and prevent icons from showing up.

You can cheaply normalize the text before lookup, e.g.:

fun KtProperty.isImageVector(): Boolean {
    val rawTypeText = typeReference?.text ?: return false
    val normalizedTypeText = rawTypeText.removeSuffix("?")
    return normalizedTypeText in IMAGE_VECTOR_TYPES
}

If you expect generic wrappers (e.g. SomeAlias<ImageVector>), you may also want to strip trivial generic wrappers or rely on resolved types instead of raw text.

Also applies to: 113-116


72-81: Avoid duplicating ImageVectorIcon construction logic

Both KtFile.createImageVectorIcon() and KtProperty.createIcon() build ImageVectorIcon from IrImageVector in the same way. To keep things DRY and easier to tweak later (e.g. if rendering parameters change), consider extracting a small helper:

private fun IrImageVector.toIcon(): Icon = ImageVectorIcon(
    vectorXml = toVectorXmlString(),
    aspectRatio = aspectRatio,
    dominantShade = dominantShadeColor,
)

private fun KtFile.createImageVectorIcon(): Icon? =
    ImageVectorPsiParser.parseToIrImageVector(this)
        ?.toIcon()

private fun KtProperty.createIcon(): Icon? =
    parseImageVectorProperty(this)
        ?.toIcon()

This keeps the IR‑>icon mapping in one place.

Also applies to: 83-91

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6497472 and 6ffc718.

📒 Files selected for processing (4)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorCompletionContributor.kt (2 hunks)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterProvider.kt (3 hunks)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/icon/ImageVectorIconProvider.kt (1 hunks)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/icon/ImageVectorIconProvider.kt
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorCompletionContributor.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.744Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
📚 Learning: 2025-12-07T20:07:49.744Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.744Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.

Applied to files:

  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterProvider.kt
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build (ubuntu-latest)
🔇 Additional comments (4)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (3)

23-34: hasImageVectorProperties() implementation matches documented behavior

The function correctly limits itself to top‑level KtProperty declarations and delegates to isImageVector(), which aligns with the KDoc and intended “file contains at least one ImageVector property” semantics. No changes needed here.


94-107: Double‑check parseImageVectorProperty behavior on compiled‑code stubs

The fallback from property to navigationElement is a good way to reach decompiled/attached source. One open question: for properties coming from compiled libraries where containingKtFile is a stub with /* compiled code */, you currently call parseToIrImageVector(containingFile) before checking for COMPILED_CODE_MARKER (the marker is only used for the navigation element path).

If ImageVectorPsiParser.parseToIrImageVector doesn’t already short‑circuit efficiently on compiled‑code stubs, it might be worth mirroring the marker check before the first parse to avoid redundant work and make behavior more predictable between the two branches.

Can you confirm that parseToIrImageVector is cheap/safe on such stubs, or consider adding the same guard for containingFile if not?


36-48: Clarify KDoc to match actual function scope

The KDoc describes a "cached gutter icon for an ImageVector property", but this is an extension on KtFile and operates at the file level, not on a specific property. Update the documentation to reflect the actual scope (e.g., "Gets or creates a cached icon for a Kotlin file that defines an ImageVector").

Regarding cache dependencies: the use of this (the KtFile instance) as the sole invalidation trigger is reasonable for file-level caching, though explicit modification tracking could be considered if stale icons become an issue in practice.

tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterProvider.kt (1)

11-12: Good delegation to shared ImageVector utilities in gutter provider

Switching to KtProperty.isImageVector() and getOrCreateGutterIcon() keeps the gutter provider focused on line‑marker wiring while centralizing detection, parsing, and caching in ImageVectorPsiUtil. The usage pattern (filter on isImageVector() before calling getOrCreateGutterIcon() for both definitions and resolved references) looks correct and should be easier to maintain than the previous inlined logic.

Also applies to: 25-52

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (2)

15-21: Normalize type detection in isImageVector to handle nullable and generic usages

Right now isImageVector relies on exact typeReference.text matching, so ImageVector?, List<ImageVector>, or other simple variations won’t be recognized and will silently skip icons. You can cheaply harden this by normalizing the type text before the membership check.

A minimal, backwards-compatible tweak:

 fun KtProperty.isImageVector(): Boolean {
-    val typeText = typeReference?.text ?: return false
-    return typeText in IMAGE_VECTOR_TYPES
+    val typeText = typeReference?.text ?: return false
+    // Strip generic arguments and nullability before matching
+    val normalized = typeText
+        .substringBefore('<') // e.g. "List<ImageVector>" -> "List"
+        .removeSuffix("?")    // e.g. "ImageVector?" -> "ImageVector"
+
+    return normalized in IMAGE_VECTOR_TYPES
 }

And keep IMAGE_VECTOR_TYPES as-is so this continues to support both short and FQN forms.


36-48: Clarify docs and consider small caching helper reuse

Two minor points here:

  1. The KtFile extension getOrCreateCachedIcon’s KDoc still talks about a “gutter icon for an ImageVector property”, while it now represents a file‑level preview icon. Updating the comment would avoid confusion with the property‑level getOrCreateGutterIcon.

  2. The caching patterns in getOrCreateCachedIcon and getOrCreateGutterIcon are nearly identical; you could factor a tiny private helper to reduce duplication and make it harder to accidentally change one without the other.

Example of a small refactor:

-fun KtFile.getOrCreateCachedIcon(): Icon? {
-    return CachedValuesManager
-        .getManager(project).getCachedValue(this) {
-            val icon = createImageVectorIcon()
-            CachedValueProvider.Result.create(icon, this)
-        }
-}
+fun KtFile.getOrCreateCachedIcon(): Icon? =
+    CachedValuesManager.getManager(project)
+        .getCachedValue(this) { CachedValueProvider.Result.create(createImageVectorIcon(), this) }
@@
-fun KtProperty.getOrCreateGutterIcon(): Icon? {
-    val cachedValuesManager = CachedValuesManager.getManager(project)
-
-    return cachedValuesManager.getCachedValue(this) {
-        val icon = createIcon()
-
-        CachedValueProvider.Result.create(icon, this)
-    }
-}
+fun KtProperty.getOrCreateGutterIcon(): Icon? =
+    CachedValuesManager.getManager(project)
+        .getCachedValue(this) { CachedValueProvider.Result.create(createIcon(), this) }

Purely cosmetic, so feel free to keep as‑is if you prefer the more verbose style.

Also applies to: 50-64

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6ffc718 and 5659989.

📒 Files selected for processing (1)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.744Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
📚 Learning: 2025-12-07T20:07:49.744Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.744Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.

Applied to files:

  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt
🧬 Code graph analysis (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1)
sdk/ir/xml/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/ir/xml/IrToVectorXml.kt (1)
  • toVectorXmlString (6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build (ubuntu-latest)
🔇 Additional comments (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (2)

23-34: Top‑level scan in hasImageVectorProperties looks appropriate

Filtering declarations to top‑level KtProperty and delegating to isImageVector() is straightforward and matches the “one ImageVector per generated file” model. This looks good as long as you intentionally exclude icons nested inside objects/classes from project‑view previews.


66-104: Parsing strategy and library fallback look solid; just be aware of single‑icon‑per‑file assumption

The flow in parseImageVectorProperty (parse containing file first, then fall back to navigationElement and skip stubs via COMPILED_CODE_MARKER) is a nice, robust way to handle both project and library properties.

One behavioral detail: since ImageVectorPsiParser.parseToIrImageVector is invoked with a KtFile (not the specific KtProperty), this effectively assumes there is a single relevant ImageVector per file. That’s consistent with generated icon files, but if you ever support multiple vectors per Kotlin file, different properties in the same file would all resolve to whichever vector the parser picks.

If that multi‑icon scenario is in scope later, you may want to:

  • Either extend ImageVectorPsiParser with a property‑aware overload, or
  • Pass some property identity (e.g., name) to select the correct IrImageVector.

For the current generator‑driven use case, the implementation looks good.

If you expect to support multiple ImageVector properties in a single Kotlin file in the future, please double‑check how ImageVectorPsiParser.parseToIrImageVector(KtFile) behaves in that case and whether you’d want property‑specific selection.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (4)

18-21: Raw type text comparison may miss some valid ImageVector types

isImageVector relies on typeReference?.text matching exactly "ImageVector" or the FQ name. This will not detect cases like ImageVector?, comments/whitespace variations, or aliased imports, and it may be brittle if formatting changes.

If you expect such patterns, consider normalizing the text (e.g., trimming nullability) or, more robustly, resolving the type to its FQ name via Kotlin type analysis and comparing that instead. If the generator always emits exact ImageVector/FQ names, current code is fine but somewhat fragile to manual edits.


56-64: Per‑property cache may re‑parse the same file multiple times

KtProperty.getOrCreateGutterIcon caches per property, but createIcon()parseImageVectorProperty re‑parses the whole KtFile each time. If multiple ImageVector properties live in the same file, you’ll parse that file N times (once per property), even though they all resolve to the same IrImageVector under the current API.

Not critical, but if you notice performance issues on large icon packs, consider introducing a file‑level cache for the parsed IrImageVector (similar to getOrCreateCachedIcon), and let the gutter helper delegate to that instead of re‑parsing.


84-101: Clarify assumptions in parseImageVectorProperty and tighten stub handling

A couple of points worth considering in parseImageVectorProperty:

  1. The function ignores the specific KtProperty when parsing and instead calls parseToIrImageVector(KtFile) both for the containing and navigation‑element files. This implicitly assumes “one ImageVector per file”. If a file ever contained multiple ImageVectors, all gutter icons would end up using whatever parseToIrImageVector returns for that file. If the generator guarantees one vector per file, that’s fine; otherwise the parser API may eventually need a property‑aware variant.

  2. For library properties, you first attempt to parse property.containingKtFile and only later check COMPILED_CODE_MARKER on the navigation file. You could avoid attempting to parse compiled stubs at all by checking the marker before calling the parser, both for the containing file and the navigation file. For example:

-    val containingFile = property.containingKtFile
-    ImageVectorPsiParser.parseToIrImageVector(containingFile)?.let { return it }
+    val containingFile = property.containingKtFile
+    if (COMPILED_CODE_MARKER !in containingFile.text) {
+        ImageVectorPsiParser.parseToIrImageVector(containingFile)?.let { return it }
+    }
@@
-        val sourceFile = navigationElement.containingKtFile
-
-        // Only parse if we have actual source code (not a stub)
-        if (COMPILED_CODE_MARKER !in sourceFile.text) {
-            return ImageVectorPsiParser.parseToIrImageVector(sourceFile)
-        }
+        val sourceFile = navigationElement.containingKtFile
+        if (COMPILED_CODE_MARKER !in sourceFile.text) {
+            return ImageVectorPsiParser.parseToIrImageVector(sourceFile)
+        }

Not urgent, but this would make the intent around stubs vs real sources more explicit and avoid unnecessary parsing attempts.


103-108: Hard‑coded type names and compiled‑code marker are acceptable but a bit brittle

IMAGE_VECTOR_TYPES and COMPILED_CODE_MARKER work, but they encode IDE/Kotlin compiler details as plain strings:

  • If you ever support additional ImageVector type aliases (or migrate to another package), you’ll need to update IMAGE_VECTOR_TYPES manually.
  • COMPILED_CODE_MARKER mirrors how Kotlin decompiled stubs are rendered today; if that marker ever changes upstream, stub detection here will silently stop working.

Neither is a blocker, but consider:

  • Renaming IMAGE_VECTOR_TYPES to clarify they’re FQ/simple names, and centralizing any future aliases here.
  • Adding a short comment that COMPILED_CODE_MARKER matches the current Kotlin decompiled stub output, so future maintainers know why that magic string exists.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5659989 and d8d8357.

📒 Files selected for processing (1)
  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.744Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.
📚 Learning: 2025-12-07T20:07:49.744Z
Learnt from: egorikftp
Repo: ComposeGears/Valkyrie PR: 750
File: tools/gradle-plugin/src/main/kotlin/io/github/composegears/valkyrie/gradle/internal/task/GenerateImageVectorsTask.kt:71-85
Timestamp: 2025-12-07T20:07:49.744Z
Learning: In the Valkyrie Gradle plugin (Kotlin), the `useFlatPackage` flag in `IconPackExtension` is only applicable when nested packs are configured. For single icon packs (without nested packs), the flag is intentionally not propagated to `ImageVectorGeneratorConfig` as there is no package hierarchy to flatten.

Applied to files:

  • tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt
🧬 Code graph analysis (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1)
sdk/ir/xml/src/commonMain/kotlin/io/github/composegears/valkyrie/sdk/ir/xml/IrToVectorXml.kt (1)
  • toVectorXmlString (6-6)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: build (ubuntu-latest)
🔇 Additional comments (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (2)

29-34: Only top‑level KtProperty declarations are considered for file ImageVectors

hasImageVectorProperties checks only declarations.filterIsInstance<KtProperty>(), i.e. top‑level properties. Properties inside objects/classes (e.g. object Icons { val Foo: ImageVector }) will be ignored.

If Valkyrie‑generated icons are guaranteed to expose a top‑level KtProperty per file (including extension properties like Icons.Filled.Foo), this matches the intended model. Otherwise, you may want to descend into nested declarations when scanning for ImageVector properties.


72-82: Icon creation pipeline looks clean and cohesive

createImageVectorIcon/createIcon + IrImageVector.toIcon form a clear, reusable pipeline that centralizes the XML generation and visual metadata (aspectRatio, dominantShadeColor). This keeps callers simple and should make it easy to adjust rendering details later without touching the PSI plumbing.

@egorikftp egorikftp merged commit 36d72fd into ComposeGears:main Dec 10, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Plugin] Icon preview instead default kotlin icon for kt files

2 participants