[Gutter] Add support for multiple icons in a file#894
Merged
Conversation
egorikftp
force-pushed
the
feature/idea/gutter
branch
from
feb93e2 to
7e21e6e
Compare
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughAdds KtProperty-based parsing across the ImageVector parsing stack: a new parseToIrImageVector(KtProperty) in ImageVectorPsiParser and corresponding parse(KtProperty) entry points plus internal KtProperty.parseImageVector() extensions in MaterialImageVectorPsiParser and RegularImageVectorPsiParser. ImageVectorPsiUtil now prefers parsing from a KtProperty/navigation element. ImageVectorIconProvider gains KtProperty handling and early guards. ImageVectorIcon.toString() was added. Tests for multiple gutter icons and Gradle/JUnit5 test config changes were included. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorIcon.kt (1)
88-90:toString()truncation always appends...even for short XML.Minor nit: if
vectorXmlis ≤100 chars, the trailing...is misleading since nothing was truncated.Also, this
toString()is used in tests to assert icon inequality. Consider implementingequals/hashCode(or a dedicated content-comparison method) for more robust comparisons, since two icons differing only after the 100th character ofvectorXmlwould appear equal viatoString().Suggested fix for the trailing ellipsis
override fun toString(): String { - return "ImageVectorIcon(size=$size, aspectRatio=$aspectRatio, dominantShade=$dominantShade, vectorXml=${vectorXml.take(100)}...)" + val xmlPreview = if (vectorXml.length > 100) "${vectorXml.take(100)}..." else vectorXml + return "ImageVectorIcon(size=$size, aspectRatio=$aspectRatio, dominantShade=$dominantShade, vectorXml=$xmlPreview)" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorIcon.kt` around lines 88 - 90, The toString() implementation of ImageVectorIcon always appends "..." after vectorXml even when vectorXml.length ≤ 100, and tests rely on toString() for comparisons which can hide differences beyond 100 chars; update the toString() method to conditionally append "..." only if vectorXml.length > 100 and include the full string when shorter, and additionally implement proper equality semantics by overriding equals() and hashCode() (or add a dedicated contentEquals(other: ImageVectorIcon) method) to compare size, aspectRatio, dominantShade and the full vectorXml so tests and consumers compare actual content rather than a truncated representation.tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterTest.kt (2)
116-119: Icon inequality relies ontoString()— fragile but acceptable for now.The comparison uses
toString()which truncatesvectorXmlat 100 chars. If two icons ever differ only beyond that point, this assertion would incorrectly pass. For these specific test cases, the builder names differ ("WithoutPath" vs "MyPath"), so it works.If you add
equals/hashCodetoImageVectorIcon(or a content-based comparison method), the test could use that directly for a more robust assertion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterTest.kt` around lines 116 - 119, The test in ImageVectorGutterTest uses toString() on ImageVectorIcon for inequality (the assertNotEquals comparing gutterMark1.lineMarkerInfo.icon.cast<ImageVectorIcon>().toString() and gutterMark2...toString()), which is fragile because toString() truncates vectorXml; update the test to compare icons by content instead: either add/enable an equals/hashCode implementation on ImageVectorIcon (or a dedicated contentEquals method) and use assertNotEquals on the cast icons themselves (gutterMark1.lineMarkerInfo.icon.cast<ImageVectorIcon>() vs gutterMark2...), or, if changing the model is unwanted, extract and compare the full vectorXml or builder name fields from the ImageVectorIcon instances for a deterministic content-based comparison.
1-12: Mixed assertion libraries — consider consistency.The tests use three different assertion sources:
assertk(assertThat(...).hasSize(...))kotlin.test(assertNotEquals)- Inherited JUnit assertions (
assertEquals,assertNotNull— presumably from theKotlinCodeInsightTestbase class)This works, but mixing assertion libraries in a single file can be confusing for contributors. Consider unifying on one style (e.g., all
assertkor allkotlin.test).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterTest.kt` around lines 1 - 12, The test mixes assertion libraries (assertk, kotlin.test, and JUnit via KotlinCodeInsightTest); pick one (recommend assertk) and make the file consistent: replace kotlin.test.assertNotEquals and any JUnit-style assertEquals/assertNotNull usages with assertk equivalents (e.g., isNotEqualTo, isEqualTo, isNotNull) in ImageVectorGutterTest and update imports to only import assertk.assertThat and assertk.assertions.* while removing kotlin.test and JUnit assertion imports; ensure assertions that check collection size use hasSize from assertk and run tests to confirm no remaining mixed assertions.sdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/parser/RegularImageVectorPsiParser.kt (2)
38-40:parse(ktProperty)silently accepts anyKtPropertywithout a type guard.Unlike
parse(ktFile), which pre-filters properties withtypeReference?.text == "ImageVector", this overload performs no such check. It will silently attempt to parse and returnnullfor non-ImageVector properties (likely fine), but a malformed property with a coincidentally matching structure could produce a corruptIrImageVector. If the call site always pre-filters, at minimum a KDoc contract note would prevent future misuse.💡 Suggested guard (or document the precondition)
fun parse(ktProperty: KtProperty): IrImageVector? { + require(ktProperty.typeReference?.text == "ImageVector") { + "ktProperty must be of type ImageVector" + } return ktProperty.parseImageVector() }Or, at minimum, document the precondition:
+/** Caller must ensure [ktProperty] has a type reference of `ImageVector`. */ fun parse(ktProperty: KtProperty): IrImageVector? {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/parser/RegularImageVectorPsiParser.kt` around lines 38 - 40, The parse(ktProperty: KtProperty) overload lacks a type guard and may attempt to parse non-ImageVector properties; update parse(ktProperty) to first check the property's typeReference (e.g. ktProperty.typeReference?.text == "ImageVector") and return null (or throw) if it doesn't match, or alternatively add a clear KDoc on parse(ktProperty) stating the precondition that callers must only pass ImageVector-typed KtProperty instances; reference parseFile/parse(ktFile) which already filters by type and ensure parseImageVector is only invoked when the type guard passes.
42-42: Consider makingKtProperty.parseImageVector()privatefor tighter encapsulation.The extension is only used within
RegularImageVectorPsiParserand is not shared with other parsers (e.g.,MaterialImageVectorPsiParserdefines its own separate implementation). Usingprivateinstead ofinternalrestricts visibility to the immediate scope where it's needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/parser/RegularImageVectorPsiParser.kt` at line 42, The extension function KtProperty.parseImageVector() is declared internal but only used inside RegularImageVectorPsiParser; change its visibility to private to tighten encapsulation by replacing the internal modifier with private on the KtProperty.parseImageVector() declaration and verify there are no external references (e.g., other parser classes like MaterialImageVectorPsiParser) before committing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@sdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/ImageVectorPsiParser.kt`:
- Around line 21-29: The KtProperty overload parseToIrImageVector(ktProperty)
treats a null containingKtFile.importList as isMaterial = false and proceeds to
parse, which is inconsistent with the KtFile overload that returns null when
importList is missing; change the importList handling in
parseToIrImageVector(ktProperty) to short-circuit and return null when
containingKtFile.importList is null (e.g., use
containingKtFile.importList?.isMaterial() ?: return null) so both overloads
behave consistently before calling MaterialImageVectorPsiParser.parse or
RegularImageVectorPsiParser.parse.
In `@tools/idea-plugin/build.gradle.kts`:
- Around line 86-87: The test task in build.gradle.kts is not configured to run
JUnit5, so add configuration to the Gradle test task to call useJUnitPlatform()
(i.e. configure the Test task named "test" to invoke useJUnitPlatform()) so
JUnit5 tests are discovered and executed; locate the test task declaration or
add a test { useJUnitPlatform() } block in build.gradle.kts and verify any prior
IJPL workarounds (opentest4j/IJPL-157292, JUnit4 runtime/IJPL-159134) are still
needed with the current IntelliJ Platform Gradle Plugin version.
---
Nitpick comments:
In
`@sdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/parser/RegularImageVectorPsiParser.kt`:
- Around line 38-40: The parse(ktProperty: KtProperty) overload lacks a type
guard and may attempt to parse non-ImageVector properties; update
parse(ktProperty) to first check the property's typeReference (e.g.
ktProperty.typeReference?.text == "ImageVector") and return null (or throw) if
it doesn't match, or alternatively add a clear KDoc on parse(ktProperty) stating
the precondition that callers must only pass ImageVector-typed KtProperty
instances; reference parseFile/parse(ktFile) which already filters by type and
ensure parseImageVector is only invoked when the type guard passes.
- Line 42: The extension function KtProperty.parseImageVector() is declared
internal but only used inside RegularImageVectorPsiParser; change its visibility
to private to tighten encapsulation by replacing the internal modifier with
private on the KtProperty.parseImageVector() declaration and verify there are no
external references (e.g., other parser classes like
MaterialImageVectorPsiParser) before committing.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorIcon.kt`:
- Around line 88-90: The toString() implementation of ImageVectorIcon always
appends "..." after vectorXml even when vectorXml.length ≤ 100, and tests rely
on toString() for comparisons which can hide differences beyond 100 chars;
update the toString() method to conditionally append "..." only if
vectorXml.length > 100 and include the full string when shorter, and
additionally implement proper equality semantics by overriding equals() and
hashCode() (or add a dedicated contentEquals(other: ImageVectorIcon) method) to
compare size, aspectRatio, dominantShade and the full vectorXml so tests and
consumers compare actual content rather than a truncated representation.
In
`@tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterTest.kt`:
- Around line 116-119: The test in ImageVectorGutterTest uses toString() on
ImageVectorIcon for inequality (the assertNotEquals comparing
gutterMark1.lineMarkerInfo.icon.cast<ImageVectorIcon>().toString() and
gutterMark2...toString()), which is fragile because toString() truncates
vectorXml; update the test to compare icons by content instead: either
add/enable an equals/hashCode implementation on ImageVectorIcon (or a dedicated
contentEquals method) and use assertNotEquals on the cast icons themselves
(gutterMark1.lineMarkerInfo.icon.cast<ImageVectorIcon>() vs gutterMark2...), or,
if changing the model is unwanted, extract and compare the full vectorXml or
builder name fields from the ImageVectorIcon instances for a deterministic
content-based comparison.
- Around line 1-12: The test mixes assertion libraries (assertk, kotlin.test,
and JUnit via KotlinCodeInsightTest); pick one (recommend assertk) and make the
file consistent: replace kotlin.test.assertNotEquals and any JUnit-style
assertEquals/assertNotNull usages with assertk equivalents (e.g., isNotEqualTo,
isEqualTo, isNotNull) in ImageVectorGutterTest and update imports to only import
assertk.assertThat and assertk.assertions.* while removing kotlin.test and JUnit
assertion imports; ensure assertions that check collection size use hasSize from
assertk and run tests to confirm no remaining mixed assertions.
.../kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/ImageVectorPsiParser.kt
Show resolved
Hide resolved
egorikftp
force-pushed
the
feature/idea/gutter
branch
2 times, most recently
from
76d7ef0 to
1c2e4b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt (1)
84-93:⚠️ Potential issue | 🟠 MajorFallback still uses file-level parsing — wrong icon returned for non-first library properties.
navigationElementis already confirmed to be aKtPropertyat line 86. CallingparseToIrImageVector(sourceFile)at line 91 always resolves to the firstImageVectorproperty in the library source file, so any non-first library icon resolves to the wrong gutter icon. SinceparseToIrImageVector(KtProperty)now exists, use it directly.🐛 Proposed fix
if (navigationElement is KtProperty && navigationElement != property) { 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) + return ImageVectorPsiParser.parseToIrImageVector(navigationElement) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt` around lines 84 - 93, The current fallback parses the whole source file via ImageVectorPsiParser.parseToIrImageVector(sourceFile) which always returns the first ImageVector in the file; instead, when navigationElement is a KtProperty (we've already checked this) call ImageVectorPsiParser.parseToIrImageVector(navigationElement as KtProperty) so the parser targets that specific property; update the block around navigationElement / KtProperty / property to return the result of parseToIrImageVector(navigationElement) rather than parseToIrImageVector(sourceFile).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorIcon.kt`:
- Around line 88-90: The ImageVectorIcon.toString() always appends "..." after
vectorXml.take(100) even when vectorXml is shorter than 100 chars; update
toString() in class ImageVectorIcon to conditionally append "..." only when
vectorXml.length > 100 (or use takeLast/substring logic) so the representation
shows the full vectorXml when short; reference the toString() method, the
vectorXml property and existing fields size, aspectRatio, dominantShade when
implementing the conditional truncation.
---
Outside diff comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt`:
- Around line 84-93: The current fallback parses the whole source file via
ImageVectorPsiParser.parseToIrImageVector(sourceFile) which always returns the
first ImageVector in the file; instead, when navigationElement is a KtProperty
(we've already checked this) call
ImageVectorPsiParser.parseToIrImageVector(navigationElement as KtProperty) so
the parser targets that specific property; update the block around
navigationElement / KtProperty / property to return the result of
parseToIrImageVector(navigationElement) rather than
parseToIrImageVector(sourceFile).
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorIcon.kt
Show resolved
Hide resolved
t-regbs
reviewed
Feb 23, 2026
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kt
Outdated
Show resolved
Hide resolved
t-regbs
requested changes
Feb 23, 2026
egorikftp
force-pushed
the
feature/idea/gutter
branch
2 times, most recently
from
be55b85 to
4167455
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/idea-plugin/build.gradle.kts`:
- Around line 74-75: Remove the redundant explicit JUnit5 dependencies by
deleting the testImplementation(libs.junit5.jupiter) and
testRuntimeOnly(libs.junit.launcher) declarations; the project already
configures JUnit5 via testFramework(TestFrameworkType.JUnit5) so leaving those
two entries causes duplicate JUnit classes on the test classpath—locate and
remove the lines that call testImplementation and testRuntimeOnly for junit5 to
rely on the IDE plugin-managed JUnit setup.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
sdk/intellij/psi/imagevector/api/imagevector.apisdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/ImageVectorPsiParser.ktsdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/parser/MaterialImageVectorPsiParser.ktsdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/parser/RegularImageVectorPsiParser.kttools/idea-plugin/CHANGELOG.mdtools/idea-plugin/build.gradle.ktstools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorIcon.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/icon/ImageVectorIconProvider.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/util/ImageVectorPsiUtil.kttools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterTest.kt
🚧 Files skipped from review as they are similar to previous changes (4)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/completion/ImageVectorIcon.kt
- sdk/intellij/psi/imagevector/src/main/kotlin/io/github/composegears/valkyrie/sdk/intellij/psi/imagevector/parser/RegularImageVectorPsiParser.kt
- tools/idea-plugin/CHANGELOG.md
- tools/idea-plugin/src/test/kotlin/io/github/composegears/valkyrie/gutter/ImageVectorGutterTest.kt
t-regbs
approved these changes
Feb 25, 2026
egorikftp
force-pushed
the
feature/idea/gutter
branch
from
4167455 to
31a199b
Compare
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
2 participants
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.
📝 Changelog
If this PR introduces user-facing changes, please update the relevant Unreleased section in changelogs: