Introduce CodepointParser with json and css implementation#918
Merged
Introduce CodepointParser with json and css implementation#918
Conversation
egorikftp
force-pushed
the
task/introduce-codepoint_parser
branch
from
6c3c871 to
2f82dda
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdds a new 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 1
🧹 Nitpick comments (3)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/common/data/CodepointParser.kt (1)
3-8: KDoc mentions CSS but interface also handles JSON.The documentation states "extract icon name -> codepoint mapping from a CSS text" but
BootstrapCodepointParserparses JSON. Consider updating the KDoc to be more generic (e.g., "from text input" or "from provider-specific format").📝 Suggested KDoc update
/** - * A strategy that can extract icon name -> codepoint mapping from a CSS text. - * Different icon providers can provide their own implementations (only differing by regex). + * A strategy that extracts icon name -> codepoint mapping from text input. + * Different icon providers can provide their own implementations based on their format (CSS, JSON, etc.). */ interface CodepointParser {🤖 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/ui/screen/webimport/standard/common/data/CodepointParser.kt` around lines 3 - 8, Update the KDoc for the CodepointParser interface to be format-agnostic: replace the phrase about "CSS text" with a generic description such as "text input" or "provider-specific format" so it accurately reflects implementations like BootstrapCodepointParser that parse JSON; reference the interface name CodepointParser and its parse(text: String): Map<String, Int> method in the doc so consumers know it accepts arbitrary provider-specific text formats.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/data/LucideCodepointParser.kt (1)
5-8: Semicolon is required but may be optional in CSS.The regex requires a semicolon before the closing brace (
;\s*}), whileRemixCodepointParserandBoxCodepointParsermake it optional (;?\s*}). CSS allows omitting the trailing semicolon for the last declaration in a block. Consider making it optional for robustness.♻️ Suggested fix
class LucideCodepointParser : RegexCssCodepointParser( - Regex("""\.icon-([a-z0-9-]+)::?before\s*\{\s*content:\s*["']\\([a-fA-F0-9]+)["'];\s*}"""), + Regex("""\.icon-([a-z0-9-]+)::?before\s*\{\s*content:\s*["']\\([a-fA-F0-9]+)["'];?\s*}"""), )🤖 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/ui/screen/webimport/standard/lucide/data/LucideCodepointParser.kt` around lines 5 - 8, The regex in LucideCodepointParser (which extends RegexCssCodepointParser) currently requires a trailing semicolon before the closing brace in the pattern /\.icon-([a-z0-9-]+)::?before\s*\{\s*content:\s*["']\\([a-fA-F0-9]+)["'];\s*}/ — change it to make the semicolon optional (i.e., `;?`) like in RemixCodepointParser and BoxCodepointParser so that CSS blocks that omit the final semicolon still match; update the Regex passed to RegexCssCodepointParser in LucideCodepointParser accordingly.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/data/BoxIconsRepository.kt (1)
16-18: Minor formatting nit: extra blank line in constructor.There's an unnecessary blank line between the
codepointParserparameter and the closing parenthesis.Suggested fix
class BoxIconsRepository( private val httpClient: HttpClient, private val codepointParser: CodepointParser, - ) {🤖 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/ui/screen/webimport/standard/boxicons/data/BoxIconsRepository.kt` around lines 16 - 18, Constructor formatting in BoxIconsRepository contains an extra blank line after the codepointParser parameter; remove the blank line so the primary constructor parameter list closes immediately after the codepointParser: CodepointParser, ) => ensure the parameter list ends with ") {" directly following the last parameter to keep consistent Kotlin formatting for the BoxIconsRepository constructor.
🤖 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/ui/screen/webimport/standard/boxicons/data/BoxCodepointParser.kt`:
- Around line 5-8: BoxCodepointParser's Regex passed to RegexCssCodepointParser
currently only matches double quotes in the content value; update the pattern in
the Regex(...) argument inside class BoxCodepointParser to accept both single
and double quotes (use a character class like ["'] where the current
double-quote is used) so it behaves consistently with RemixCodepointParser and
LucideCodepointParser and remains robust if BoxIcons ever uses single quotes.
---
Nitpick comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/data/BoxIconsRepository.kt`:
- Around line 16-18: Constructor formatting in BoxIconsRepository contains an
extra blank line after the codepointParser parameter; remove the blank line so
the primary constructor parameter list closes immediately after the
codepointParser: CodepointParser, ) => ensure the parameter list ends with ") {"
directly following the last parameter to keep consistent Kotlin formatting for
the BoxIconsRepository constructor.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/common/data/CodepointParser.kt`:
- Around line 3-8: Update the KDoc for the CodepointParser interface to be
format-agnostic: replace the phrase about "CSS text" with a generic description
such as "text input" or "provider-specific format" so it accurately reflects
implementations like BootstrapCodepointParser that parse JSON; reference the
interface name CodepointParser and its parse(text: String): Map<String, Int>
method in the doc so consumers know it accepts arbitrary provider-specific text
formats.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/data/LucideCodepointParser.kt`:
- Around line 5-8: The regex in LucideCodepointParser (which extends
RegexCssCodepointParser) currently requires a trailing semicolon before the
closing brace in the pattern
/\.icon-([a-z0-9-]+)::?before\s*\{\s*content:\s*["']\\([a-fA-F0-9]+)["'];\s*}/ —
change it to make the semicolon optional (i.e., `;?`) like in
RemixCodepointParser and BoxCodepointParser so that CSS blocks that omit the
final semicolon still match; update the Regex passed to RegexCssCodepointParser
in LucideCodepointParser accordingly.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/data/BootstrapCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/data/BootstrapRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/di/BootstrapModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/data/BoxCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/data/BoxIconsRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/di/BoxIconsModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/domain/BoxIconsUseCase.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/common/data/CodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/common/data/RegexCssCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/data/LucideCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/data/LucideRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/di/LucideModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/data/RemixCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/data/RemixRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/di/RemixModule.kt
egorikftp
force-pushed
the
task/introduce-codepoint_parser
branch
from
2f82dda to
c0b3c82
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/data/RemixRepository.kt (1)
40-45: LGTM!The refactoring to use the injected
codepointParseris clean and aligns well with the abstraction goal.Optional nit: For consistency with the font loading pattern (lines 34-36), you could wrap the CPU-bound parsing in
Dispatchers.Default:♻️ Optional: Wrap parsing in Default dispatcher
private val codepoints = suspendLazy { withContext(Dispatchers.IO) { val cssText = httpClient.get(CSS_URL).bodyAsText() - codepointParser.parse(cssText) + withContext(Dispatchers.Default) { + codepointParser.parse(cssText) + } } }🤖 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/ui/screen/webimport/standard/remix/data/RemixRepository.kt` around lines 40 - 45, The current code in the suspendLazy initializer for the codepoints variable performs network I/O and CPU-bound parsing both on Dispatchers.IO; change it to perform the httpClient.get(CSS_URL).bodyAsText() call inside withContext(Dispatchers.IO) and then switch to withContext(Dispatchers.Default) for codepointParser.parse(cssText) so the CPU-bound parse runs on the Default dispatcher, referencing the codepoints suspendLazy block, httpClient.get, and codepointParser.parse to locate the changes.tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/domain/BoxIconsUseCase.kt (1)
33-46: Consider stabilizing icon order after switching toMap.
loadConfig()now maps directly overMapentries. Sorting by key here would make generated config ordering deterministic across parser/map implementations.♻️ Suggested change
- val icons = repository.loadCodepoints().map { entry -> - val style = entry.key.toStyle() - val displayName = entry.key.toDisplayNameWithoutPrefix(style) + val icons = repository.loadCodepoints() + .toSortedMap() + .map { (name, codepoint) -> + val style = name.toStyle() + val displayName = name.toDisplayNameWithoutPrefix(style) StandardIcon( - name = entry.key, + name = name, displayName = displayName, - exportName = entry.key.toExportName(), - codepoint = entry.value, + exportName = name.toExportName(), + codepoint = codepoint, tags = emptyList(), - category = inferCategoryFromTags(entry.key, emptyList()), + category = inferCategoryFromTags(name, emptyList()), style = style.value, ) - } + }🤖 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/ui/screen/webimport/standard/boxicons/domain/BoxIconsUseCase.kt` around lines 33 - 46, The map iteration over repository.loadCodepoints() can produce nondeterministic ordering; before mapping to StandardIcon in BoxIconsUseCase (where repository.loadCodepoints(), StandardIcon(...), and inferCategoryFromTags(...) are used), sort the entries by their key (e.g., entry.key) to stabilize generated config order across parser/map implementations; update the pipeline to order entries deterministically (by key) then map to StandardIcon so exportName/displayName/codepoint/category remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/domain/BoxIconsUseCase.kt`:
- Around line 33-46: The map iteration over repository.loadCodepoints() can
produce nondeterministic ordering; before mapping to StandardIcon in
BoxIconsUseCase (where repository.loadCodepoints(), StandardIcon(...), and
inferCategoryFromTags(...) are used), sort the entries by their key (e.g.,
entry.key) to stabilize generated config order across parser/map
implementations; update the pipeline to order entries deterministically (by key)
then map to StandardIcon so exportName/displayName/codepoint/category remain
consistent.
In
`@tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/data/RemixRepository.kt`:
- Around line 40-45: The current code in the suspendLazy initializer for the
codepoints variable performs network I/O and CPU-bound parsing both on
Dispatchers.IO; change it to perform the httpClient.get(CSS_URL).bodyAsText()
call inside withContext(Dispatchers.IO) and then switch to
withContext(Dispatchers.Default) for codepointParser.parse(cssText) so the
CPU-bound parse runs on the Default dispatcher, referencing the codepoints
suspendLazy block, httpClient.get, and codepointParser.parse to locate the
changes.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/data/BootstrapCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/data/BootstrapRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/di/BootstrapModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/data/BoxCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/data/BoxIconsRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/di/BoxIconsModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/domain/BoxIconsUseCase.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/common/data/CodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/common/data/RegexCssCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/data/LucideCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/data/LucideRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/di/LucideModule.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/data/RemixCodepointParser.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/data/RemixRepository.kttools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/di/RemixModule.kt
🚧 Files skipped from review as they are similar to previous changes (9)
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/common/data/RegexCssCodepointParser.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/data/RemixCodepointParser.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/remix/di/RemixModule.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/data/LucideCodepointParser.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/common/data/CodepointParser.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/di/BoxIconsModule.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/boxicons/data/BoxCodepointParser.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/lucide/data/LucideRepository.kt
- tools/idea-plugin/src/main/kotlin/io/github/composegears/valkyrie/ui/screen/webimport/standard/bootstrap/data/BootstrapCodepointParser.kt
t-regbs
approved these changes
Feb 28, 2026
4 tasks
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.
Make a clear abstraction
📝 Changelog
If this PR introduces user-facing changes, please update the relevant Unreleased section in changelogs: