Add support for <style> tags
#99
Conversation
WalkthroughThis pull request introduces a comprehensive enhancement to the SVG-to-Compose project's parsing and lexing capabilities, focusing on CSS parsing, token management, and SVG node processing. The changes span multiple files and introduce new classes and interfaces for handling CSS tokens, rules, and selectors. A key addition is the implementation of a robust CSS parser that can handle complex CSS syntax, including at-rules, selectors, and declarations. The project also adds support for computing CSS specificity and integrating computed styles into SVG node rendering. Changes
Sequence DiagramsequenceDiagram
participant SVGParser
participant CSSTokenizer
participant CSSParser
participant StyleSheetConsumer
SVGParser->>CSSTokenizer: Tokenize CSS content
CSSTokenizer-->>SVGParser: Return tokens
SVGParser->>CSSParser: Parse tokens
CSSParser->>StyleSheetConsumer: Consume tokens
StyleSheetConsumer-->>CSSParser: Generate StyleSheet
SVGParser->>SVGParser: Apply computed styles
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (28)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/CssConsumer.kt (3)
8-11: Add KDoc documentation for the class.Consider adding documentation to describe:
- The purpose and responsibility of this consumer
- The type parameter
TElement- The constructor parameters
- Usage guidelines for implementers
+/** + * Base consumer for CSS parsing that processes tokens into specific CSS elements. + * + * @param TElement The specific type of CSS element this consumer produces + * @property content The raw CSS content being parsed + * @property parser The CSS AST parser instance + */ internal abstract class CssConsumer<out TElement : CssElement>( protected val content: String, protected val parser: CssAstParser, )
8-11: Consider adding constructor validation.The constructor parameters should be validated to ensure they're not empty/blank.
internal abstract class CssConsumer<out TElement : CssElement>( protected val content: String, protected val parser: CssAstParser, -) { +) { + init { + require(content.isNotBlank()) { "Content cannot be blank" } + requireNotNull(parser) { "Parser cannot be null" } + }
12-12: Add documentation for the abstract method with error handling guidance.The
consumemethod should be documented with:
- Expected behavior
- Error handling requirements
- Return value specifications
+ /** + * Consumes a CSS token and produces a corresponding CSS element. + * + * @param token The CSS token to consume + * @return The parsed CSS element + * @throws IllegalStateException if the token is invalid or unexpected + */ abstract fun consume(token: Token<out CssTokenKind>): TElementsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/selectors/AggregateSelectorParser.kt (2)
10-19: Consider enhancing documentation with error handling detailsThe KDoc is well-written but could be more comprehensive by documenting the behavior when parsing fails or when invalid combinations are encountered.
Add details about error handling:
/** * Creates an aggregate selector parser, which parses multiple selectors separated by combinators. * * This function is responsible for parsing selectors that are combined together, * such as `div > p` or `a.link + span`. * + * If parsing fails for any selector in the chain, the function will stop and return + * the successfully parsed selectors up to that point. * * @param iterator The iterator to use for parsing. * @param initiator The initial selector to start with. * @return A [CssComponent.Multiple] representing the aggregate selector. */
20-44: Consider several improvements to the implementationThe implementation is functional but could benefit from the following improvements:
- Replace
while(true)with a more explicit condition- Preserve combinator information for better CSS representation
- Add logging for debugging failed parsing scenarios
- Add input validation for the initiator parameter
Here's a suggested improvement:
internal fun SelectorParser.createAggregateSelectorParser( iterator: AstParserIterator<CssTokenKind, CssRootNode>, initiator: CssComponent, ): CssComponent.Multiple { + require(initiator != CssComponent.None) { "Initiator must be a valid selector" } + val selectors = mutableListOf(initiator) - while (true) { + while (!iterator.isAtEnd()) { var next = iterator.next() - if (CssCombinator.from(next?.kind) != null) + val combinator = CssCombinator.from(next?.kind) + if (combinator != null) { next = iterator.next() + } if (next == null || next.kind in terminalTokens) { iterator.rewind() break } val selector = parse(next) if (selector != null) { + // Store the combinator information if needed for future use + // selectors += CombinedSelector(selector, combinator) selectors += selector } else { + logger.debug { "Failed to parse selector at token: $next" } break } } return CssComponent.Multiple(components = selectors) }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/PropertyParser.kt (3)
113-176: Consider refactoringparseColorValueto reduce duplicationThe
parseColorValuemethod contains similar logic for handling hash colors and identifier colors. Refactoring this method by extracting common code or creating helper functions could improve maintainability and readability.
228-237: Consider generalizing value extraction inparseStringValueandparseIdentifierValueThe methods
parseStringValueandparseIdentifierValueshare a similar pattern of checking the token kind and extracting the value from the content. Creating a generic method to handle this pattern can reduce code duplication and enhance readability.Also applies to: 247-256
266-279: Clarify documentation for numerical value parsing methodsThe documentation for
parseNumberValue,parseDimensionValue, andparsePercentageValuementions parsing numerical values potentially followed by units. However:
parseNumberValuehandles numbers without units.parseDimensionValuehandles numbers with units.parsePercentageValuehandles percentage values.Consider updating the documentation to accurately reflect each function's specific behavior.
Also applies to: 289-302, 312-325
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssElements.kt (1)
35-36: Consider renaming multipleMultipledata classes for clarityHaving multiple data classes named
Multiplein different contexts (CssComponent.Multiple,PropertyValue.Multiple) may cause confusion. Renaming them can improve code readability.Possible renames:
- data class Multiple(val components: List<CssComponent>) : CssComponent + data class CompositeComponent(val components: List<CssComponent>) : CssComponent - data class Multiple(val values: List<PropertyValue>) : PropertyValue + data class CompositeValue(val values: List<PropertyValue>) : PropertyValueAlso applies to: 79-79
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssLexer.kt (2)
16-16: Remove debugging print statementThe
printlnstatement on line 16 appears to be a leftover from debugging and should be removed to avoid unnecessary console output in production code.Apply this diff to remove the debugging statement:
- println("offset: $offset, char: $char")
62-84: Simplify the logic for handling#tokensThe logic for handling the
#character from lines 62-84 is complex and may affect readability. Consider refactoring this section to make it clearer and more maintainable. Explicitly separating the handling of IDs and hex color values could improve code clarity.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Token.kt (1)
3-9: LGTM with suggestions for enhancementThe Token data class implementation is clean and type-safe. Consider these enhancements:
- Add input validation to ensure startOffset <= endOffset
- Add toString() implementation for better debugging
- Consider adding a length property
data class Token<T : TokenKind>( val kind: T, // Inclusive val startOffset: Int, // Exclusive val endOffset: Int, +) { + init { + require(startOffset <= endOffset) { + "startOffset ($startOffset) must not be greater than endOffset ($endOffset)" + } + } + + val length: Int get() = endOffset - startOffset + + override fun toString(): String = "Token(kind=$kind, range=$startOffset..$endOffset)" }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt (1)
3-9: Add KDoc documentation for better API understandingThe TokenKind interface is well-designed but would benefit from comprehensive documentation explaining its purpose and usage.
+/** + * Represents a type of token in the lexical analysis process. + * + * @property representation The string representation of this token kind + * + * Example usage: + * ``` + * enum class MyTokenKind(override val representation: String) : TokenKind { + * NUMBER("0123456789"), + * OPERATOR("+-*/") + * } + * ``` + */ interface TokenKind { val representation: String + /** + * Checks if the given character belongs to this token kind's representation. + * + * @param other The character to check + * @return true if the character is part of this token kind's representation + */ operator fun contains(other: Char): Boolean { return other in representation } }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/AstParser.kt (1)
20-20: Document potential exceptions in parse methodThe
parsemethod should document possible exceptions that might occur during parsing.- fun parse(tokens: List<Token<out TTokenKind>>): TAstNode + /** + * Parses a list of tokens into an AST node. + * + * @param tokens The list of tokens to parse. + * @return The root node of the parsed AST. + * @throws IllegalStateException if the tokens form an invalid syntax + * @throws IllegalArgumentException if the token list is empty + */ + fun parse(tokens: List<Token<out TTokenKind>>): TAstNodesvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt (3)
54-62: Consider validating color function parametersThe
Colortoken'scolorIdentifiersset should include validation for the parameters of these color functions (e.g., valid ranges for RGB values).Consider creating a companion object with validation methods:
companion object { fun isValidRgbValue(value: Int) = value in 0..255 fun isValidHslHue(value: Float) = value in 0f..360f fun isValidHslSaturationOrLightness(value: Float) = value in 0f..100f }
48-52: Group related tokens using sealed interfacesConsider grouping related tokens (like numeric types) under sealed interfaces for better organization.
+ sealed interface NumericToken : CssTokenKind + - data object Number : CssTokenKind(representation = "") - data object Dimension : CssTokenKind(representation = "px|em|rem|cm|mm|in|pt|pc|vh|vw|vmin|vmax|ch|ex") - data object Percentage : CssTokenKind(representation = "%") - data object HexDigit : CssTokenKind(representation = "") + data object Number : NumericToken(representation = "") + data object Dimension : NumericToken(representation = "px|em|rem|cm|mm|in|pt|pc|vh|vw|vmin|vmax|ch|ex") + data object Percentage : NumericToken(representation = "%") + data object HexDigit : NumericToken(representation = "")
49-49: Consider using an enum for CSS unitsThe Dimension token's representation contains a hard-coded list of CSS units. Consider using an enum to make it more maintainable.
enum class CssUnit(val value: String) { PIXEL("px"), EM("em"), REM("rem"), // ... other units ; companion object { val representation = values().joinToString("|") { it.value } } } data object Dimension : CssTokenKind(representation = CssUnit.representation)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt (1)
Line range hint
1-481: LGTM! Well-structured test suite with comprehensive coverage.The test suite effectively covers CSS specificity calculations with clear test cases and good use of the AAA pattern.
Consider these improvements:
- Add KDoc comments explaining CSS specificity calculation rules
- Add test cases for invalid selectors
- Consider combining similar test cases using @burst annotation
Example KDoc:
/** * Tests for CSS specificity calculations. * * Specificity is calculated as a tuple of three numbers (a,b,c) where: * - a = number of ID selectors * - b = number of class selectors * - c = number of type selectors * * The higher the number in each position, the higher the specificity. */ class CssSpecificityTest {svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssLexerTest.kt (1)
476-481: Consider extracting common token sequences to reduce duplication.The
asserthelper method is good, but the test cases contain a lot of duplicated token lists.Consider creating helper methods for common token sequences:
private fun createWhitespaceToken(start: Int, end: Int) = Token(kind = CssTokenKind.WhiteSpace, startOffset = start, endOffset = end) private fun createIdentifierToken(value: String, start: Int) = Token(kind = CssTokenKind.Identifier, startOffset = start, endOffset = start + value.length) private fun createBasicRuleTokens(selector: String, property: String, value: String): List<Token<CssTokenKind>> { var offset = 0 return buildList { add(createIdentifierToken(selector, offset)) offset += selector.length add(createWhitespaceToken(offset, offset + 1)) // ... continue building common token sequence } }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/selectors/SelectorParser.kt (2)
32-47: Refactor duplicated logic in selector parsingThe code segments in lines 32-47 and 59-72 perform similar tasks for creating selectors and handling combinators. Refactoring this duplicated logic into a shared method would enhance maintainability and reduce code redundancy.
Also applies to: 59-72
49-56: Consider using a custom exception for parsing errorsCurrently, the code uses
error(buildErrorMessage(...))to throw anIllegalStateExceptionupon encountering unexpected tokens. Using a custom exception class for parsing errors can provide more specific error information and improve error handling throughout the parser.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt (1)
79-85: Return an empty list instead ofnullfor unsupported node typesIn the
asNodesfunction, returningnullfor unsupported node types may lead to unnecessary null checks and potentialNullPointerExceptions. Consider returning an empty list (emptyList()) to simplify handling and improve code safety.Apply this diff to implement the change:
-): List<ImageVectorNode>? = when (this) { +): List<ImageVectorNode> = when (this) { is SvgRootNode -> asNodes(minified = minified) is SvgGroupNode -> flatNode(masks, minified) is SvgCircleNode -> listOf(asNode(minified = minified)) is SvgPathNode -> listOf(asNode(minified = minified)) is SvgRectNode -> listOf(asNode(minified = minified)) is SvgPolygonNode -> listOf(asNode(minified = minified)) is SvgPolylineNode -> listOf(asNode(minified = minified)) is SvgEllipseNode -> listOf(asNode(minified = minified)) - else -> null + else -> emptyList() }This change ensures the function consistently returns a list, eliminating the need for null checks.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/Collections.extension.kt (1)
6-7: Simplify thefirstInstanceOffunction implementationThe current implementation uses
firstOrNullfollowed by a safe cast and throws aNoSuchElementExceptionif no element is found. Since you're filtering for elements of typeT, you can simplify the function by usingfirstand casting directly.Apply this diff to simplify the function:
inline fun <reified T> Iterable<*>.firstInstanceOf(): T = - firstOrNull { it is T } as? T ?: throw NoSuchElementException("No element of type ${T::class.simpleName} found.") + first { it is T } as TThis leverages the standard
firstfunction, which throwsNoSuchElementExceptionby default if no matching element is found, and avoids the unnecessary safe cast.buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.testing.gradle.kts (1)
9-9: Consider adding Burst plugin configuration.The Burst plugin is added but lacks configuration. Consider adding configuration to customize test execution if needed.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt (3)
22-22: Consider removing debug println statements.The println statements used for debugging should be removed or replaced with proper test logging mechanisms.
- println("viewBox = $viewBox, expected = $expected")Also applies to: 46-46, 70-70
14-19: Consider adding more edge cases to test parameters.The test cases could benefit from additional edge cases such as:
- Very large numbers
- Scientific notation
- Whitespace variations
- Unicode spaces
Also applies to: 37-43, 61-67, 85-89
140-142: Improve comment clarity for memoization test.The current comment could be more explicit about why instance equality is being tested.
- // We use equals in this situation since we want to check if - // the viewBox instance is the same. + // Using assertEquals instead of assertContentEquals to verify + // that the same FloatArray instance is returned (memoization), + // not just an array with the same content.svg-to-compose/src/nativeMain/kotlin/Main.kt (1)
151-161: Enhance help message formatting.The help message for mapIconNameTo could be more readable with proper markdown formatting.
help = """Replace the icon's name first value of this parameter with the second. |This is useful when you want to remove part of the icon's name from the output icon. | - |For example, running the following command: - |``` - | s2c <args> \ - | -o ./my-app/src/my/pkg/icons \ - | -rt Icons.Filled \ - | --map-icon-name-from-to "_filled" "" - | ./my-app/assets/svgs - |``` + |**Example:** + |```bash + |s2c <args> \\ + | -o ./my-app/src/my/pkg/icons \\ + | -rt Icons.Filled \\ + | --map-icon-name-from-to "_filled" "" \\ + | ./my-app/assets/svgs + |``` | |An icon named `bright_sun_filled.svg` will create a `ImageVector` named `BrightSun`. """.trimMargin(),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (2)
.run/svg-to-compose [allTests].run.xmlis excluded by!**/.run/**config/detekt.ymlis excluded by!**/config/detekt.yml
📒 Files selected for processing (32)
buildSrc/build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.testing.gradle.kts(1 hunks)gradle/libs.versions.toml(2 hunks)svg-to-compose/build.gradle.kts(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRootNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/Collections.extension.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Lexer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Token.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssLexer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/AstParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/Element.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssElements.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/CssConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/PseudoClassConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssElementParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificity.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/IdentifierElementParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/PropertyParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/selectors/AggregateSelectorParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/selectors/SelectorParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssLexerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt(1 hunks)svg-to-compose/src/jvmMain/kotlin/Main.kt(1 hunks)svg-to-compose/src/nativeMain/kotlin/Main.kt(5 hunks)
✅ Files skipped from review due to trivial changes (1)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/Element.kt
🔇 Additional comments (17)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/selectors/AggregateSelectorParser.kt (1)
1-9: LGTM: Well-organized package structure and imports
The package declaration and imports follow Kotlin conventions and are properly organized.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt (1)
1-45: Implementation of AtRuleConsumer is correct and well-structured
The AtRuleConsumer class correctly parses CSS at-rules with appropriate error handling and integration into the parsing framework.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/PseudoClassConsumer.kt (1)
1-64: PseudoClassConsumer correctly handles pseudo-class selectors
The implementation accurately parses pseudo-class selectors, including those with parameters, and properly integrates with the selector parser.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/IdentifierElementParser.kt (1)
17-79: Well-implemented IdentifierElementParser class
The IdentifierElementParser class is well-structured, with appropriate handling of different CSS token kinds and robust error handling. The code is clean, readable, and follows best practices.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParser.kt (1)
29-158: Effective implementation of CssAstParser
The CssAstParser class efficiently parses CSS tokens into an abstract syntax tree (AST). The logic is sound, and the methods are well-organized, contributing to maintainability and readability.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Lexer.kt (1)
3-5: Interface Lexer is well-defined
The Lexer interface provides a clear and concise contract for tokenization processes, facilitating consistent implementation across different lexer classes.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssElementParser.kt (1)
7-9: 🛠️ Refactor suggestion
Consider adding error handling and documentation
The interface is well-focused but could benefit from:
- Error handling mechanism for parsing failures
- Comprehensive documentation
- Context parameter for parsing state
+/**
+ * Internal parser interface for CSS elements.
+ *
+ * @param T The specific Element type this parser produces
+ */
internal interface CssElementParser<T: Element> {
+ /**
+ * Attempts to parse a CSS element starting from the given token.
+ *
+ * @param starterToken The token that starts this element
+ * @param context The parsing context (optional)
+ * @return The parsed element or null if parsing fails
+ * @throws CssParseException if an unrecoverable error occurs during parsing
+ */
- fun parse(starterToken: Token<out CssTokenKind>): T?
+ fun parse(
+ starterToken: Token<out CssTokenKind>,
+ context: CssParserContext = CssParserContext()
+ ): Result<T>
}
+
+/**
+ * Represents the current state and configuration of the parsing process.
+ */
+class CssParserContext {
+ // Add relevant parsing state and configuration
+}This enhancement:
- Provides better error handling through Result type
- Adds parsing context for state management
- Improves documentation
- Maintains backward compatibility through default parameter
Let's check if other parsers in the codebase follow similar patterns:
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt (1)
22-22: Consider adding parameter validation for steps
The peek and rewind methods accept an Int parameter but don't specify behavior for negative values. Consider adding validation or documenting the behavior for negative steps.
- fun peek(steps: Int = 1): Token<out TTokenKind>?
+ /**
+ * Peeks the next token without consuming it.
+ * @param steps Number of steps to look ahead (must be positive)
+ * @throws IllegalArgumentException if steps is negative
+ */
+ fun peek(steps: Int = 1): Token<out TTokenKind>?
- fun rewind(steps: Int = 1)
+ /**
+ * Rewinds the iterator by the specified number of steps.
+ * @param steps Number of steps to rewind (must be positive)
+ * @throws IllegalArgumentException if steps is negative
+ */
+ fun rewind(steps: Int = 1)Also applies to: 27-27
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/selectors/SelectorParser.kt (2)
13-73: Implementation of SelectorParser is well-structured
The SelectorParser class handles CSS selector parsing effectively, with clear logic for different token kinds and appropriate use of helper methods. The overall structure is coherent and follows good coding practices.
38-42:
Ensure null safety when retrieving combinators
In lines 40 and 68, nextToken?.kind and next?.kind can be null. Passing a null value to CssCombinator.from might lead to unexpected behavior or a NullPointerException. Please ensure that CssCombinator.from gracefully handles null inputs.
Run the following script to verify if CssCombinator.from handles null arguments appropriately:
Also applies to: 67-69
buildSrc/build.gradle.kts (1)
16-16: Verify the necessity and compatibility of the burst Gradle plugin
The libs.app.cash.burst.gradle.plugin has been added to the dependencies. Ensure that this plugin is necessary for your project's requirements and that it's compatible with your existing Gradle setup.
Run the following script to check for plugin compatibility and potential conflicts:
✅ Verification successful
Let me gather more information about the burst plugin usage in the project.
Let me check for actual usage of the burst plugin in the project's build files.
The burst plugin is correctly configured and used in the project
The burst plugin is properly integrated into the project's build system:
- It's defined in the version catalog with version 2.2.0
- It's applied in the Kotlin multiplatform test configuration convention plugin
- The plugin is used alongside power-assert for test configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the compatibility of the burst Gradle plugin with the project's Gradle version.
# Display the current Gradle version
./gradlew --version
# Check for known issues or conflicts with the burst plugin
# (Assuming you have access to the plugin's documentation or issue tracker)
echo "Please verify the burst plugin documentation for compatibility notes."
Length of output: 355
Script:
#!/bin/bash
# Check for burst plugin configuration and usage in the project
echo "Searching for burst plugin configuration..."
rg -l "burst" --type gradle
# Check version catalog for burst plugin version
echo -e "\nChecking version catalog for burst plugin..."
fd "libs.versions.toml" --type f --exec cat {}
# Look for any documentation about burst plugin
echo -e "\nChecking for documentation mentioning burst..."
rg -i "burst" -g "*.md"
Length of output: 5273
Script:
#!/bin/bash
# Search for burst plugin application and configuration
echo "Checking for burst plugin usage in build files..."
rg -A 5 "burst" --type gradle
# Check the specific buildSrc directory for burst-related configurations
echo -e "\nChecking buildSrc directory for burst configurations..."
fd . buildSrc --type f --exec cat {}
Length of output: 16131
buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.testing.gradle.kts (1)
9-9: Verify Burst plugin compatibility.
Let's verify the compatibility of the Burst plugin with the current Kotlin version.
✅ Verification successful
Burst plugin is compatible with current setup
Based on the version catalog (libs.versions.toml), the project uses:
- Burst plugin version: 2.2.0
- Kotlin version: 2.1.0
The Burst plugin 2.2.0 is compatible with Kotlin 2.1.0 as it's a recent version of the plugin that supports modern Kotlin versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if Burst plugin version is compatible with current Kotlin version
rg -A 1 "kotlin\s*=\s*\"[^\"]*\"" | grep -A 1 "burst\s*=\s*\"[^\"]*\""
Length of output: 66
Script:
#!/bin/bash
# Let's first check the Kotlin version being used
rg "kotlin\s*=\s*\"[^\"]*\"" --type kotlin
# Check for Burst plugin version
rg "burst\s*=\s*\"[^\"]*\"" --type kotlin
# Check build.gradle files for version declarations
fd "build.gradle" --exec cat {}
# Check gradle.properties for version declarations
fd "gradle.properties" --exec cat {}
# Check for any version catalogs
fd "libs.versions.toml" --exec cat {}
Length of output: 19642
svg-to-compose/build.gradle.kts (1)
22-23: Verify necessity of Clikt markdown dependency.
The addition of Clikt markdown support seems unrelated to the PR's objective of adding <style> tag support. Please clarify the purpose of this dependency.
✅ Verification successful
Let me check if there are any actual usages of markdown features in the Kotlin code.
Let me check the CLI command implementations to see if there's any planned usage of markdown features.
Let me check for any CLI-related files that might use help documentation.
The Clikt markdown dependency is justified.
Looking at the code, the markdown dependency is actively used in the CLI's help formatter configuration through MordantMarkdownHelpFormatter. This is used to provide formatted help documentation for the command-line interface, as seen in the Client class initialization:
context {
helpFormatter = { context ->
MordantMarkdownHelpFormatter(
context = context,
showDefaultValues = true,
requiredOptionMarker = "*",
)
}
}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if markdown features are used in the codebase
rg -l "clikt.*markdown" --type kotlin
Length of output: 71
Script:
#!/bin/bash
# Search for markdown-related imports or usage in Kotlin files
rg "markdown" --type kotlin -g '!*.gradle.kts' -A 3
Length of output: 51
Script:
#!/bin/bash
# Find main CLI command files and check their content
fd -e kt "Command|Cli" --type f --exec cat {}
Length of output: 14810
Script:
#!/bin/bash
# Search for main CLI entry points and help-related code
fd "Main.kt|Cli.kt" --type f --exec cat {}
Length of output: 28676
gradle/libs.versions.toml (2)
15-16: LGTM: Clikt dependency organization.
The Clikt dependencies are well organized using the module notation, which is the recommended approach.
3-3: Verify Burst version compatibility.
The Burst library version 2.2.0 is added. Let's verify if this is the latest stable version and check for any known issues.
Also applies to: 21-21
✅ Verification successful
Burst version 2.2.0 is up to date
The specified Burst library version 2.2.0 matches the latest stable release available in the official repository.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest version of Burst library
curl -s "https://api.github.com/repos/cashapp/burst/releases/latest" | jq -r '.tag_name'
Length of output: 92
svg-to-compose/src/nativeMain/kotlin/Main.kt (1)
41-49: Good addition of structured help formatting!
The addition of MordantMarkdownHelpFormatter with showDefaultValues and requiredOptionMarker improves CLI usability.
svg-to-compose/src/jvmMain/kotlin/Main.kt (1)
22-22: Package naming convention change looks good.
The update from dot notation to underscore notation in package names is consistent with Kotlin's recommended naming conventions for multiplatform projects.
Also applies to: 28-28
...se/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/selectors/AggregateSelectorParser.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/PropertyParser.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssElements.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificity.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssLexerTest.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRootNode.kt
Show resolved
Hide resolved
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.kmp.gradle.kts (1)
17-19: Document the usage of experimental compiler featureConsider adding a comment explaining why this experimental feature is needed, particularly its role in CSS parsing implementation.
kotlin { useNative() useJvm() + // Enable when-guards for improved CSS selector pattern matching + // Note: This is an experimental Kotlin compiler feature compilerOptions { freeCompilerArgs.add("-Xwhen-guards") } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
config/detekt.ymlis excluded by!**/config/detekt.yml
📒 Files selected for processing (1)
buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.kmp.gradle.kts(1 hunks)
buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.kmp.gradle.kts
Outdated
Show resolved
Hide resolved
rafaeltonholo
force-pushed
the
feat/add-support-style-tag
branch
from
835c608 to
689ecec
Compare
There was a problem hiding this comment.
Actionable comments posted: 39
🧹 Outside diff range and nitpick comments (60)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt (3)
6-11: Enhance interface documentation for better clarity.While the basic documentation is present, consider adding:
- A more detailed description of the interface's role in AST parsing
- Usage examples
- Explanation of the relationship with Token and TokenKind
Here's a suggested improvement:
/** - * An iterator for the AST parser. + * An iterator for traversing tokens during AST parsing, providing forward and backward + * navigation capabilities. This interface allows for controlled token consumption with + * look-ahead and rewind functionality. * + * Example usage: + * ``` + * val iterator: AstParserIterator<MyTokenKind> = ... + * while (iterator.hasNext()) { + * val nextToken = iterator.peek() // Look ahead without consuming + * if (shouldProcess(nextToken)) { + * processToken(iterator.next()) // Consume the token + * } else { + * break + * } + * } + * ``` * * @param TTokenKind The type of token kind. */
12-36: Consider enhancing method specifications for edge cases.The interface methods would benefit from more detailed documentation regarding:
- Return type documentation for all methods
- Validation requirements for negative step values
- Behavior specification when steps exceed available tokens
Here's a suggested improvement for the method documentation:
fun hasNext(): Boolean /** * Returns the next token in the iteration. + * + * @return The next token in the sequence, or null if no more tokens are available. */ fun next(): Token<out TTokenKind>? /** * Peeks the next token without consuming it. + * + * @param steps The number of tokens to look ahead (positive) or behind (negative). + * Must not exceed the available tokens in either direction. + * @return The token at the specified position, or null if the position is out of bounds. + * @throws IllegalArgumentException if steps is zero. */ fun peek(steps: Int = 1): Token<out TTokenKind>? /** * Returns the current token, which is the token that was previously consumed. * This is equivalent to peeking one step back. * * @return The current token, or null if there is no current token. */ fun current(): Token<out TTokenKind>? = peek(steps = -1) /** * Rewinds the iterator by the specified number of steps. + * + * @param steps The number of steps to rewind. Must be positive and not exceed + * the number of consumed tokens. + * @throws IllegalArgumentException if steps is negative or zero. + * @throws IllegalStateException if attempting to rewind beyond the start of the token stream. */ fun rewind(steps: Int = 1)
1-36: Consider adding companion interfaces for specialized parsing scenarios.The current interface provides a solid foundation for basic token iteration. Consider extending the architecture to support:
- A specialized interface for error recovery during parsing
- A marker interface for tracking significant parsing positions
- A companion interface for batch token processing
This would make the parsing system more robust and maintainable as the project grows.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt (2)
6-12: Consider enhancing KDoc documentation for the toString parameter.While the interface is well-documented, the
toString(indent: Int)method's parameter could benefit from documentation explaining its purpose and usage./** * The base interface for all CSS nodes. + * + * @property location The location of this node in the source code + * @param indent The number of spaces to indent the string representation */
26-49: Consider enhancing documentation and validation.While the implementation is solid, consider these improvements:
- Add property documentation
- Consider adding range validation for start/end positions
- Document the Undefined constant's purpose
/** * Represents a location in a CSS source file. + * + * @property source The source text containing this location + * @property start The starting position in the source (inclusive) + * @property end The ending position in the source (exclusive) */ data class CssLocation( val source: String, val start: Int, val end: Int, ) { + init { + require(start <= end || (start == -1 && end == -1)) { + "Start position ($start) must not be greater than end position ($end)" + } + } // ... rest of the implementation ... companion object { + /** + * Represents an undefined location, used as a sentinel value + * when a location cannot be determined. + */ val Undefined = CssLocation(svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt (3)
18-101: Consider removing debug print statement and adding test documentationThe test case is well-structured, but has two minor improvements to consider:
- The
println(actual)statement on line 99 appears to be for debugging purposes and should be removed.- Consider adding KDoc documentation to describe the test's purpose and expected behavior.
+ /** + * Tests the construction of a CSS tree with a pseudo-element selector (::after) + * and complex content declaration using the attr() function. + */ @Test fun `when pseudo-element is present should build a valid css tree`() { val location = CssLocation( source = "", start = 0, end = 0 ) val rule = QualifiedRule( // ... existing code ... ) val expected = // ... existing code ... val actual = rule.toString(indent = 0) - println(actual) assertEquals(expected, actual) }
103-263: Consider refactoring test setup and removing debug printThe test case is comprehensive but could benefit from some improvements:
- The
CssLocationobject is repetitive across tests. Consider extracting it to a companion object or test fixture.- The
println(actual)statement on line 261 should be removed.class CssTreeTest { + companion object { + private val DEFAULT_LOCATION = CssLocation( + source = "", + start = 0, + end = 0 + ) + } + // ... other tests ... @Test fun `when at-rule is present should build a valid css tree`() { - val location = CssLocation( - source = "", - start = 0, - end = 0 - ) + val location = DEFAULT_LOCATION val rule = AtRule( // ... existing code ... ) val expected = // ... existing code ... val actual = rule.toString(indent = 0) - println(actual) assertEquals(expected, actual) }
355-422: Maintain consistent test structureThe test structure differs from other tests by declaring the expected result before constructing the rule. Consider maintaining consistency with other tests by:
- Following the same pattern: rule construction → expected string → assertion
- Removing the
println(actual)statement on line 419@Test fun `when attribute selector is present should build a valid css tree`() { val location = CssLocation( source = "", start = 0, end = 0, ) - val expected = """ - |input[type="password"]:not(:valid) { - | color: red; - |} - """.trimMargin() - val rule = QualifiedRule( // ... existing code ... ) + val expected = """ + |input[type="password"]:not(:valid) { + | color: red; + |} + """.trimMargin() val actual = rule.toString(indent = 0) - println(actual) assertEquals(expected, actual) }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt (3)
7-22: Consider optimizing the regex pattern implementation.While the implementation is correct, consider these performance improvements:
- Move the regex pattern to a companion object to avoid recompilation on each call.
- Use word boundaries for more precise matching.
+ private companion object { + private val DIVIDER_PATTERN = "([_\\-. ])\\b[a-zA-Z0-9]".toRegex() + } private fun String.replaceDividers(): String { - val pattern = "([_\\-. ])[a-zA-Z0-9]".toRegex() - return replace(pattern) { it.value.last().uppercase() } + return replace(DIVIDER_PATTERN) { it.value.last().uppercase() } }
Line range hint
90-100: Consider enhancing error handling in toPercentage.While the implementation is functional, consider providing more descriptive error messages and handling edge cases:
- Negative percentages
- Invalid number formats
- Malformed percentage strings (e.g., "50%%" or "%50")
fun String.toPercentage(): Float { val value = this.toFloatOrNull() return when { - value == null && this.endsWith("%") -> this.removeSuffix("%").toInt() / PERCENT + value == null && this.endsWith("%") -> try { + this.removeSuffix("%").toInt() / PERCENT + } catch (e: NumberFormatException) { + error("Invalid percentage format: '$this'. Expected format: '50%' or '0.5'") + } value != null -> value - else -> error("Invalid percentage value '$this'") + else -> error("Invalid percentage value '$this'. Expected format: '50%' or '0.5'") }.coerceIn(range = 0f..1f) }
102-105: Consider optimizing the regex pattern.Move the regex pattern to a companion object to avoid recompilation on each call.
+private companion object { + private val TRAILING_ZERO_PATTERN = "\\.0\\b".toRegex() +} -inline fun String.removeTrailingZero(): String = replace("\\.0\\b".toRegex(), "") +inline fun String.removeTrailingZero(): String = replace(TRAILING_ZERO_PATTERN, "")svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt (2)
15-16: Document the magic number in the offset calculationThe
-4in the offset calculation appears to account for the "url(" prefix, but this should be documented for maintainability.- val start = iterator.offset - 4 + // Subtract 4 to account for the "url(" prefix length + val start = iterator.offset - 4
1-6: Add class-level documentationConsider adding KDoc documentation to explain:
- The class's role in the CSS parsing pipeline
- The URL token format it handles
- Links to relevant CSS specification sections
+/** + * Consumes URL tokens according to CSS Syntax Level 3 § 4.3.6. + * @see <a href="https://www.w3.org/TR/css-syntax-3/#consume-url-token">CSS Syntax Level 3</a> + */ internal class UrlTokenConsumer( iterator: TokenIterator<CssTokenKind>,svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt (1)
19-21: Consider lazy initialization of trimmed tokensThe current implementation eagerly trims tokens during initialization, which might be unnecessary if the iterator isn't fully consumed. Consider using Kotlin's
lazydelegate for better performance.- internal val tokens: List<Token<out CssTokenKind>> = tokens.trim() + internal val tokens: List<Token<out CssTokenKind>> by lazy { tokens.trim() }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SelectorListItemConsumer.kt (1)
14-14: Remove duplicateCssTokenKind.ColonfromselectorTokensset
CssTokenKind.Colonis included twice in theselectorTokensset (lines 14 and 17). Since sets automatically eliminate duplicates, including the same token multiple times is unnecessary.Apply this diff to remove the duplicate token:
CssTokenKind.OpenSquareBracket, -CssTokenKind.Colon, CssTokenKind.DoubleColon,Also applies to: 17-17
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Prelude.kt (2)
5-16: Documentation is comprehensive, but could be enhanced.The interface is well-designed with appropriate type constraints and clear documentation. Consider adding an example in the KDoc to illustrate typical usage.
/** * Represents the prelude of a [Rule], which can be either * a selector list or an at-rule prelude. * * @param T The type of component value nodes within the prelude. + * + * @sample + * val selectorPrelude = Selector(listOf(SelectorListItem(...))) + * val atRulePrelude = AtRule(listOf(AtRulePrelude(...))) */
20-34: Optimize toString() implementation for better performance.While the current implementation is readable, it creates multiple intermediate strings. Consider using sequence operations for better memory efficiency when dealing with large selector lists.
data class Selector( override val components: List<SelectorListItem>, ) : Prelude<SelectorListItem> { override fun toString(): String = buildString { appendLine("Selector(") - appendLine("components = [".prependIndent(indentSize = 2)) - components.forEach { - appendLine(it.toString().prependIndent(indentSize = 4)) - } - appendLine("],".prependIndent(indentSize = 2)) + append(" components = [\n") + components.asSequence() + .map { " $it" } + .joinTo(this, "\n") + append("\n ],\n") appendLine(")") } }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssParserException.kt (3)
34-35: Simplify message handling by usingsuper.messageYou can use
super.messagedirectly instead of creating a private_messagevariable, which reduces redundancy.Apply this diff:
- private val _message = message override val message: String get() = buildString { - appendLine(_message) + appendLine(super.message) // ... }
80-102: Reduce code duplication inparserErrorfunctionsThe
parserErrorfunctions forCssIteratorandAstParserIteratorshare similar logic. Consider refactoring them to eliminate duplication by extracting common code into a private helper function.
124-163: Eliminate redundancy inparserCheckfunctionsThe
parserCheckfunctions have duplicated code forCssIteratorandAstParserIterator. Refactor them to reduce redundancy, possibly by creating a generic function or using an interface.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Comment.kt (2)
3-5: Enhance documentation with more details and examplesThe current documentation is minimal. Consider adding:
- Examples of both CSS and HTML comments
- Description of each property
- Usage examples
/** * Represents a CSS comment. + * + * This class can handle both CSS-style comments (/* ... */) and HTML-style comments (<!-- ... -->). + * + * @property value The content of the comment without the comment delimiters + * @property location The position of this comment in the source code + * @property isHtmlComment If true, formats the comment using HTML style (<!---->), otherwise uses CSS style (/**/) + * + * @sample + * // CSS comment + * Comment("Hello world", location, false) // toString(): /* Hello world */ + * // HTML comment + * Comment("Hello world", location, true) // toString(): <!-- Hello world --> */
11-15: Remove unused parameter and consider string template optimizationThe
indentparameter is not used in the implementation. Also, string concatenation could be optimized using templates.- override fun toString(indent: Int): String = if (isHtmlComment) { - "<!-- $value -->" - } else { - "/* $value */" - } + override fun toString(): String = when { + isHtmlComment -> "<!-- $value -->" + else -> "/* $value */" + }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/IdentTokenConsumer.kt (2)
7-12: Consider adding KDoc documentationThe class structure and visibility modifiers are well-designed. Consider adding KDoc documentation to describe the purpose and behavior of this token consumer, especially since it's part of a public API component.
Example addition:
+/** + * Consumes CSS identifier tokens, handling both standard identifiers and function names. + * An identifier token represents custom names in CSS (e.g., class names, IDs, custom properties). + * + * @property iterator The token iterator used to traverse the input + */ internal class IdentTokenConsumer( iterator: TokenIterator<CssTokenKind>, ) : TokenConsumer(iterator) {
1-45: Consider enhancing error recovery and debugging capabilitiesAs this is part of a CSS parsing system, consider these architectural improvements:
- Add error recovery mechanisms to handle malformed identifiers gracefully
- Consider adding debug logging for better troubleshooting
- Add support for collecting parsing statistics (e.g., number of identifiers processed, error rates)
This would make the system more robust and maintainable in production environments.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt (1)
56-57: Optimize theisHexDigitextension functionThe
isHexDigitfunction can be optimized by utilizing existing Kotlin functions for checking hexadecimal characters.Suggested improvement:
-private fun Char.isHexDigit(): Boolean { - return this in '0'..'9' || this in 'a'..'f' || this in 'A'..'F' +private fun Char.isHexDigit(): Boolean = this.isDigit() || this.lowercaseChar() in 'a'..'f'This refactor uses
isDigit()andlowercaseChar()for a more concise and readable implementation.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Rule.kt (2)
34-51: Consistent string representation forAtRuleThe
toStringmethods forAtRuleprovide two different outputs. This could be confusing when debugging.Consider unifying the
toStringimplementations or clearly distinguishing their purposes. For example, you might rename one of them totoDebugStringif it provides more detailed output.
57-70: Missing indentation inQualifiedRule.toStringThe
toStringmethod forQualifiedRuledoesn't include indentation for better readability, unlike theAtRuleversion.Consider adding indentation to improve consistency and readability:
override fun toString(): String { return buildString { appendLine("QualifiedRule(") appendLine("location = $location,".prependIndent(indentSize = 2)) appendLine("prelude = $prelude,".prependIndent(indentSize = 2)) appendLine("block = $block,".prependIndent(indentSize = 2)) append(")") } }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Value.kt (1)
77-84: Avoid using names that may clash with common identifiersDefining a data class named
Functionmight cause confusion, asFunctionis a common term in programming and Kotlin. Consider renaming it toFunctionValueorCssFunctionto improve readability and avoid potential naming conflicts.Apply this diff to rename the data class:
- data class Function( + data class FunctionValue( override val location: CssLocation, val name: kotlin.String, val arguments: List<Value>, ) : Value { override fun toString(indent: Int): kotlin.String = "$name(${arguments.joinToString(", ") { it.toString(indent = 0) }})" }Remember to update all references to this class in your codebase.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/Consumer.kt (2)
33-44: UseparserCheckfor consistent error handlingIn the
expectTokenmethod, you're usingcheckNotNullandcheckfor validation. To maintain consistent and informative error handling across your parser, consider using theparserCheckfunction, which includes the content context in error messages.Apply this diff to update the error checking:
- checkNotNull(current) { + parserCheck(value = current != null, content = content) { "Expected $kind but got null" } - check(current.kind == kind) { + parserCheck(value = current?.kind == kind, content = content) { "Expected $kind but got ${current?.kind}" }This ensures that all parser errors provide consistent and helpful information.
49-60: UseparserCheckfor consistent error handlingSimilarly, in the
expectNextTokenmethod, replacingcheckNotNullandcheckwithparserCheckwill provide better error messages and consistency.Apply this diff to update the error checking:
- checkNotNull(next) { + parserCheck(value = next != null, content = content) { "Expected $kind but got null" } - check(next.kind == kind) { + parserCheck(value = next?.kind == kind, content = content) { "Expected $kind but got ${next?.kind}" }This change improves error reporting and aligns with the rest of your parser's error handling strategy.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt (1)
65-65: Provide more descriptive error messagesThe error message
"Parser error."is not very informative and may hinder debugging efforts. Consider providing a more descriptive error message that specifies the issue, such as indicating that the expected closing token was not found.Apply this diff to improve the error message:
- iterator.parserError(content, "Parser error.") + iterator.parserError(content, "Expected closing token '$closingToken' but reached end without finding it.")This enhancement will make it easier to identify and resolve parsing issues.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssSpecificity.kt (2)
81-105: Clarify specificity calculation logic in selectorsConsider adding comments to explain how specificity is calculated for each selector type in the
calculateSpecificityfunction. This will improve code readability and help future maintainers understand the logic.
107-147: Enhance documentation for pseudo-class specificity calculationsThe
Selector.PseudoClass.calculateSpecificity()function handles pseudo-classes comprehensively. Adding inline comments to explain each case can increase clarity and maintainability.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleSelectorConsumer.kt (1)
51-60: Enhance attribute selector parsing to handle matchers and valuesCurrently, the parsing of attribute selectors only handles the attribute name and sets
matcherandvaluetonull. This limits the support to basic attribute selectors like[attribute]. Consider extending the implementation to handle matchers and values to support selectors like[attribute=value],[attribute~=value], etc.Example update:
Selector.Attribute( location = calculateLocation(current, next), name = content.substring(next.startOffset, endIndex = next.endOffset), - matcher = null, - value = null, + matcher = parseMatcher(iterator), + value = parseAttributeValue(iterator), combinator = lookupForCombinator(iterator), )svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Selector.kt (1)
15-20: Ensure consistency in 'toString' methods across selectorsThe base
toStringmethod in theSelectorinterface concatenatesnameandcombinator. However, derived classes likeClassandIdprepend characters like.and#respectively. Ensure alltoStringimplementations provide consistent and accurate string representations of selectors.Consider adjusting the base
toStringor ensuring all derived classes correctly override it.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt (1)
3-5: Enhance interface documentationConsider adding more detailed documentation explaining:
- The purpose and use cases of this interface
- How it fits into the lexer architecture
- Examples of concrete implementations
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Tokenizer.kt (1)
8-15: Consider documenting error handling behaviorThe interface looks well-designed, but the documentation should specify:
- How parsing errors are handled
- Whether exceptions can be thrown
- What happens with invalid input
Additionally, consider adding:
@Throws(IllegalArgumentException::class) // if applicable fun tokenize(input: String): List<Token<out T>>svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/AstParser.kt (1)
12-20: Robust interface design with suggested improvementsThe interface is well-designed with appropriate generic constraints. Consider these enhancements:
- Document error handling:
/** * @throws IllegalArgumentException if tokens list is empty * @throws ParseException for syntax errors */
- Add input validation:
fun parse(tokens: List<Token<out TTokenKind>>): TAstNode { require(tokens.isNotEmpty()) { "Token list cannot be empty" } // ... parsing logic }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/AtKeywordTokenConsumer.kt (1)
7-13: Add documentation for the class and its supported tokensMissing documentation explaining the purpose of this consumer and examples of supported @-keywords.
Add KDoc documentation:
+/** + * Consumes CSS @-keywords (e.g., @media, @import, @keyframes). + * + * Example valid tokens: + * - @media + * - @import + * - @keyframes + */ internal class AtKeywordTokenConsumer( iterator: TokenIterator<CssTokenKind>, ) : TokenConsumer(iterator) {svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/WhitespaceTokenConsumer.kt (1)
7-13: Add comprehensive documentationMissing documentation explaining the purpose and behavior of the whitespace consumer.
Add KDoc documentation:
+/** + * Consumes CSS whitespace tokens. + * + * Handles the following whitespace characters: + * - Space + * - Tab + * - Newline + * - Carriage return + * + * Multiple consecutive whitespace characters are combined into a single token. + */ internal class WhitespaceTokenConsumer( iterator: TokenIterator<CssTokenKind>, ) : TokenConsumer(iterator) {svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/TokenConsumer.kt (1)
30-33: Consider enhancing documentation for error handlingThe abstract
consumemethod's documentation could be improved by specifying:
- Expected behavior when invalid tokens are encountered
- Error handling guidelines for implementers
- Return value semantics (empty list vs null)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt (2)
7-9: Add KDoc documentation to explain class purposeConsider adding documentation to explain:
- The purpose of this consumer
- Examples of tokens it handles
- Usage examples
22-27: Add validation for token kindThe
consumemethod should validate that the providedkindis insupportedTokenKinds.override fun consume(kind: CssTokenKind): List<Token<out CssTokenKind>> { + require(kind in supportedTokenKinds) { "Unsupported token kind: $kind" } val start = iterator.offset val end = iterator.nextOffset() val token = Token(kind, start, end) return listOf(token) }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt (1)
7-9: Add KDoc documentation to explain string handlingConsider adding documentation to explain:
- String token format expectations
- Handling of quotes and escape sequences
- Error conditions and recovery
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/AtRulePrelude.kt (1)
5-16: Documentation looks good, consider adding @see referenceThe KDoc is well-written with clear examples. Consider adding a
@seereference to related classes likeAtRulefor better documentation navigation.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/StyleSheet.kt (1)
15-18: Consider validating empty children listThe toString implementation might benefit from handling empty children lists explicitly, perhaps with a special representation or validation.
override fun toString(indent: Int): String { + if (children.isEmpty()) return "/* empty stylesheet */" return children.joinToString("\n") { it.toString(indent) } }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Declaration.kt (2)
5-12: Enhance KDoc documentation with examples and constraintsWhile the documentation covers the basic purpose, it would be more helpful to include:
- Format constraints for the
propertyfield (e.g., valid CSS property names)- Example usage with sample values
- Common use cases
13-18: Consider adding property name validationThe
propertyfield should validate CSS property name format to prevent invalid declarations. Consider adding a validation check or using a sealed class/enum for well-known CSS properties.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt (1)
43-49: Reconsider negative location values for empty blocksUsing negative values (-1) for location properties might cause issues in error reporting or debugging. Consider:
- Using 0 as a default position
- Creating a dedicated
EmptyLocationobject- Making location nullable for empty blocks
Also applies to: 54-60
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssCombinator.kt (1)
21-21: Consider caching the tokens listThe
entries.mapoperation creates a new list on each access. Consider caching the result:- val tokens = entries.map { it.tokenKind } + val tokens: List<CssTokenKind> = entries.map { it.tokenKind }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumer.kt (1)
27-36: Consider using sequences for memory efficiencyThe current implementation stores all children in memory. For large stylesheets, this could be memory-intensive.
Consider using sequences:
- val children = mutableListOf<CssStatementNode>() + val children = sequence { + while (iterator.hasNext()) { + val next = iterator.next() ?: break + when (next.kind) { + CssTokenKind.WhiteSpace, CssTokenKind.Semicolon -> continue + CssTokenKind.EndOfFile -> break + CssTokenKind.AtKeyword -> yield(atRuleConsumer.consume(iterator)) + else -> yield(qualifiedRuleConsumer.consume(iterator)) + } + } + }.toList()svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt (1)
67-80: Optimize memory usage in prelude creationThe code creates multiple string copies when building the prelude.
Consider using string builders or ranges:
- source = content.substring(preludeStartOffset, preludeContentEndOffset), + source = content, // Store reference to original content + range = preludeStartOffset..preludeContentEndOffset, // Store range insteadsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt (2)
Line range hint
27-48: Consider consolidating duplicate normalization logic.The
normalizedId()extension function and companion object method contain duplicate logic. Consider refactoring to use a single implementation.sealed interface SvgNode : XmlNode { - fun String.normalizedId(): String = normalizeId(id = this) + fun String.normalizedId(): String = Companion.normalizedId(this) companion object { // ... other companion object members ... fun String.normalizedId(): String = normalizeId(this) private fun normalizeId(id: String): String = id.removePrefix("#").removePrefix("url(#").removeSuffix(")") } }
Line range hint
52-81: Consider adding null safety checks and documentation for edge cases.The
stackedTransformfunction handles complex transform stacking logic but could benefit from additional null safety checks and documentation about edge cases, especially aroundSvgDefsNodehandling./** * Calculates the stacked transform of the node by traversing up the tree * and concatenating the transform attributes of the ancestors. * @param parent The parent node of the current node. * @return The stacked transform of the node. * @throws IllegalStateException if the parent hierarchy is invalid * @note Special handling is applied for SvgDefsNode, where transform stacking stops */ fun SvgNode.stackedTransform(parent: XmlParentNode): SvgTransform? { + requireNotNull(parent) { "Parent node cannot be null" } var stacked = attributes[ATTR_TRANSFORM] if (parent !is SvgDefsNode) { var currentParent: XmlParentNode? = parentsvg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt (1)
256-326: Consider adding edge case tests.While the current test coverage is good, consider adding tests for:
- Invalid selector combinations
- Empty selector lists
- Malformed selectors
- Maximum specificity values
Example test case:
@Test fun `should handle empty selector list`() { // Arrange val rule = QualifiedRule( location = location, prelude = Prelude.Selector( components = listOf( SelectorListItem( location = location, selectors = emptyList(), ), ), ), block = Block.EmptyDeclarationBlock, ) val expected = "(0, 0, 0)" // Act val actual = CssSpecificity(rule) // Assert assertEquals(expected, actual.toString()) }svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (3)
10-312: Consider adding more edge cases to strengthen test coverage.While the existing tests cover the basic scenarios well, consider adding tests for:
- Invalid CSS syntax
- Empty selectors
- Comments within rules
- Nested selectors
- Vendor prefixes
486-520: Remove debug print statement from test.The
printlnstatement on line 499 should be removed as it's not necessary for the test execution and could clutter test output.- println("url: '$url'")
640-645: Add KDoc documentation to the helper method.The
assertTokenshelper method should include documentation explaining its purpose and parameters.+ /** + * Asserts that the tokenizer produces the expected tokens for the given content. + * + * @param content The CSS content to tokenize + * @param tokens The expected tokens + */ private fun assertTokens(content: String, tokens: List<Token<out CssTokenKind>>) {svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumerTest.kt (2)
1-107: Consider simplifying test setup using test fixtures.The test setup is quite verbose. Consider:
- Creating a test fixture class to handle the common setup
- Using builder pattern for test data construction
- Moving the token creation to separate helper methods
This would improve test readability and maintenance.
667-707: Consider using a dependency injection framework.The
buildStyleSheetConsumermethod manually wires many dependencies. Consider:
- Using a dependency injection framework to manage these dependencies
- Creating a factory class for consumer creation
- Adding documentation for the method and its dependencies
- Using builder pattern for consumer configuration
This would improve maintainability and make the testing setup more flexible.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRootNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt(4 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Token.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Tokenizer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/AtKeywordTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/IdentTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/NumberTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/TokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/WhitespaceTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/AstParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssCombinator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssSpecificity.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/Consumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/DeclarationConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/QualifiedRuleConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SelectorListItemConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleSelectorConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssParserException.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/AtRulePrelude.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Comment.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Declaration.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Prelude.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Rule.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Selector.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/StyleSheet.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Value.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt(1 hunks)svg-to-compose/src/jvmMain/kotlin/Main.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Token.kt
- svg-to-compose/src/jvmMain/kotlin/Main.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRootNode.kt
🔇 Additional comments (33)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt (1)
1-5: LGTM! Clean package structure and imports.
The package naming and import statements follow Kotlin best practices and are well-organized.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt (2)
1-5: LGTM! Package structure and imports are well-organized.
The package name follows Kotlin conventions, and imports are minimal and relevant.
14-18: LGTM! Clear purpose and well-documented interface.
The interface appropriately represents CSS component values and includes helpful documentation.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt (2)
1-16: LGTM! Well-organized imports and class structure
The imports are properly organized and the test class follows Kotlin conventions.
265-353: Verify edge cases for multiple selectors
While the test covers basic multiple selector scenarios, consider adding edge cases:
- Empty selector list
- Selectors with special characters
- Very long selector chains
Also, remove the println(actual) statement on line 351.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt (2)
23-33: LGTM! Clean and functional implementation.
The case conversion functions are well-implemented with proper documentation and correct handling of character cases.
Line range hint 61-88: Excellent documentation enhancement!
The comprehensive documentation with examples greatly improves the function's usability and maintainability.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt (1)
7-12: LGTM: Class declaration and token support are well-defined
The class is properly scoped as internal, extends the appropriate base class, and correctly declares its supported token kind.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt (1)
1-18: LGTM! Well-structured class declaration with proper documentation.
The class is properly encapsulated as internal, and the documentation clearly explains its purpose and responsibility.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Prelude.kt (1)
1-4: LGTM! Package structure and imports are well-organized.
The package naming follows Kotlin conventions, and imports are specific to the requirements.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssParserException.kt (1)
1-163: Overall Implementation Looks Good
The custom exception class and associated functions are well-implemented, providing detailed error reporting for CSS parsing. The use of contracts and extension functions enhances the code quality.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Comment.kt (1)
6-10: Verify CssStatementNode interface contract
The class implements CssStatementNode but we need to verify if all required methods are properly implemented.
✅ Verification successful
CssStatementNode interface contract is properly implemented
The Comment class correctly implements all required members of the interface hierarchy:
locationproperty fromCssNodetoString(indent: Int)method fromCssNode
The class properly extends CssStatementNode, which is a sealed interface that extends CssComponentValueNode, which in turn extends CssNode. All required members are implemented correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the CssStatementNode interface definition and its implementations
# Search for the interface definition
ast-grep --pattern 'interface CssStatementNode {
$$$
}'
# Search for other implementations to compare
ast-grep --pattern 'class $_ : CssStatementNode {
$$$
}'
Length of output: 120
Script:
#!/bin/bash
# Let's try a different approach to find the interface and its implementations
# First, let's find the interface file
fd "CssStatementNode" --type f
# Then let's look for the interface definition
rg "interface CssStatementNode" -A 5
# And find all files that mention this interface
rg "CssStatementNode"
Length of output: 1627
Script:
#!/bin/bash
# Let's check the CssComponentValueNode interface since CssStatementNode extends it
rg "interface CssComponentValueNode" -A 5
# Also let's check what properties/methods Comment needs to implement
rg "class Comment.*CssStatementNode" -B 5 -A 5
Length of output: 821
Script:
#!/bin/bash
# Let's check the base CssNode interface
rg "interface CssNode" -A 5
# And get the full content of the Comment class file
fd "Comment.kt" --type f --exec cat {}
Length of output: 1107
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/IdentTokenConsumer.kt (2)
1-6: LGTM: Clean package structure and imports
The package structure follows Kotlin conventions, and imports are specific and necessary.
14-44:
Fix boundary conditions and validation
There are several issues that need attention:
- The boundary check
start - 1 > 0is incorrect and could lead to issues:- It should be
>= 0to handle the first character - Current logic skips position 0
- It should be
- Missing validation of the 'kind' parameter against supportedTokenKinds
- The function detection logic might miss valid CSS functions that don't have preceding whitespace
Suggested fixes:
override fun consume(kind: CssTokenKind): List<Token<out CssTokenKind>> {
+ require(kind in supportedTokenKinds) { "Unsupported token kind: $kind" }
val start = iterator.offset
while (iterator.hasNext()) {
val char = iterator.get()
when (char) {
in '0'..'9',
in 'a'..'z',
in 'A'..'Z',
'-',
'_' -> {
iterator.nextOffset()
}
else -> {
break
}
}
}
return listOf(
if (
iterator.hasNext() && iterator.get() == '(' &&
- start - 1 > 0 && iterator.lookup(start - 1) in CssTokenKind.WhiteSpace
+ (start == 0 || iterator.lookup(start - 1) in CssTokenKind.WhiteSpace)
) {
Token(CssTokenKind.Function, start, iterator.offset)
} else {
Token(CssTokenKind.Ident, start, iterator.offset)
},
)
}Let's verify the CSS function parsing behavior:
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt (1)
25-34: Ensure thread safety for shared TokenIterator
All token consumers share the same iterator instance. If CssTokenizer is used in a multithreaded context, this could lead to concurrency issues.
Please confirm whether CssTokenizer is intended to be thread-safe. If so, consider creating separate iterator instances for each consumer or implementing synchronization mechanisms.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/DeclarationConsumer.kt (1)
25-26: Handle missing property name gracefully
If the expected CssTokenKind.Ident token is missing, the expectToken method may throw an exception.
Ensure that the parser handles errors gracefully when the property name is missing or malformed. Consider adding error handling or validation to provide meaningful feedback.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Value.kt (1)
93-100: Review the URL quoting logic in the toString method
In the Url data class, the toString method conditionally quotes the URL based on whether it starts with #. Please verify if this logic adheres to the CSS specification. According to CSS syntax, URLs should be quoted if they contain certain characters or are of specific types. It's important to ensure that the quoting logic correctly handles all valid URL formats.
Consider reviewing the CSS specification on URL token serialization to confirm that the current implementation handles all cases appropriately.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/QualifiedRuleConsumer.kt (1)
42-87: Code logic is clear and robust
The consume function correctly parses qualified rules, handling various token kinds and ensuring proper error handling.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt (2)
12-58: Comprehensive definition of CSS token kinds
The CssTokenKind enum is well-defined and covers the necessary token types for CSS parsing.
60-101: Efficient token kind retrieval
The fromChar method effectively retrieves the corresponding token kind from a character, enhancing tokenizer performance.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssParser.kt (1)
46-83: Well-structured CSS parser implementation
The CssParser class is well-designed, utilizing consumers to modularize the parsing logic. This approach enhances maintainability and testability.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt (1)
170-217: Handle incomplete URL parsing in 'parseUrl' method
In the parseUrl function, there is a possibility of parsing errors if the URL is incomplete or malformed. Ensure that the method robustly handles cases where the URL is not properly closed with a parenthesis.
Run the following script to check for unhandled cases of incomplete URLs:
✅ Verification successful
Let me gather more information about how incomplete URLs are handled in the test cases.
Incomplete URL parsing is properly handled
The code already handles incomplete URLs appropriately:
- The
parseUrlmethod correctly detects and reports incomplete URLs when encountering semicolons or closing braces - Test cases verify this behavior, specifically checking for "Incomplete URL value: missing closing ')'" error messages
- Both the lexer and parser test suites include cases for malformed URL scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usages of 'url(' without a closing parenthesis.
# Test: Find 'url(' occurrences not followed by ')'. Expect: No matches.
rg 'url\([^)]*$' --context 2
Length of output: 1995
Script:
#!/bin/bash
# Search for test cases related to URL parsing
ast-grep --pattern 'fun $test() {
$$$
url($$$)
$$$
}'
# Also search for specific test cases handling malformed URLs
rg "background-image.*url" -B 2 -A 5 "Test.kt"
Length of output: 156
Script:
#!/bin/bash
# Search for test cases in the correct test files
rg "background-image.*url" -B 2 -A 5 "*Test.kt"
# Look for URL-related test cases
ast-grep --pattern 'fun $test$_() {
$$$
parseUrl($$$)
$$$
}'
# Check for error handling test cases
rg "parserError.*Incomplete URL" -B 2 -A 2
Length of output: 212
Script:
#!/bin/bash
# Search for test files with full path
fd "Test.kt$" | xargs rg "background-image.*url" -B 2 -A 5
# Look for test cases that verify URL parsing errors
fd "Test.kt$" | xargs rg "Incomplete URL" -B 5 -A 5
# Check the implementation of URL parsing error handling
ast-grep --pattern 'parserError(
$$$
message = "Incomplete URL"
$$$
)'
Length of output: 5379
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt (1)
1688-1691: Enhance the assert method for better error messages
The current assert method provides limited information when assertions fail. Improving the error messages can significantly aid in debugging test failures.
As previously suggested, consider enhancing the assert method to include detailed error messages.
assertEquals(
expected.toString().replace(Regex("\\s+"), " "),
actual.toString().replace(Regex("\\s+"), " "),
"Expected and actual ASTs do not match.\nContent: $content\nExpected: ${expected.prettyPrint()}\nActual: ${actual.prettyPrint()}"
)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt (1)
6-18: Well-designed interface with idiomatic Kotlin usage
The interface is well-structured with:
- Appropriate use of operator overloading for the contains check
- Immutable representation using Set
- Clear separation of concerns
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/AtRulePrelude.kt (1)
17-35: Implementation looks good!
The data class is well-structured with:
- Proper interface implementation
- Clear property definitions
- Both compact and debug-friendly toString implementations
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/StyleSheet.kt (1)
11-14: Good choice using immutable List for children
Using List instead of MutableList for children promotes immutability and thread safety. This is a good practice for AST nodes.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Declaration.kt (1)
19-30: LGTM: Well-structured toString implementations
The toString implementations are well-designed:
- Proper indentation handling
- Clear formatting for both debug and CSS output
- Efficient use of buildString
Also applies to: 32-47
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt (2)
9-14: LGTM: Well-designed sealed class hierarchy
Good use of:
- Generic type parameter for flexibility
- Sealed class for type safety
- Clear inheritance structure
20-38: LGTM: Robust SimpleBlock implementation
The SimpleBlock implementation is well-structured with:
- Clear property definitions
- Proper toString formatting
- Efficient use of buildString
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssCombinator.kt (2)
13-18: LGTM: Well-defined combinator representations
The enum class is well-structured with:
- Clear string representations
- Proper token kind mappings
- Comprehensive coverage of CSS combinators
37-43: LGTM: Efficient token mapping implementation
The from function implementation is:
- Exhaustive in handling all cases
- Clear in its null handling
- Efficient with when expression
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt (1)
Line range hint 83-105: LGTM! Well-structured and documented conversion function.
The asNodes function is well-implemented with:
- Clear documentation
- Comprehensive type handling
- Efficient use of inline modifier
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (1)
1-9: LGTM! Well-structured test class with clear organization.
The test class follows good testing practices with:
- Clear package structure
- Appropriate use of Burst for parameterized testing
- Necessary imports for testing functionality
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt
Show resolved
Hide resolved
...o-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt
Show resolved
Hide resolved
...o-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt
Outdated
Show resolved
Hide resolved
...ompose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/NumberTokenConsumer.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt
Outdated
Show resolved
Hide resolved
...pose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumerTest.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (4)
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (1)
403-412: Improve test efficiency by combining similar test casesThe test cases for
css rule with number without unitandcss rule with margin with two valuesare similar in structure. Consider using parameterized tests to reduce code duplication and improve maintainability.svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt (1)
1162-1163: Add test for parsing comments in CSS rulesCurrently, there are no tests covering the parsing of CSS comments. Adding such tests can ensure that the parser correctly handles comments within CSS code.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt (2)
98-100: Optimize fromChar method performanceThe current implementation iterates through all entries for each character lookup. For better performance, consider using a lookup map.
Consider this optimization:
companion object { + private val charToTokenKind: Map<Char, CssTokenKind> = entries + .flatMap { kind -> kind.representation.map { it to kind } } + .toMap() + fun fromChar(char: Char): CssTokenKind? { - return entries.firstOrNull { char in it } + return charToTokenKind[char] } }
73-85: Complete the WhiteSpaceSignificantAdjacentTokens setThe set is missing some token types where whitespace is significant:
AtKeywordfor at-rulesCDCandCDOfor XML compatibilityDimensionfor unitsAdd these token types to the set:
val WhiteSpaceSignificantAdjacentTokens = setOf( Ident, Hash, Dot, Number, String, Function, Url, OpenParenthesis, CloseParenthesis, OpenSquareBracket, CloseSquareBracket, + AtKeyword, + CDC, + CDO, + Dimension, )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/CommentTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/WhitespaceTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenIterator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/WhitespaceTokenConsumer.kt
🧰 Additional context used
📓 Learnings (3)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt (1)
Learnt from: rafaeltonholo
PR: rafaeltonholo/svg-to-compose#99
File: svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt:26-26
Timestamp: 2024-12-12T23:17:01.116Z
Learning: In the `svg-to-compose` project, the `TokenKind` interface defines the `contains` operator function that accepts a `Char`. Since `CssTokenKind` implements `TokenKind`, expressions like `char in CssTokenKind.TokenType` are valid for checking if a `Char` is contained within a specific `CssTokenKind`.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt (1)
Learnt from: rafaeltonholo
PR: rafaeltonholo/svg-to-compose#99
File: svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt:26-26
Timestamp: 2024-12-12T23:17:01.116Z
Learning: In the `svg-to-compose` project, the `TokenKind` interface defines the `contains` operator function that accepts a `Char`. Since `CssTokenKind` implements `TokenKind`, expressions like `char in CssTokenKind.TokenType` are valid for checking if a `Char` is contained within a specific `CssTokenKind`.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt (1)
Learnt from: rafaeltonholo
PR: rafaeltonholo/svg-to-compose#99
File: svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt:26-26
Timestamp: 2024-12-12T23:17:01.116Z
Learning: In the `svg-to-compose` project, the `TokenKind` interface defines the `contains` operator function that accepts a `Char`. Since `CssTokenKind` implements `TokenKind`, expressions like `char in CssTokenKind.TokenType` are valid for checking if a `Char` is contained within a specific `CssTokenKind`.
🔇 Additional comments (1)
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt (1)
1684-1696: Enhance the assert helper method for better error messages
The current assert method may not provide detailed information when a test fails, making it difficult to identify discrepancies between the expected and actual AST structures. Enhancing the error messages can improve debugging.
Consider modifying the assert method to include the input content and a clear comparison of the expected and actual outputs.
private fun assert(
content: String,
tokens: List<Token<out CssTokenKind>>,
expected: StyleSheet,
) {
val astParser = CssParser(content)
val actual = astParser.parse(tokens)
- assertEquals(expected, actual)
+ assertEquals(expected, actual, "Parsing failed for content:\n$content\nExpected:\n$expected\nActual:\n$actual")
}| while (true) { | ||
| val current = iterator.peek(currentOffset++) | ||
| when (current) { | ||
| ESCAPE_CHAR if iterator.hasNext() && iterator.peek(currentOffset) in quotesTokens -> { | ||
| currentOffset++ | ||
| } | ||
|
|
||
| in quotesTokens, Char.EMPTY -> { | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Add EOF checks to prevent potential exceptions
In the consume method's while (true) loop, there is a risk of encountering an IndexOutOfBoundsException if the end of the iterator is reached without finding a terminating quote. Before calling iterator.peek(currentOffset++), you should check if the iterator has enough characters to peek ahead.
Apply this diff to add EOF checks:
while (true) {
+ if (!iterator.hasNext(currentOffset)) {
+ // Unterminated string
+ break
+ }
val current = iterator.peek(currentOffset++)
when (current) {
- ESCAPE_CHAR if iterator.hasNext() && iterator.peek(currentOffset) in quotesTokens -> {
+ ESCAPE_CHAR if iterator.hasNext(currentOffset) && iterator.peek(currentOffset) in quotesTokens -> {
currentOffset++
}
in quotesTokens, Char.EMPTY -> {
break
}
}
}📝 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.
| while (true) { | |
| val current = iterator.peek(currentOffset++) | |
| when (current) { | |
| ESCAPE_CHAR if iterator.hasNext() && iterator.peek(currentOffset) in quotesTokens -> { | |
| currentOffset++ | |
| } | |
| in quotesTokens, Char.EMPTY -> { | |
| break | |
| } | |
| } | |
| } | |
| while (true) { | |
| if (!iterator.hasNext(currentOffset)) { | |
| // Unterminated string | |
| break | |
| } | |
| val current = iterator.peek(currentOffset++) | |
| when (current) { | |
| ESCAPE_CHAR if iterator.hasNext(currentOffset) && iterator.peek(currentOffset) in quotesTokens -> { | |
| currentOffset++ | |
| } | |
| in quotesTokens, Char.EMPTY -> { | |
| break | |
| } | |
| } | |
| } |
...ompose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt
Show resolved
Hide resolved
...mpose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/CommentTokenConsumer.kt
Show resolved
Hide resolved
...mpose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/CommentTokenConsumer.kt
Show resolved
Hide resolved
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt
Show resolved
Hide resolved
rafaeltonholo
force-pushed
the
feat/add-support-style-tag
branch
from
dfa75d4 to
c611b10
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (3)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleSelectorConsumer.kt (2)
21-23:⚠️ Potential issueDefine missing
selectorTokensvariableThe
selectorTokensvariable used inconsumemethod is not defined. This will cause a compilation error.Add this property to the class:
internal class SimpleSelectorConsumer( content: String, ) : Consumer<Selector>(content) { + private val selectorTokens = setOf( + CssTokenKind.Ident, + CssTokenKind.Asterisk, + CssTokenKind.Hash, + CssTokenKind.Dot, + CssTokenKind.OpenSquareBracket, + CssTokenKind.DoubleColon, + CssTokenKind.Colon + )
116-131:⚠️ Potential issuePrevent infinite loop in buildParameters
The
buildParametersmethod could enter an infinite loop ifconsumedoesn't advance the iterator.Add a safety check:
private fun buildParameters(iterator: AstParserIterator<CssTokenKind>): List<Selector> { if (iterator.expectNextTokenNotNull().kind != CssTokenKind.OpenParenthesis) { iterator.rewind() return emptyList() } val parameters = mutableListOf<Selector>() + var lastOffset = iterator.currentOffset() while (iterator.hasNext()) { val next = iterator.expectNextTokenNotNull() if (next.kind == CssTokenKind.CloseParenthesis) { break } + val currentOffset = iterator.currentOffset() + if (currentOffset == lastOffset) { + iterator.parserError(content, "Parser did not advance; potential infinite loop") + } + lastOffset = currentOffset parameters += consume(iterator) } return parameters }svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt (1)
117-127:⚠️ Potential issueTest case doesn't validate incomplete viewBox scenario.
The test case for incomplete viewBox is identical to the missing viewBox test and doesn't actually test an incomplete viewBox scenario.
Apply this diff to properly test incomplete viewBox:
@Test fun `when incomplete viewBox must convert to floatArrayOf of '0 0 300 150'`() { val svgNode = SvgRootNode( parent = XmlRootNode(children = mutableSetOf()), children = mutableSetOf(), - attributes = mutableMapOf(), + attributes = mutableMapOf( + "viewBox" to "0 0 300" // Incomplete viewBox missing height + ), ) val expected = floatArrayOf(0f, 0f, 300f, 150f) val actual = svgNode.viewBox assertContentEquals(expected, actual) }
🧹 Nitpick comments (44)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt (2)
15-15: Avoid using magic numbers for offsets.The offset calculation subtracts the numeric constant 4, which may reduce code clarity. Consider using a named constant or a comment explaining why 4 is needed here.
24-39: Clarify handling of quoted vs. unquoted URLs.While returning a Function token for quoted URLs may follow the spec, it's unclear if there's any additional parsing or content extraction needed for these cases. Consider documenting or adding follow-up logic to handle the quoted URL content effectively.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt (1)
Line range hint
72-84: Efficient transformation pipeline.Chaining mapNotNull, flatten, and reduceOrNull in createGroupClipPath is concise, but consider short-circuiting early if performance becomes a concern with large node sets.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (1)
1-8: Solid test file structure and usage of println
This file provides a structured set of test cases, leveraging parameterized testing via Burst and accurately verifying token outputs for various CSS constructs. However, there's a println statement at line 491 and line 520 that might introduce verbose console logs, which could be replaced by a logger or removed in production test code.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/Consumer.kt (6)
9-13: Clarify usage context of the typealias.
While the typealias is self-explanatory, adding a brief note on when or why it's used (e.g., "Used by all Consumer subclasses to iterate over tokens") can help new contributors quickly grasp its significance.
22-24: Consider refining parameter naming for clarity.
The parameter name “content” is a bit abstract. If it specifically represents CSS content or source text, renaming it to something like “sourceContent” or “cssContent” might be more self-evident for future maintainers.
33-44: Standardize error handling approach.
This function uses “checkNotNull” and “check” for validation, whereas other functions in the class (e.g., line 73) use “parserCheck.” Consider adopting a single, consistent error-checking mechanism to streamline debugging and maintain a uniform style throughout the codebase.
65-77: Avoid repeating token-check logic.
The logic in this overload for multiple expected kinds largely overlaps with the single-kind variant. To reduce duplication, you could reuse a shared helper method and pass either a single CssTokenKind or a set of them.
82-88: Include token position in error messages.
When a null token is encountered, debugging would be easier if the error message included the position/index at which the null token was expected. Consider extending the error message to indicate the location in the source content.
93-99: Align naming with “expectNextToken.”
Method “expectNextTokenNotNull” performs a parallel function to “expectNextToken”, but for null checks only. For clarity, consider an alternate name (e.g., “expectNextNonNullToken”) to keep the naming consistent.svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt (6)
23-141: Consider adding edge case assertions in your test.
While this test thoroughly covers a basic .class selector, it might be useful to add variations such as ".class-name_with-dash" or ".className123" to ensure correct token parsing.
143-261: De-duplicate test code for standard property checks.
This block and the previous test block share a very similar pattern of verifying "background" and "color" properties. Consider extracting a helper function for building or asserting repeated property tokens to keep your tests DRY.
263-482: Expand coverage for multiple combined selectors.
You’ve tested .my-rule and #my-rule in separate blocks here, which is good. However, consider a scenario where a single rule includes a class, ID, and pseudo-class or attribute selector. This would bolster coverage for more complex real-world usage.
575-687: Validate correctness of multiple URL-based properties.
The test with multiple selectors is solid, but additional tests for different URL-based properties (e.g., "background-image") or references to external resources could provide more robust coverage of the parser's URL-handling logic.
1162-1256: Potential synergy with previous test blocks.
This test is very similar to the "class" + "ID" checks above. By combining them under a single parameterized test (using Burst or a custom parameter injection), you could streamline your test approach.
1258-1411: Consider filtering out extraneous whitespace tokens.
The usage of WhiteSpace tokens for these multi-selector tests is correct. However, if the code stands to benefit from ignoring or compressing superfluous whitespace in production or performance-critical scenarios, you might add an option in the parser for whitespace compression.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumer.kt (4)
11-20: Consider enhancing KDoc documentationWhile the documentation is clear, consider adding:
@returndocumentation describing the constructed StyleSheet- A simple example usage demonstrating the consumer in action
/** * Consumes a CSS stylesheet and builds a [StyleSheet] object. * * This class iterates through the CSS tokens and delegates the consumption of * at-rules and qualified rules to specific consumers. * * @param content The CSS content string. * @param atRuleConsumer The consumer responsible for handling at-rules. * @param qualifiedRuleConsumer The consumer responsible for handling qualified rules. + * @return A [StyleSheet] object representing the parsed CSS content + * + * Example: + * ``` + * val consumer = StyleSheetConsumer( + * content = "body { color: red; }", + * atRuleConsumer = AtRuleConsumer(content), + * qualifiedRuleConsumer = QualifiedRuleConsumer(content) + * ) + * val styleSheet = consumer.consume(iterator) + * ``` */
28-32: Consider dynamic location end calculationThe location's end is set to the entire content length upfront, which might not accurately reflect the actual parsed content's end position. Consider updating the end position based on the last successfully parsed token's position.
39-41: Enhance error message for null tokenThe error message could be more specific about which token was null and its position in the stream.
- checkNotNull(next) { - "Expected token but got null" - } + checkNotNull(next) { + "Expected token at position ${iterator.currentPosition} but got null" + }
42-48: Consider making the when expression exhaustiveThe current when expression relies on the else branch to handle all unspecified cases. Consider making it exhaustive by explicitly handling all CssTokenKind values.
when (next.kind) { CssTokenKind.WhiteSpace, CssTokenKind.Semicolon -> Unit CssTokenKind.EndOfFile -> return styleSheet CssTokenKind.AtKeyword -> children += atRuleConsumer.consume(iterator) - else -> children += qualifiedRuleConsumer.consume(iterator) + CssTokenKind.IdentToken, + CssTokenKind.DelimToken, + CssTokenKind.NumberToken, + // ... other specific token kinds ... + -> children += qualifiedRuleConsumer.consume(iterator) }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt (2)
7-9: Add KDoc documentation for the DirectTokenConsumer classConsider adding documentation to explain the purpose and responsibility of this token consumer, especially its role in CSS parsing.
Example addition:
+/** + * Consumes direct CSS tokens that don't require additional parsing logic. + * These include punctuation marks and brackets that are interpreted as-is. + * + * @property iterator The token iterator providing access to the input stream + */ internal class DirectTokenConsumer( iterator: TokenIterator<CssTokenKind>, ) : TokenConsumer(iterator) {
10-20: Consider grouping related token kinds with commentsWhile the set of supported tokens is comprehensive, readability could be improved by grouping related tokens.
override val supportedTokenKinds: Set<CssTokenKind> = setOf( + // Punctuation CssTokenKind.Greater, CssTokenKind.Dot, CssTokenKind.Comma, CssTokenKind.Colon, CssTokenKind.Semicolon, + // Braces CssTokenKind.OpenCurlyBrace, CssTokenKind.CloseCurlyBrace, + // Parentheses CssTokenKind.OpenParenthesis, CssTokenKind.CloseParenthesis, )svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/gradient/SvgGradient.kt (2)
Line range hint
48-93: Consider refactoring coordinate calculation functions to reduce duplicationThe three coordinate calculation functions share similar patterns and logic. Consider extracting common functionality into a reusable helper function.
Here's a suggested refactoring:
+ private fun calculateGradientCoordinate( + length: SvgLength, + target: List<PathNodes>, + getDimension: (root: SvgRootNode) -> Float, + getBoundingBoxValue: (boundingBox: Rectangle) -> Float, + getBoundingBoxOffset: (boundingBox: Rectangle) -> Float, + ): Float { + val root = rootParent as SvgRootNode + return if (gradientUnits == "objectBoundingBox") { + check(target.isNotEmpty()) + val boundingBox = target.boundingBox() + length.toFloat(getBoundingBoxValue(boundingBox)) + getBoundingBoxOffset(boundingBox) + } else { + length.toFloat(getDimension(root)) + } + } + internal fun calculateGradientXCoordinate( length: SvgLength, target: List<PathNodes> = emptyList(), - ): Float { - val root = rootParent as SvgRootNode - return if (gradientUnits == "objectBoundingBox") { - check(target.isNotEmpty()) - val boundingBox = target.boundingBox() - length.toFloat(boundingBox.width.toFloat()) + boundingBox.x.toFloat() - } else { - val baseDimension = root.viewportWidth - length.toFloat(baseDimension) - } + ): Float = calculateGradientCoordinate( + length, + target, + { it.viewportWidth }, + { it.width.toFloat() }, + { it.x.toFloat() } }Similar changes can be applied to
calculateGradientYCoordinateandcalculateGradientXYCoordinate.
Line range hint
48-93: Enhance error handling for empty target listThe
check(target.isNotEmpty())could provide a more descriptive error message to help developers understand the issue.- check(target.isNotEmpty()) + check(target.isNotEmpty()) { "Cannot calculate gradient coordinates with empty target list when gradientUnits is 'objectBoundingBox'" }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleSelectorConsumer.kt (2)
65-69: Simplify location calculation for pseudo selectorsThe location calculation logic is duplicated between pseudo-class and pseudo-element selectors. Consider extracting this to a helper method.
+ private fun calculatePseudoSelectorEndOffset( + next: Token<CssTokenKind>, + parameters: List<Selector> + ): Int = if (parameters.isEmpty()) next.endOffset else parameters.last().location.end + 1 val parameters = buildParameters(iterator) - val endOffset = if (parameters.isEmpty()) { - next.endOffset - } else { - parameters.last().location.end + 1 - } + val endOffset = calculatePseudoSelectorEndOffset(next, parameters)Also applies to: 81-85
98-100: Optimize combinator lookupThe current implementation of
lookupForCombinatorcreates a newCssCombinatorinstance for every selector, even when there isn't one.private fun lookupForCombinator(iterator: AstParserIterator<CssTokenKind>): CssCombinator? { - return CssCombinator.from(iterator.peek(steps = 0)?.kind) + val nextKind = iterator.peek(steps = 0)?.kind ?: return null + return CssCombinator.from(nextKind) }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt (2)
12-16: Add documentation for the token mappingConsider adding KDoc documentation to explain the purpose and usage of this mapping. This would help maintainers understand the relationship between opening and closing tokens.
+/** + * Maps opening tokens to their corresponding closing tokens for different types of blocks: + * - Parentheses: ( ) + * - Square brackets: [ ] + * - Curly braces: { } + */ private val blockOpeningTokens = mapOf(
69-75: Add example in SimpleRuleBlockConsumer documentationFor consistency with SimpleDeclarationBlockConsumer, consider adding an example in the documentation.
/** * Consumes a simple block of rules. + * + * E.g.: `@media screen { rule1 {} rule2 {} }` */svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Value.kt (1)
89-100: Clarify quoting behavior in Url.toString()
Your logic selectively adds quotes only if the URL doesn't start with '#'. Validate whether this condition covers all scenarios—for example, how you want data URIs or other special cases handled.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ImageParser.kt (3)
18-20: New CSS dependencies introduced
Added imports for the CSS parser, specificity, and syntax nodes. Ensure that only needed elements are imported to maintain clarity.
183-183: Pass 'computedRules' to 'asNodes'
This is crucial for applying CSS-based transformations or attribute overrides. Unit test coverage is recommended to confirm that selectors, specificity, and style application logic are correct.
205-226: 'resolveStyleTags' logic
Extracts style content, parses CSS, and accumulates 'ComputedRule' objects. Carefully handle errors in CSS syntax or empty style nodes. Possibly guard against duplicates or conflicting specificity.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgClipPath.kt (1)
21-27: 'asNodeWrapper' now accepts 'computedRules'
Passing in computed style rules allows fine-grained styling in clip paths. Ensure that expensive rule checks do not degrade performance if clipPaths are heavily used or nested.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssSpecificity.kt (1)
70-76: Consider caching specificity calculationsThe
calculateSelectorsSpecificityfunction recalculates specificity for each selector, which could be expensive for large stylesheets.Consider implementing caching:
private val specificityCache = mutableMapOf<SelectorListItem, CssSpecificity>() fun calculateSelectorsSpecificity(prelude: Prelude.Selector): Map<SelectorListItem, CssSpecificity> { return prelude.components.associateWith { selector -> specificityCache.getOrPut(selector) { selector.selectors.fold(CssSpecificity()) { acc, sel -> acc + sel.calculateSpecificity() } } } }svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt (2)
11-32: Consider adding edge cases for space-separated viewBox values.While the current test cases are good, consider adding edge cases such as:
- Multiple consecutive spaces
- Scientific notation (e.g., "1e2")
- Very large/small numbers
34-56: Consider reducing test duplication.The test logic for comma separators is very similar to the space-separated test. Consider extracting common test data into a shared structure:
private val commonTestCases = listOf( TestCase(input = "0,0,120,120", expected = floatArrayOf(0f, 0f, 120f, 120f)), // ... more cases )svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt (1)
Line range hint
113-142: Enhance documentation for transform stacking edge cases.While the implementation is solid, the documentation should clarify:
- Why SvgDefsNode is handled differently
- The order of transform application
- Edge cases in the transform stack
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Selector.kt (3)
14-64: Consider adding input validation for selector names.While the implementation is well-structured, consider adding validation for selector names to ensure they follow CSS naming conventions:
- Class names should not contain spaces
- IDs should be unique
- Type selectors should be valid HTML/SVG element names
75-150: Consider extracting common toString logic.PseudoClass and PseudoElement have very similar toString implementations. Consider extracting the common logic:
private fun buildParameterizedString( prefix: String, name: String, parameters: List<Selector>, combinator: CssCombinator?, indent: Int ): String = buildString { append("$prefix$name") if (parameters.isNotEmpty()) { append("(") append(parameters.joinToString(", ") { it.toString(indent = 0) }) append(")") } append(combinator?.representation.orEmpty()) }
158-181: Consider optimizing StringBuilder initial capacity.For better performance, consider pre-sizing the StringBuilder based on the expected output size:
- override fun toString(): String { - return buildString { + override fun toString(): String { + return buildString(capacity = 128) { // Adjust based on average sizesvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt (1)
222-261: Consider extracting color parsing logic into separate methodsThe color parsing logic could be more maintainable if split into separate methods for hex and function-based colors.
Consider this refactor:
private fun Iterator.parseColor(): Value.Color { val current = expectTokenNotNull() return when (current.kind) { - CssTokenKind.Hash -> { - val next = expectNextToken(kind = CssTokenKind.HexDigit) - Value.Color( - location = CssLocation( - source = content.substring(startIndex = current.startOffset, endIndex = next.endOffset), - start = current.startOffset, - end = next.endOffset, - ), - value = content.substring(startIndex = current.startOffset, endIndex = next.endOffset), - ) - } + CssTokenKind.Hash -> parseHexColor(current) + CssTokenKind.Function -> parseFunctionColor(current) + else -> error("Unexpected token: $current") + } +} - CssTokenKind.Function -> { - expectNextToken(kind = CssTokenKind.OpenParenthesis) - var colorEndOffset = current.endOffset - while (hasNext()) { - val next = expectNextTokenNotNull() - if (next.kind == CssTokenKind.CloseParenthesis) { - break - } - colorEndOffset = next.endOffset - } - // account ) in color function - colorEndOffset++ - Value.Color( - location = CssLocation( - source = content.substring(startIndex = current.startOffset, endIndex = colorEndOffset), - start = current.startOffset, - end = colorEndOffset, - ), - value = content.substring(startIndex = current.startOffset, endIndex = colorEndOffset), - ) - } +private fun Iterator.parseHexColor(current: Token): Value.Color { + val next = expectNextToken(kind = CssTokenKind.HexDigit) + return createColorValue(current.startOffset, next.endOffset) +} - else -> error("Unexpected token: $current") +private fun Iterator.parseFunctionColor(current: Token): Value.Color { + expectNextToken(kind = CssTokenKind.OpenParenthesis) + var colorEndOffset = current.endOffset + while (hasNext()) { + val next = expectNextTokenNotNull() + if (next.kind == CssTokenKind.CloseParenthesis) { + break + } + colorEndOffset = next.endOffset } -} + return createColorValue(current.startOffset, colorEndOffset + 1) +} + +private fun createColorValue(startOffset: Int, endOffset: Int): Value.Color = + Value.Color( + location = CssLocation( + source = content.substring(startIndex = startOffset, endIndex = endOffset), + start = startOffset, + end = endOffset, + ), + value = content.substring(startIndex = startOffset, endIndex = endOffset), + )svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt (3)
14-16: Consider making location a companion object constantSince
locationis shared and immutable across all tests, it could be moved to a companion object for better encapsulation.class CssSpecificityTest { - private val location = CssLocation.Undefined + companion object { + private val LOCATION = CssLocation.Undefined + }
109-296: Add documentation for complex test casesWhile the test cases are comprehensive, consider adding documentation for complex scenarios, particularly in the
should calculate specificity for multiple selectorstest. For example, the test case with PseudoClass and :not selector (lines 250-258) would benefit from a comment explaining the expected specificity calculation.@Test @Burst fun `should calculate specificity for multiple selectors`( // Arrange rules: Pair<List<Selector>, String> = burstValues( + // Basic combination of type selector with multiple classes listOf( Selector.Type(location = CssLocation.Undefined, name = "li"), Selector.Class(location = CssLocation.Undefined, name = "my-class"), Selector.Class(location = CssLocation.Undefined, name = "my-second-class"), ) to "[(0, 2, 1)]", + // Descendant selector with universal selector listOf( Selector.Type( location = CssLocation.Undefined, name = "ol",
1-297: Consider extracting test data to a companion objectTo improve maintainability and reusability, consider extracting the test data into a companion object or separate test data class. This would make the test cases more maintainable and allow reuse in other test classes.
Example structure:
companion object { private object TestData { val SINGLE_SELECTORS = listOf( TestCase( selectors = listOf(Selector.Type(...)), expectedSpecificity = "[(0, 0, 1)]" ) ) // ... more test data } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
.run/svg-to-compose [allTests].run.xmlis excluded by!**/.run/**config/detekt.ymlis excluded by!**/config/detekt.ymlsamples/svg/css/house-with-trees.svgis excluded by!**/*.svg,!**/samples/**samples/svg/css/mountain-with-clouds.svgis excluded by!**/*.svg,!**/samples/**
📒 Files selected for processing (74)
buildSrc/build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.kmp.gradle.kts(0 hunks)buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.testing.gradle.kts(1 hunks)gradle/libs.versions.toml(2 hunks)svg-to-compose/build.gradle.kts(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgClipPath.kt(2 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgElementNode.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGraphicNode.kt(0 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt(5 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgLinearGradientNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgMaskNode.kt(0 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgPathNode.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRootNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/gradient/SvgGradient.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt(4 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/Collections.extension.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt(4 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Token.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Tokenizer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/AtKeywordTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/CommentTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/IdentTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/NumberTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/TokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/WhitespaceTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ImageParser.kt(4 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/SvgParser.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/XmlParser.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/AstParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/Element.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssCombinator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssSpecificity.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/Consumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/DeclarationConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/QualifiedRuleConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SelectorListItemConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleSelectorConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssParserException.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/AtRulePrelude.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Comment.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Declaration.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Prelude.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Rule.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Selector.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/StyleSheet.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Value.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt(1 hunks)svg-to-compose/src/jvmMain/kotlin/Main.kt(2 hunks)svg-to-compose/src/nativeMain/kotlin/Main.kt(5 hunks)
💤 Files with no reviewable changes (3)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgMaskNode.kt
- buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.kmp.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGraphicNode.kt
🚧 Files skipped from review as they are similar to previous changes (43)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/Element.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Token.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Tokenizer.kt
- buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.testing.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Comment.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/Collections.extension.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Declaration.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/AstParser.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SelectorListItemConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/AtKeywordTokenConsumer.kt
- buildSrc/build.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/StyleSheet.kt
- gradle/libs.versions.toml
- svg-to-compose/src/jvmMain/kotlin/Main.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenIterator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/AtRulePrelude.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/NumberTokenConsumer.kt
- svg-to-compose/build.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/QualifiedRuleConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/CommentTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/IdentTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Rule.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/DeclarationConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssCombinator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRootNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt
- svg-to-compose/src/nativeMain/kotlin/Main.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/WhitespaceTokenConsumer.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/TokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssParserException.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumerTest.kt
🧰 Additional context used
📓 Learnings (1)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt (1)
Learnt from: rafaeltonholo
PR: rafaeltonholo/svg-to-compose#99
File: svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt:26-26
Timestamp: 2024-12-12T23:17:01.116Z
Learning: In the `svg-to-compose` project, the `TokenKind` interface defines the `contains` operator function that accepts a `Char`. Since `CssTokenKind` implements `TokenKind`, expressions like `char in CssTokenKind.TokenType` are valid for checking if a `Char` is contained within a specific `CssTokenKind`.
🔇 Additional comments (44)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt (2)
18-22: Add bounds checking to whitespace consumption.
As noted in a previous review, the while loop that consumes whitespace should ensure the iterator has enough characters to peek, preventing any out-of-bounds reads or potential infinite loops.
48-54: Enhance URL validation according to CSS spec.
As previously noted, current logic does not handle escape sequences, invalid characters, or “bad URL” states comprehensively. Consider implementing these checks to adhere to the CSS specification and prevent incorrect or partial URL consumption.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt (5)
8-8: Import looks good.
The introduced import for ComputedRule is logical given the new styling rules.
31-37: Great use of default parameters for computedRules.
Providing a default empty list of computedRules ensures that existing logic won't break if no rules are passed. The call to createGroupClipPath with computedRules, masks, and minified helps unify style handling across the node creation process.
44-44: Consider safe-cast usage.
Using (it as? SvgNode)?.asNodes(...) is a safe cast, and is beneficial if children could be non-SvgNode types. Ensure that this doesn't mask potential errors if a child is unexpectedly non-SvgNode.
53-56: Signature consistency.
The flatNode function now mirrors asNode’s updated signature and default parameters, ensuring consistent usage across the codebase. This improves maintainability and reduces the risk of mismatched calls.
92-104: Robust clip-path resolution.
The added logic checks fallback scenarios for clip-path (via attributes or the root parent’s clipPaths) before settling on null. This is thorough. However, consider gracefully handling cases where (rootParent as? SvgRootNode) is not actually an SvgRootNode to avoid potential runtime exceptions in more complex hierarchies.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (4)
9-700: Comprehensive coverage of CSS constructs
The tests thoroughly cover multiple CSS scenarios: classes, IDs, multiple selectors, URL tokens, escaped sequences, and dimension/number tokens. The systematic approach of comparing each token's start and end offsets helps ensure correctness and reliability.
681-683: Fix syntax error in test function definition
This function name contains mismatched backticks, which may break the test runner. This repeats a prior comment from an outdated review.
Apply this diff to fix the function definition:
- fun `create tokens for a rule with BadUrl with `(badString: BadStringParams) {
+ @Burst
+ fun `create tokens for a rule with BadUrl`(badString: BadStringParams) {702-706: Assertion utility is straightforward and maintainable
The custom assertTokens function cleanly compares the expected and actual tokens. This approach improves readability and consistency across tests.
708-781: Enums for parameterized tests are well-organized
The enumerations (BadUrlTokenParam, UrlAsFunctionParam, BadStringParams, NumberOrDimensionTokenParam) provide a structured approach for parameterizing complex CSS token scenarios. This design makes the tests more descriptive and easier to extend.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt (5)
10-11: LGTM: Well-structured interface additions
The new nullable properties for CSS support are appropriately defined at the interface level, making them available to all XML nodes.
24-25: LGTM: Consistent implementation in pending parent
The null initialization of style properties aligns with the temporary nature of the pending parent element.
56-57: LGTM: Well-implemented property delegation
The properties are correctly implemented using the attribute delegate, with appropriate mapping of "className" to the "class" attribute.
102-111: Consider defensive copying of parent attributes
The direct reference to parent's attributes could lead to unintended modifications if the text node modifies the map.
class XmlTextNode(
parent: XmlParentNode,
val content: String,
) : XmlChildNode(parent) {
override val tagName: String = parent.tagName
- override val attributes: MutableMap<String, String> = parent.attributes
+ override val attributes: MutableMap<String, String> = parent.attributes.toMutableMap()
override fun toString(): String = toJsString {
append("\"content\": $content")
}
}118-119: LGTM: Consistent root node implementation
The null initialization of style properties is appropriate for the root node and follows the established pattern.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssAstParserTest.kt (3)
808-967: Verify precedence for multiple mixed-type selectors.
Though your code checks that different selector types (tag, class, ID) can appear together, adding a test for more complex specificity ordering (e.g., combining pseudo-classes, attribute selectors) might preempt future complexity issues.
1636-1672: Usage of the 'Burst' library for testing is properly applied.
This demonstrates a neat approach to enumerating multiple “BadUrlParam” cases in a single test. Good coverage for negative test scenarios.
1674-1682: Consider improving error message detail in test assertions.
Your “assert” method currently uses a straightforward equality check. Building on the previously suggested approach about improving debugging info, you might incorporate a “pretty print” or a more descriptive comparison to clarify where the AST structure deviates.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumer.kt (2)
1-10: LGTM! Well-organized imports
The package structure and imports are clean, specific, and well-organized.
21-25: LGTM! Well-structured class declaration
The class design follows good practices with appropriate visibility modifiers and inheritance structure.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt (1)
1-6: LGTM! Clean package structure and imports
The package naming and import statements follow best practices with specific imports and clear organization.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/gradient/SvgGradient.kt (1)
Line range hint 48-93: Verify performance impact of removing inline modifier
The removal of the inline modifier might affect performance in cases where these functions are called frequently. Consider measuring the performance impact.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt (1)
45-64: Consider handling nested blocks
The current implementation might not properly handle nested blocks of the same type. Consider adding a depth counter to track nesting level.
Let's verify if nested blocks are used in the codebase:
Consider implementing a stack-based approach for tracking nested blocks if they are part of your CSS subset.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgLinearGradientNode.kt (2)
19-20: Expanded return type for 'toBrush'
Changing the return type from ComposeBrush.Gradient.Linear to ComposeBrush.Gradient allows more flexible usage of different gradient types. Ensure that all call sites expecting a linear gradient can handle the broader ComposeBrush.Gradient appropriately.
20-38: Conditional block for 'href' handling
The newly added conditional block retrieves attributes from a referenced gradient and recursively calls toBrush on the adjusted node. This is a clean way to reuse existing gradient definitions, but watch out for:
- Potential null pointer if 'href' is missing from root.gradients.
- Overwrites or merges of attributes requiring thorough unit tests to ensure correct final state.
Would you like a shell script that searches for references to gradients[href] to confirm it handles missing keys?
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Value.kt (2)
21-27: Using 'String' as a data class name can cause confusion
This mirrors a previous comment about the collision risk. Consider renaming it to avoid overshadowing the Kotlin built-in type and confusion with its usage (e.g., requiring fully qualified references to kotlin.String).
43-49: Using 'Number' as a data class name can cause confusion
Likewise, you may wish to rename this to NumberValue or similar to avert overshadowing of numeric types in Kotlin.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ImageParser.kt (3)
136-147: New 'ComputedRule' data class
This is a helpful concept for bridging CSS specificity with the SVG DOM. Think about caching or indexing these rules if performance becomes a concern with large style sheets.
178-178: Invoke 'resolveStyleTags'
The new call ensures the SVG root node has its style tags resolved before proceeding. Verify that style resolution is robust for complex nested or multiple style elements.
180-180: Sequence of root node transformations
After resolving style tags, the next step is to resolve use nodes. Confirm no cyclical references or interactions cause unexpected attribute overrides.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgClipPath.kt (1)
6-6: New import for 'ComputedRule'
Integrating style computation into clip paths looks consistent with the rest of the changes. No issues here.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNode.kt (3)
26-27:
Implement toString method
The toString method is empty. It should generate a JavaScript string representation of the style node:
- override fun toString(): String = toJsString {
- }
+ override fun toString(): String = toJsString {
+ append("const style = document.createElement('style');")
+ append("style.textContent = ${content.replace("\"", "\\\"")};")
+ }Likely invalid or redundant comment.
19-23:
Add validation for content extraction
The content property assumes a text node will always be present and valid. Consider adding proper validation:
val content: String by lazy {
children
.firstInstanceOf<XmlTextNode>()
+ ?.content
+ ?: throw IllegalStateException("Style tag must contain valid CSS content")
- .content
}Likely invalid or redundant comment.
29-37:
Add content validation and improve thread safety
The tree resolution logic needs content validation and thread safety improvements:
+ @Volatile
+ private var _tree: StyleSheet? = null
+
internal fun resolveTree(parser: AstParser<CssTokenKind, StyleSheet>): StyleSheet {
+ if (content.isBlank()) {
+ throw IllegalStateException("Empty style content")
+ }
val current = _tree
return if (current != null) {
current
} else {
- val tokens = CssTokenizer().tokenize(content).toList()
- parser.parse(tokens).also { _tree = it }
+ synchronized(this) {
+ _tree ?: run {
+ val tokens = CssTokenizer().tokenize(content).toList()
+ parser.parse(tokens).also { _tree = it }
+ }
+ }
}
}Likely invalid or redundant comment.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgPathNode.kt (1)
61-62: Verify computedRules usage across the codebase
The addition of computedRules parameter needs verification of its usage across the codebase:
Also applies to: 88-88
✅ Verification successful
Based on the shell script results, I can now generate the final response:
computedRules parameter is consistently used across the codebase
The verification shows that:
computedRulesparameter is only used in clip path related operations- All
asNodeWrappercalls that handle clip paths consistently pass thecomputedRulesparameter - Other
asNodeimplementations don't needcomputedRulesas they don't handle clip paths
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for other occurrences of asNodeWrapper to ensure consistent parameter usage
rg -A 2 "asNodeWrapper\("
# Check for other implementations of asNode to ensure consistent parameter ordering
ast-grep --pattern 'fun $_.asNode($$$)'
Length of output: 25165
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt (2)
1-9: Well-structured test setup with Burst!
Good use of the Burst framework for parameterized testing, which will help reduce code duplication and make the tests more maintainable.
129-143: Well-documented memoization test!
Good test case with clear comments explaining the use of equals for instance comparison.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt (1)
13-29: Excellent documentation and interface design!
The KDoc comments are thorough and the interface is well-structured with clear separation of concerns.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/XmlParser.kt (1)
228-233: LGTM: Style tag content handling is well implemented
The special handling for text nodes within style tags is correctly implemented, creating XmlTextNode instances to preserve the CSS content.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt (1)
85-98: Consider using regex for more reliable unit extraction
The current implementation using takeLastWhile may not handle all valid CSS units correctly.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/SvgParser.kt (1)
193-198: LGTM: Style node handling is properly implemented
The SvgStyleNode creation and storage in the root's styles collection follows the established patterns and is correctly implemented.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt (2)
1-13: LGTM! Well-organized imports and package structure.
The imports are properly organized and all are being utilized in the test implementation.
17-107: LGTM! Well-structured basic test cases.
The test methods demonstrate:
- Clear naming convention following "when X then Y" pattern
- Consistent structure using Arrange-Act-Assert
- Comprehensive coverage of basic selector types
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/Consumer.kt
Outdated
Show resolved
Hide resolved
...ompose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt
Show resolved
Hide resolved
| CssTokenKind.OpenSquareBracket -> { | ||
| val next = iterator.expectNextToken(kind = CssTokenKind.Ident) | ||
| Selector.Attribute( | ||
| location = calculateLocation(current, next), | ||
| name = content.substring(next.startOffset, endIndex = next.endOffset), | ||
| matcher = null, | ||
| value = null, | ||
| combinator = lookupForCombinator(iterator), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Incomplete attribute selector implementation
The attribute selector implementation doesn't handle the matcher (e.g., =, ^=, $=) and value parts of the attribute selector syntax (e.g., [attr=value]).
Consider implementing the full attribute selector syntax:
CssTokenKind.OpenSquareBracket -> {
val next = iterator.expectNextToken(kind = CssTokenKind.Ident)
+ val matcher = when (iterator.peek(0)?.kind) {
+ CssTokenKind.Equals -> {
+ iterator.next()
+ Selector.Attribute.Matcher.Equals
+ }
+ CssTokenKind.PrefixMatch -> {
+ iterator.next()
+ Selector.Attribute.Matcher.PrefixMatch
+ }
+ // Add other matchers...
+ else -> null
+ }
+ val value = if (matcher != null) {
+ iterator.expectNextToken(CssTokenKind.String).value
+ } else null
+ iterator.expectNextToken(CssTokenKind.CloseSquareBracket)
Selector.Attribute(
location = calculateLocation(current, next),
name = content.substring(next.startOffset, endIndex = next.endOffset),
- matcher = null,
- value = null,
+ matcher = matcher,
+ value = value,
combinator = lookupForCombinator(iterator),
)
}Committable suggestion skipped: line range outside the PR's diff.
...compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Prelude.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgElementNode.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssParser.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssParser.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssSpecificity.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt
Show resolved
Hide resolved
This commit introduces a basic CSS lexer to parse CSS rules and extract tokens. The lexer can handle common CSS syntax, including: - Class and ID selectors - Properties and values - Hex color codes - URL values - Whitespace and comments The lexer generates a sequence of tokens, each representing a specific element in the CSS syntax. This sequence can then be used by a parser to build an abstract syntax tree (AST) for further processing.
This commit introduces a basic CSS lexer to parse CSS rules and extract tokens. The lexer can handle common CSS syntax, including: - Class and ID selectors - Properties and values - Hex color codes - URL values - Whitespace and comments The lexer generates a sequence of tokens, each representing a specific element in the CSS syntax. This sequence can then be used by a parser to build an abstract syntax tree (AST) for further processing.
This commit introduces a CSS AST (Abstract Syntax Tree) parser to analyze and extract style information from CSS code. The parser can handle various CSS rules, including class, ID, and tag selectors, as well as declarations with different property value types such as color, string literals, numbers, URLs, and more. This functionality is crucial for understanding and applying CSS styles within the project.
This commit adds support for identifier property values in the CSS parser. Identifier property values are represented by the `CssTokenKind.Identifier` token and are parsed into `PropertyValue.Identifier` objects. This change allows the parser to handle CSS properties with identifier values, such as `font-family`, `font-weight`, and `text-align`.
refactor: viewBox parsing logic, adding memoization and unit tests
chore: enhance aggregate selectors
This commit enhances the CSS lexer to support dimensions (e.g., px, em, rem) and percentages. The lexer can now recognize and tokenize dimensions and percentages, which are crucial for handling CSS properties related to sizes, lengths, and proportions. This enhancement improves the lexer's ability to parse CSS rules and extract more comprehensive information about styles.
This commit renames the `CssSelector` class to `CssComponent` to better reflect its purpose. The previous name, `CssSelector`, was misleading as it implied the class was solely responsible for representing CSS selectors. However, it also handles other CSS components such as declarations and rules. The new name, `CssComponent`, is more accurate and generic. This change is purely a refactoring and does not affect the functionality of the code.
This commit introduces support for more CSS features such as: - **Color parsing**: Added support for parsing a color value starting from either a '#' token or a valid color identifier (e.g., `rgb`, `rgba`, `hsl`, `hsla`, `currentColor`). - **Numeric values parsing**: Enhanced parsing of numeric values with units (e.g., '16px', '1.5em', '50%') using `Dimension` and `Percentage` value types. - **Pseudo-class parsing**: Implemented parsing of CSS pseudo-classes, including cases with parameters in parenthesis (e.g., `:not(.hidden)`, `:nth-child(2)`). - **At-rule parsing**: Introduced the ability to parse CSS at-rules (e.g., `@media`, `@font-face`). These changes improve the functionality of the parser by enhancing its capabilities in handling more complex CSS rules.
This commit removes unnecessary leading and trailing whitespaces in the generated CSS AST to ensure a more concise and accurate representation of the styles. Previously, the parser kept unnecessary whitespace tokens in the AST even when they were at the beginning or end of a token sequence. This change ensures that only meaningful whitespaces are included in the AST, resulting in a more compact and manageable data structure.
feat: add `String.prependIndent` function deprecating `String.indented` docs: add missing function ktdocs
refactor(css): rewrite CSS parser architecture to a modular, node-based structure refactor(css): improved tokenizer logic
This commit introduces the ability to parse CSS code within `<style>` tags in SVG files. It adds a new `SvgStyleNode` class to represent `<style>` elements and stores the parsed CSS as an Abstract Syntax Tree (AST) for later use.
…ead of QualifiedRule
This commit improves the CSS lexer's ability to tokenize numbers and dimensions, specifically handling cases where they start with a dot, plus, or minus sign. The following improvements were made: - Numbers and dimensions starting with a dot, plus, or minus are now correctly recognized as such. - The lexer now correctly identifies the start and end positions of these tokens. - The lexer handles leading whitespace in values correctly, ensuring that dimensions with leading whitespace are parsed as numbers.
rafaeltonholo
force-pushed
the
feat/add-support-style-tag
branch
from
c611b10 to
c802f53
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt (1)
102-111:⚠️ Potential issueCreate defensive copy of parent attributes
The current implementation directly references the parent's attributes map, which could lead to unintended modifications.
- override val attributes: MutableMap<String, String> = parent.attributes + override val attributes: MutableMap<String, String> = parent.attributes.toMutableMap()svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleSelectorConsumer.kt (3)
24-25:⚠️ Potential issueDefine missing selectorTokens variable
The
selectorTokensvariable used initerator.expectToken(selectorTokens)is not defined. This will cause a compilation error.
51-60:⚠️ Potential issueImplement complete attribute selector parsing
The attribute selector implementation is incomplete as it doesn't handle matchers (e.g., =, ^=, $=) and values.
116-131:⚠️ Potential issueAdd infinite loop protection in buildParameters
The while loop in buildParameters may become infinite if consume(iterator) doesn't advance the iterator.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt (1)
85-98:⚠️ Potential issueImprove unit extraction in 'parseDimensionToken'
The current method of extracting the unit using
takeLastWhile { char.isLetter() }may not handle all valid CSS units correctly.Apply this diff to improve unit extraction:
- private fun Iterator.parseDimensionToken(): Value.Dimension { - val current = expectToken(kind = CssTokenKind.Dimension) - val value = content.substring(startIndex = current.startOffset, endIndex = current.endOffset) - var unit = value.takeLastWhile { char -> char.isLetter() } - return Value.Dimension( - location = CssLocation( - source = value, - start = current.startOffset, - end = current.endOffset, - ), - value = value.removeSuffix(unit), - unit = unit, - ) - } + private fun Iterator.parseDimensionToken(): Value.Dimension { + val current = expectToken(kind = CssTokenKind.Dimension) + val value = content.substring(startIndex = current.startOffset, endIndex = current.endOffset) + val matchResult = Regex("""^([+-]?[\d.]+)([a-zA-Z%]+)$""").find(value) + ?: parserError(content, "Invalid dimension token: $value") + val (number, unit) = matchResult.destructured + return Value.Dimension( + location = CssLocation( + source = value, + start = current.startOffset, + end = current.endOffset, + ), + value = number, + unit = unit, + ) + }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt (1)
74-84:⚠️ Potential issueAdd error handling to style attribute parsing
The implementation needs error handling for malformed style attributes, as suggested in the previous review.
Additionally, consider adding validation for empty or whitespace-only values:
private fun MutableMap<String, String>.resolveAttributesByStyleAttribute() { style?.let { style -> val parts = style.split(";") for (part in parts) { + if (part.isBlank()) continue try { val (property, value) = part.split(":") if (property.isBlank() || value.isBlank()) continue if (attributes.containsKey(property).not()) { put(property.trim(), value.trim()) } } catch (e: Exception) { // Log warning about style parsing error } } } }
🧹 Nitpick comments (29)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/CssConsumers.kt (2)
47-56: Consider using read-only properties if feasible.
Although these are optional custom consumers, if you do not need them to be reassigned outside the builder’s scope, making them read-only (val) could prevent accidental mutations.
58-109: Builder ensures sensible defaults but consider lazy instantiation.
The code clearly initializes a default consumer if a custom one is not provided. However, consider lazy initialization to avoid unnecessary creation of objects if some consumers remain unused.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt (5)
15-19: Consider adding documentation for 'offset' visibility.
While this variable is marked internal and has a private setter, it would be helpful to document its intended usage and constraints more explicitly for maintainers.
24-32: Return null vs. throwing for 'next()' exhaustion.
Returning null when no tokens remain is a valid approach, but it introduces a requirement for callers to handle a nullable response. Optionally, consider throwing an exception or returning a sentinel token to avoid potential issues from callers ignoring a null value.
39-46: Clarify 'current()' method behavior in edge cases.
Because this method peeks one step back, if no token has been consumed yet, it will return null. Be sure to clarify this behavior in your method description or usage references.
139-153: 'parserCheck' is nearly identical to 'parserRequire'.
The difference in semantics may be minimal. Ensure the team understands which to use in specific parsing scenarios to avoid confusion.
175-191: Consider unifying 'parserCheckNotNull' with Kotlin's built-in contracts.
This method is good for clarity, though you might explore whether a standard Kotlin function or a small inline extension might suffice.svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt (1)
56-57: Consider adding KDoc for style propertyWhile the implementation is correct, the
styleproperty could benefit from documentation describing the expected CSS format and parsing behavior.+ /** + * The inline CSS styles for this node. + * Expected format: semicolon-separated CSS declarations (e.g., "color: red; margin: 10px") + */ override val style: String? by attribute()svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgPathNode.kt (2)
10-10: Consider moving ComputedRule to a dedicated packageThe nested location of
ComputedRulewithinSvgParsersuggests tight coupling. Consider moving it to a dedicated package likedev.tonholo.s2c.domain.cssto improve modularity and make the CSS-related types more discoverable.
88-88: LGTM! Consider adding documentationThe integration of
computedRuleswith clip paths is implemented correctly. Consider adding KDoc comments to document how CSS rules are applied to clip paths.ImageVectorNode.Group( params = ImageVectorNode.Group.Params( + // Forward computed CSS rules to clip path for proper styling inheritance clipPath = it.asNodeWrapper(computedRules, minified), ),svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt (3)
18-101: Consider extracting common test setup codeThe test is well-structured but contains repetitive location object creation. Consider creating a helper method or property for the common
CssLocationsetup to reduce boilerplate across all tests.private val dummyLocation = CssLocation( source = "", start = 0, end = 0 )
103-353: Remove debug println statementsThe
printlnstatements on lines 261 and 351 appear to be debugging artifacts and should be removed from the test code.- println(actual) assertEquals(expected, actual)Additionally, consider breaking down these complex test cases into smaller, more focused tests that verify specific aspects of the CSS tree construction.
355-422: Enhance test assertionsWhile the string representation test is good, consider adding more specific assertions to verify the internal structure of the generated CSS tree. This would make the tests more robust against implementation changes.
Example additional assertions:
// Add before toString() verification with(rule.prelude as Prelude.Selector) { assertEquals(1, components.size) with(components[0].selectors[1] as Selector.Attribute) { assertEquals("type", name) assertEquals("password", value) } }Also, remove the debug
printlnstatement on line 419.svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssParserTest.kt (2)
23-1710: Consider organizing tests into nested classesThe test class contains multiple test methods that could be organized into nested classes based on functionality (e.g.,
SelectorTests,DeclarationTests, etc.). This would improve readability and maintainability.Example refactor:
class CssParserTest { + @Nested + inner class SelectorTests { + @Test + fun `parse css class rule to StyleSheet`() { + // existing test code + } + + @Test + fun `parse css id rule to StyleSheet`() { + // existing test code + } + } + + @Nested + inner class DeclarationTests { + @Test + fun `parse css rule with number without unit`() { + // existing test code + } + } }
1711-1734: Add KDoc documentation for helper methods and enumsThe helper methods and enums would benefit from KDoc documentation explaining their purpose and usage.
Example improvement:
+ /** + * Asserts that the CSS parser correctly parses the given content and tokens + * into the expected StyleSheet. + * + * @param content The CSS content string + * @param tokens The list of CSS tokens + * @param expected The expected StyleSheet AST + */ private fun assert( content: String, tokens: List<Token<out CssTokenKind>>, expected: StyleSheet, ) { // existing code } + /** + * Test parameters for bad URL scenarios in CSS. + * Used with the Burst test framework for parameterized testing. + */ @Suppress("unused", "EnumEntryName") enum class BadUrlParam(private val value: String) { // existing code }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumer.kt (2)
11-20: Consider enhancing documentation with usage example.While the documentation is clear, adding a code example would make it more developer-friendly by demonstrating how to properly instantiate and use the consumer.
Add something like:
/** * Consumes a CSS stylesheet and builds a [StyleSheet] object. * * This class iterates through the CSS tokens and delegates the consumption of * at-rules and qualified rules to specific consumers. * * @param content The CSS content string. * @param atRuleConsumer The consumer responsible for handling at-rules. * @param qualifiedRuleConsumer The consumer responsible for handling qualified rules. + * + * Example usage: + * ``` + * val content = """ + * @media screen { + * .header { color: blue; } + * } + * """ + * val consumer = StyleSheetConsumer( + * content = content, + * atRuleConsumer = AtRuleConsumer(content), + * qualifiedRuleConsumer = QualifiedRuleConsumer(content) + * ) + * val styleSheet = consumer.consume(iterator) + * ``` */
28-32: Consider dynamic end position calculation.The location's end position is set to the content length before parsing. This might not accurately reflect the actual parsed content's end position if there are parsing errors or if the content is malformed.
Consider updating the end position after parsing is complete:
- val location = CssLocation( - source = content, - start = 0, - end = content.length, - ) + var currentPosition = 0 + val location = CssLocation( + source = content, + start = 0, + end = 0, // Will be updated after parsing + )Then update the end position after the while loop:
location.end = currentPositionsvg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleSelectorConsumer.kt (1)
62-92: Reduce code duplication in pseudo selectorsThe pseudo-element and pseudo-class implementations share significant code structure. Consider extracting the common logic into a private method.
+ private fun createPseudoSelector( + current: Token<CssTokenKind>, + iterator: AstParserIterator<CssTokenKind>, + createSelector: (CssLocation, String, List<Selector>, CssCombinator?) -> Selector + ): Selector { + val next = iterator.expectNextToken(kind = CssTokenKind.Ident) + val parameters = buildParameters(iterator) + val endOffset = if (parameters.isEmpty()) { + next.endOffset + } else { + parameters.last().location.end + 1 + } + return createSelector( + calculateLocation(current.startOffset, endOffset), + content.substring(next.startOffset, endIndex = next.endOffset), + parameters, + lookupForCombinator(iterator) + ) + } override fun consume(iterator: AstParserIterator<CssTokenKind>): Selector { // ... existing code ... - CssTokenKind.DoubleColon -> { - val next = iterator.expectNextToken(kind = CssTokenKind.Ident) - val parameters = buildParameters(iterator) - val endOffset = if (parameters.isEmpty()) { - next.endOffset - } else { - parameters.last().location.end + 1 - } - Selector.PseudoElement( - location = calculateLocation(current.startOffset, endOffset), - name = content.substring(next.startOffset, endIndex = next.endOffset), - parameters = parameters, - combinator = lookupForCombinator(iterator), - ) - } + CssTokenKind.DoubleColon -> createPseudoSelector(current, iterator, ::Selector.PseudoElement) - CssTokenKind.Colon -> { - val next = iterator.expectNextToken(kind = CssTokenKind.Ident) - val parameters = buildParameters(iterator) - val endOffset = if (parameters.isEmpty()) { - next.endOffset - } else { - parameters.last().location.end + 1 - } - Selector.PseudoClass( - location = calculateLocation(current.startOffset, endOffset), - name = content.substring(next.startOffset, endIndex = next.endOffset), - parameters = parameters, - combinator = lookupForCombinator(iterator), - ) - } + CssTokenKind.Colon -> createPseudoSelector(current, iterator, ::Selector.PseudoClass)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt (1)
30-32: Consider consistent parameter ordering across methodsThe
computedRulesparameter is positioned differently across method signatures:
- Second parameter in
asNode()andflatNode()- First parameter in
createGroupClipPath()Consider maintaining consistent parameter ordering across all methods for better maintainability.
private fun SvgGroupNode.createGroupClipPath( - computedRules: List<ComputedRule>, masks: List<SvgMaskNode>, + computedRules: List<ComputedRule>, minified: Boolean, )Also applies to: 52-54, 71-74
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (1)
514-617: Consider removing println calls in tests.Debugging logs like println("url: '$url'") (line 491, line 520) may clutter test outputs over time. It is typically preferable to rely on the test framework for reporting. If needed, consider a logger that can be toggled.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt (2)
8-13: Consider adding support for modern CSS color functionsThe
colorFunctionsset could be expanded to include modern CSS color functions for better compatibility:
hwb(Hue, Whiteness, Blackness)lablchcolor(CSS Color 4 specification)private val colorFunctions = setOf( "rgb", "rgba", "hsl", "hsla", + "hwb", + "lab", + "lch", + "color", )
27-50: Consider extracting error messages as constantsError messages like "Incomplete URL value..." and "Unexpected token..." could be extracted as private constants for better maintainability and consistency.
+ private companion object { + private const val INCOMPLETE_URL_ERROR = "Incomplete URL value: missing closing ')' in 'url(...)" + private const val UNEXPECTED_TOKEN_ERROR = "Unexpected token: %s" + }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt (2)
23-33: Add usage examples to documentationConsider enhancing the documentation with examples, similar to the
replaceDividersfunction./** * Converts a string to camel case. + * + * For example: + * "my-string" becomes "myString" + * "MY_STRING" becomes "myString" */
Line range hint
90-104: Enhance error handling in toPercentageConsider adding more specific error messages and handling edge cases.
fun String.toPercentage(): Float { val value = this.toFloatOrNull() return when { value == null && this.endsWith("%") -> this.removeSuffix("%").toInt() / PERCENT value != null -> value - else -> error("Invalid percentage value '$this'") + else -> throw IllegalArgumentException( + "Invalid percentage value '$this'. Expected a number or percentage (e.g., '50' or '50%')" + ) }.coerceIn(range = 0f..1f) }svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt (2)
86-110: Consider caching selector resolution resultsThe implementation correctly handles CSS specificity and value resolution. However, for better performance, consider caching the filtered and sorted rules per selector.
+private val selectorRulesCache = mutableMapOf<String, List<ComputedRule>>() private fun MutableMap<String, String>.resolveAttributesBySelector( computedRules: List<ComputedRule>, selector: String, ) { - val rulesPerSelector = computedRules - .filter { it.selector.endsWith(selector) } - .sortedDescending() + val rulesPerSelector = selectorRulesCache.getOrPut(selector) { + computedRules + .filter { it.selector.endsWith(selector) } + .sortedDescending() + }
Line range hint
114-144: Enhance stackedTransform documentationWhile the implementation is correct, consider adding:
- Example of how transforms are stacked
- Explanation of special handling for SvgDefsNode
- Description of the return value format
/** * Calculates the stacked transform of the node by traversing up the tree * and concatenating the transform attributes of the ancestors. + * + * Special handling is applied for SvgDefsNode, where the transform stack is reset + * to only include the current node's transform. + * + * For example: + * If the node has transform="rotate(45)" and its parent has transform="scale(2)", + * the stacked transform will be "scale(2) rotate(45)". + * * @param parent The parent node of the current node. - * @return The stacked transform of the node. + * @return The stacked transform of the node as a SvgTransform object, + * or null if no transforms are present. */svg-to-compose/src/nativeMain/kotlin/Main.kt (1)
29-29: Consider documenting the MANUAL_LINE_BREAK constant.Adding a KDoc comment explaining the purpose and usage of this formatting constant would improve maintainability.
+/** Manual line break character (U+0085) used for formatting help messages. */ private const val MANUAL_LINE_BREAK = "\u0085"svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ImageParser.kt (2)
147-147: Add KDoc for the rules variable.Document the purpose and lifecycle of this variable.
+/** List of computed CSS rules sorted by specificity. Populated by resolveStyleTags(). */ private var rules: List<ComputedRule> = emptyList<ComputedRule>()
206-224: Consider breaking down the resolveStyleTags function.The function has multiple responsibilities: parsing CSS, filtering rules, and mapping to ComputedRule objects.
Consider extracting the rule processing logic:
+private fun processStyleRule(rule: QualifiedRule): List<ComputedRule> = + rule.prelude.specificities.map { (selector, specificity) -> + ComputedRule( + selector = selector.location.source, + specificity = specificity, + declarations = rule.block.children, + ) + } private fun SvgRootNode.resolveStyleTags() { rules = styles .flatMap { style -> val parser = CssParser(consumers = CssConsumers(style.content)) style .resolveTree(parser) .children .filterIsInstance<QualifiedRule>() - .flatMap { rule -> - rule.prelude.specificities.map { (selector, specificity) -> - ComputedRule( - selector = selector.location.source, - specificity = specificity, - declarations = rule.block.children, - ) - } - } + .flatMap(::processStyleRule) } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
.run/svg-to-compose [allTests].run.xmlis excluded by!**/.run/**config/detekt.ymlis excluded by!**/config/detekt.ymlsamples/svg/css/house-with-trees.svgis excluded by!**/*.svg,!**/samples/**samples/svg/css/mountain-with-clouds.svgis excluded by!**/*.svg,!**/samples/**
📒 Files selected for processing (75)
buildSrc/build.gradle.kts(1 hunks)buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.kmp.gradle.kts(0 hunks)buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.testing.gradle.kts(1 hunks)gradle/libs.versions.toml(2 hunks)svg-to-compose/build.gradle.kts(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgClipPath.kt(2 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgElementNode.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGraphicNode.kt(0 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt(5 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgLinearGradientNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgMaskNode.kt(0 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgPathNode.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRootNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/gradient/SvgGradient.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt(4 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/Collections.extension.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt(4 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Token.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Tokenizer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/AtKeywordTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/CommentTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/IdentTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/NumberTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/TokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/WhitespaceTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ImageParser.kt(4 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/SvgParser.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/XmlParser.kt(3 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/AstParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/Element.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssCombinator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssParser.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssSpecificity.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/Consumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/CssConsumers.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/DeclarationConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/QualifiedRuleConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SelectorListItemConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleSelectorConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/AstParserException.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/AtRulePrelude.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Comment.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Declaration.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Prelude.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Rule.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Selector.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/StyleSheet.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Value.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssParserTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt(1 hunks)svg-to-compose/src/jvmMain/kotlin/Main.kt(2 hunks)svg-to-compose/src/nativeMain/kotlin/Main.kt(5 hunks)
💤 Files with no reviewable changes (3)
- buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.kmp.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgMaskNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGraphicNode.kt
✅ Files skipped from review due to trivial changes (1)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/DeclarationConsumer.kt
🚧 Files skipped from review as they are similar to previous changes (52)
- buildSrc/build.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Token.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenKind.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/Tokenizer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/AtRulePrelude.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgClipPath.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/StyleSheet.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/AstParser.kt
- svg-to-compose/src/jvmMain/kotlin/Main.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Declaration.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssParser.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Comment.kt
- svg-to-compose/build.gradle.kts
- buildSrc/src/main/kotlin/dev.tonholo.s2c.conventions.testing.gradle.kts
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/Collections.extension.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/SvgParser.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/QualifiedRuleConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/AtRuleConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssIterator.kt
- gradle/libs.versions.toml
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgRootNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/CommentTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenIterator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenKind.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/TokenIterator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Prelude.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/UrlTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Rule.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/Element.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgElementNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/IdentTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Selector.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/TokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssSpecificity.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/XmlParser.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/gradient/SvgGradient.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/DirectTokenConsumer.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumerTest.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/AtKeywordTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/WhitespaceTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/CssCombinator.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SelectorListItemConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/NumberTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/StringTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Value.kt
- svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgViewBoxTest.kt
🔇 Additional comments (42)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/CssConsumers.kt (3)
16-16: Good use of DSL marker.
This annotation helps enforce DSL usage restrictions and clarifies the scope for the builder methods.
32-42: Solid interface definition.
The interface neatly describes the various consumer properties while remaining concise. It is clear which parts of CSS parsing each consumer handles.
112-122: DSL function is well-structured.
The function correctly applies the builder pattern and ensures a streamlined DSL usage, providing clarity when configuring custom consumers.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/Consumer.kt (4)
23-29: Well-structured usage of an abstract base class
By defining an abstract class with a generic type parameter for CssNode, you've set up a clean, extensible foundation for specialized consumers. This design choice promotes reusability and makes it easier to implement additional CSS consumers in the future.
34-45: Robust token validation approach
Your approach of ensuring the current token's kind matches the expected kind by using parserCheck and parserCheckNotNull is clear and safe. It provides excellent debugging context if a mismatch occurs, helping developers quickly identify the source of parsing errors.
50-61: Ensure safe usage of “next()” to avoid unintentional token consumption
Similar to the previously mentioned concern about peeking tokens, consider a dedicated “peekNextToken(...)” helper if you need to see upcoming tokens without consuming them. Otherwise, using “next()” is acceptable for scenarios in which you truly intend to advance the iterator.
83-90: Comprehensive checks for non-null tokens
Using parserCheckNotNull here ensures that an unexpected null token is flagged in a clear manner. This consistently guards against parsing edge cases where token streams might be incomplete or malformed.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/iterator/AstParserIterator.kt (5)
20-22: Method 'hasNext' is straightforward and clear.
The check against the token list size is concise and self-explanatory.
33-38: 'peek' method usage.
The ability to peek multiple steps is useful for lookahead. Ensure that callers handle the possibility of a null return for out-of-bounds conditions.
47-52: Potential off-by-one risks in 'rewind'.
Adjusting the offset by the given steps is flexible, but can become negative if used improperly. Consider validating that offset remains non-negative or clearly documenting the side effects of rewinding beyond the beginning.
69-83: Robust 'parserError' usage.
Throwing a custom exception with context is a great way to improve debugging. Ensure that any invoked logging or external error handling does not inadvertently expose sensitive data if the content is large or includes PII.
106-120: 'parserRequire' inline function.
This is a helpful contract-based check for ensuring syntax validity. The lazyMessage callback is an excellent pattern, as it avoids unnecessary string building when the predicate is true.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/AstParserException.kt (1)
22-58: Exception message building is thorough.
The contextual snippet showing the offending token is very helpful for debugging. Minor considerations:
• Large content snippets might need truncation for extremely long strings.
• Ensure token offsets are valid when dealing with near-boundary indices.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt (3)
10-11: LGTM: Well-structured interface additions
The new nullable properties className and style are appropriate additions for CSS support, following Kotlin conventions.
24-25: LGTM: Consistent implementation in pending element
The null implementations align with the temporary nature of the pending parent element.
118-119: LGTM: Appropriate root node implementation
The null implementations are consistent with the root node's role as a document container.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgPathNode.kt (1)
61-62: Verify impact of parameter reordering
The reordering of parameters (moving minified after computedRules) might affect existing callers if they're using named arguments. Let's verify the impact.
✅ Verification successful
No impact from parameter reordering
All existing calls to asNode are using named arguments for the minified parameter, which means the parameter reordering won't affect them. The new computedRules parameter has a default value of emptyList(), making it backward compatible with existing code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for existing calls to asNode with named arguments
rg "asNode\(.*minified\s*=.*\)" --type kotlin
Length of output: 11398
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt (1)
1-17: LGTM! Well-structured test class setup
The imports and class structure are clean and well-organized, using appropriate testing framework components.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssParserTest.kt (1)
1-22: LGTM! Well-organized imports
The imports are properly organized and include all necessary dependencies for testing CSS parsing functionality.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/StyleSheetConsumer.kt (3)
1-10: LGTM! Package and imports are well-organized.
The imports are properly scoped and all are being utilized in the implementation.
21-25: LGTM! Class structure is well-designed.
The class follows good design principles with appropriate visibility modifiers and inheritance structure.
26-50: 🛠️ Refactor suggestion
Add error handling for malformed CSS.
The current implementation might not gracefully handle malformed CSS content. Consider adding proper error handling and validation.
- Add input validation:
+ init {
+ require(content.isNotBlank()) { "CSS content cannot be blank" }
+ }- Add error handling:
override fun consume(iterator: AstParserIterator<CssTokenKind>): StyleSheet {
+ try {
val children = mutableListOf<CssStatementNode>()
// ... existing code ...
+ } catch (e: Exception) {
+ throw CssParseException("Failed to parse CSS: ${e.message}", e)
+ }
}- Make StyleSheet immutable:
- children = children,
+ children = children.toList(),svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt (2)
8-8: LGTM: Import addition for CSS rule support
The addition of the ComputedRule import aligns with the new CSS processing capabilities.
43-45: LGTM: Proper propagation of computed styles
The changes correctly pass computed rules to child nodes, ensuring consistent style application throughout the SVG hierarchy.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (5)
9-43: Well-structured test for class-based selectors.
Your test method effectively checks that class selectors (e.g. ".my-rule") and accompanying declarations (e.g. background, color) tokenize correctly, verifying each token's startOffset and endOffset. This provides valuable coverage for basic selector handling.
81-144: Robust coverage of multiple rules in a single stylesheet.
This test is a great example that covers scenarios with multiple selectors in one input (".my-rule" and "#my-rule"). Verifying the correct token offsets and ensuring no cross-contamination between subsequent rules are crucial steps. Good job!
485-512: Good use of parameterized tests for incomplete URLs.
Using @burst with BadUrlTokenParam is a neat way to confirm that the tokenizer recognizes badly formed or partially complete URLs (e.g., missing parentheses or quotes) as BadUrl tokens. This thoroughly ensures that URL edge cases are handled gracefully.
681-700: Duplicate Review Comment: Syntax error in function name.
This test method's name "fun create tokens for a rule with BadUrl with (badString: BadStringParams)" appears to have mismatched or extraneous backticks, which can lead to parsing errors in Kotlin. This was previously flagged in an older review and remains an issue.
703-706: Concise and reusable assertion logic.
Your custom assertTokens function streamlines validation of expected versus actual tokens. This improves readability and reduces duplication across all test methods.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt (1)
15-26: LGTM! Well-documented class
The class documentation clearly explains its purpose and responsibilities.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgLinearGradientNode.kt (1)
19-39: LGTM! Verify gradient references in the codebase
The implementation correctly handles gradient references and attribute inheritance. The return type change provides better flexibility for different gradient types.
✅ Verification successful
Implementation safely handles gradient references
The code implementation is safe and well-structured:
- Gradients are stored in a central map in the root node and accessed via normalized IDs
- The href handling properly copies attributes while preserving the original gradient
- No signs of recursive gradient processing that could cause infinite loops
- The gradient hierarchy is well-defined with proper abstract classes and implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential circular gradient references that could cause infinite recursion
ast-grep --pattern 'class $_ : SvgLinearGradient<$_> {
$$$
fun toBrush($_) {
$$$
href
$$$
}
}'
Length of output: 114
Script:
#!/bin/bash
# Let's try a different approach to find gradient references
# First, let's check the pattern of linear gradient definitions
ast-grep --pattern 'class SvgLinearGradientNode {
$$$
}'
# Also search for href usage in gradient context
rg -B2 -A2 "href.*normalizedId\(\)"
Length of output: 1858
Script:
#!/bin/bash
# Let's check how gradients are stored and accessed in the root node
ast-grep --pattern 'class SvgRootNode {
$$$
val gradients
$$$
}'
# And check for any recursive gradient processing
rg -B3 -A3 "fun toBrush.*target.*\{"
Length of output: 111
Script:
#!/bin/bash
# Let's try a more specific search for gradient-related code
rg -B2 -A2 "gradients\[.*\]"
# And look for any gradient-related classes and their structure
fd -e kt -x cat {} | rg -B2 -A2 "class.*Gradient"
Length of output: 4732
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt (3)
7-22: LGTM! Well-documented utility function
The implementation is clean and efficient, with clear documentation and examples.
35-44: Properly deprecates indented function
The deprecation notice and replacement implementation align with the previous review comments about updating usages of the deprecated indented function.
61-80: LGTM! Excellent documentation
The comprehensive documentation with clear examples and parameter descriptions significantly improves code maintainability.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgNode.kt (1)
29-29: Fixed potential infinite recursion
The implementation properly addresses the infinite recursion issue by moving the logic to a private helper function.
Also applies to: 47-50
svg-to-compose/src/nativeMain/kotlin/Main.kt (2)
41-49: LGTM! Well-structured help formatter configuration.
The context block properly configures MordantMarkdownHelpFormatter with clear defaults and required option marking.
82-88: LGTM! Clear and well-formatted help text.
The help text for receiverType is well-structured with good examples.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/elements/CssSpecificityTest.kt (3)
14-16: LGTM! Well-structured test class setup.
Good practice using a shared location object for all tests.
109-146: LGTM! Excellent use of parameterized testing.
The Burst annotation effectively tests multiple scenarios for ID selectors. The test is well-structured with clear arrange, act, and assert sections.
226-296: LGTM! Comprehensive test coverage for complex selectors.
The test cases cover a wide range of selector combinations including pseudo-classes, attributes, and combinators. The expected specificity values are well-documented.
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ImageParser.kt (2)
137-145: LGTM! Well-designed ComputedRule class.
The class properly implements Comparable for specificity-based sorting, and the data structure is clean and focused.
179-184: Verify style resolution order.
The order of resolveStyleTags() and resolveUseNodes() calls might affect the final styling. Ensure that use nodes are properly styled.
✅ Verification successful
The order of style and use node resolution is correct
The current implementation is safe because:
resolveStyleTags()processes CSS rules from style tags first, creating a complete set of style rulesresolveUseNodes()then resolves use nodes, which includes copying attributes and transformations to the replacement nodes- When use nodes are resolved, they inherit both inline styles and the already-processed CSS rules from style tags
The order ensures that all styles are properly computed before node resolution, allowing use nodes to correctly inherit styles from their references.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if there are any SVG files with both style tags and use elements
# to verify the interaction between style resolution and use node resolution
rg --type xml '<style' -C 5 | rg '<use'
Length of output: 41
Script:
#!/bin/bash
# Let's check the implementation of resolveStyleTags and resolveUseNodes to understand their behavior
ast-grep --pattern 'fun resolveStyleTags() {
$$$
}'
echo "---"
ast-grep --pattern 'fun resolveUseNodes() {
$$$
}'
echo "---"
# Also check if there are any style-related operations in resolveUseNodes
rg -A 5 "style" --type kotlin
Length of output: 22931
Script:
#!/bin/bash
# Let's check the implementation of resolveStyleTags and how styles are resolved
ast-grep --pattern 'fun resolveStyleTags() {
$$$
}'
echo "---"
# Let's also check how styles are applied to use nodes when resolved
ast-grep --pattern 'fun resolve(): SvgNode {
$$$
}'
Length of output: 126
Script:
#!/bin/bash
# Let's search for style resolution related code with more context
rg -g '*.kt' -B 2 -A 5 "resolveStyleTags"
echo "---"
# Let's also check how use nodes handle style attributes
rg -g '*.kt' -B 2 -A 5 "class SvgUseNode"
Length of output: 2868
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/CssTreeTest.kt
Show resolved
Hide resolved
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssParserTest.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt
Outdated
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt
Show resolved
Hide resolved
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt
Show resolved
Hide resolved
refactor(css): improve code readability chore: add more dimension tests
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (16)
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssParserTest.kt (5)
24-1635: Consider adding more edge cases to improve test coverage.While the existing test cases cover the basic scenarios well, consider adding tests for:
- Empty selectors (e.g.,
{})- Invalid CSS property values
- Malformed declarations (missing values, missing semicolons)
- Nested at-rules
- CSS comments
- Invalid color values
- Complex combinators (>, +, ~)
1638-1673: Enhance Burst test assertions.The test could benefit from more specific assertions:
- Assert the exact position where the parsing failed
- Verify the specific type of parsing error
- Check if the error message contains the problematic URL value
Consider adding assertions like:
// Assert val message = exception.message assertNotNull(message) assertContains(message, "Incomplete URL value: missing closing ')' in 'url(...)") +assertContains(message, url.toString()) +assertEquals(28, exception.position) +assertTrue(exception.cause is BadUrlException)
1711-1719: Add documentation to the assert helper method.Consider adding KDoc to explain the purpose and parameters of this helper method.
+/** + * Asserts that the CSS parser correctly parses the given content and tokens into the expected StyleSheet. + * + * @param content The CSS content to parse + * @param tokens The list of tokens to parse + * @param expected The expected StyleSheet after parsing + */ private fun assert( content: String, tokens: List<Token<out CssTokenKind>>, expected: StyleSheet, ) {
1722-1733: Add documentation to the BadUrlParam enum.Consider adding KDoc to explain the purpose of each enum value and its test scenario.
+/** + * Test parameters for invalid URL scenarios in CSS. + */ enum class BadUrlParam(private val value: String) { + /** Tests parsing of empty URL values */ `with an empty url`(""), + /** Tests parsing of URLs with hash symbols */ `with a hashed url`("#abc"), // Add documentation for other cases...
23-1734: Well-structured test implementation with room for enhancement.The test suite demonstrates good practices:
- Clear test method names
- Comprehensive test cases for common scenarios
- Good separation of concerns
- Proper error handling tests
However, consider:
- Adding more edge cases as suggested above
- Improving assertion messages for better test failure debugging
- Adding performance tests for large CSS files
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt (4)
8-13: Consider making colorFunctions private and constThe
colorFunctionsset should be marked asprivate constsince it's a compile-time constant that's only used within this file.-private val colorFunctions = setOf( +private const val colorFunctions = setOf(
173-174: Extract magic numbers into named constantsThe offsets
4and1should be extracted into named constants for better maintainability.+ private companion object { + private const val URL_PREFIX_LENGTH = 4 // "url(" + private const val URL_SUFFIX_LENGTH = 1 // ")" + } - val urlContentStartOffset = current.startOffset + 4 - val urlContentEndOffset = current.endOffset - 1 + val urlContentStartOffset = current.startOffset + URL_PREFIX_LENGTH + val urlContentEndOffset = current.endOffset - URL_SUFFIX_LENGTH
43-48: Enhance error message with token detailsConsider including the token's value and position in the error message for better debugging.
- message = "Unexpected token: $current", + message = "Unexpected token: ${current.kind} at position ${current.startOffset} with value '${content.substring(current.startOffset, current.endOffset)}'",
57-64: Extract common location and value creation logicConsider creating helper methods to reduce code duplication in location and value creation across different parse methods.
+ private fun createLocation(startOffset: Int, endOffset: Int): CssLocation { + val source = content.substring(startIndex = startOffset, endIndex = endOffset) + return CssLocation( + source = source, + start = startOffset, + end = endOffset, + ) + }Also applies to: 72-79, 89-97, 105-112, 120-127
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (3)
8-8: Solid test coverage but consider splitting for manageability.
The file is quite extensive, covering many scenarios. While this is beneficial for comprehensive coverage, it could become cumbersome to maintain. Splitting the tests into multiple classes (e.g., basic selectors, at-rules, advanced features) would improve clarity and maintainability.
249-267: Consider adding tests for sub-selector or attribute selectors.
Though you test combined selectors with spaces and child relationships, you might also consider scenarios like “[attr=value]”. This will help ensure the tokenizer coverage extends to attribute selectors.Shall I propose additional tests for attribute selectors?
578-583: Remove or replace println statements.
Using println in tests can clutter the output. Consider using a logger or removing these statements occasionally to keep test output clean.svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNodeTest.kt (4)
3-24: Consider organizing imports by sourceThe imports could be better organized by grouping them into:
- Kotlin standard library imports
- Project domain imports
- Project parser imports
-import dev.tonholo.s2c.domain.xml.XmlNode -import dev.tonholo.s2c.domain.xml.XmlRootNode -import dev.tonholo.s2c.domain.xml.XmlTextNode -import dev.tonholo.s2c.parser.ast.css.CssParser -import dev.tonholo.s2c.parser.ast.css.consumer.CssConsumers -import dev.tonholo.s2c.parser.ast.css.syntax.AstParserException -import dev.tonholo.s2c.parser.ast.css.syntax.node.AtRule -import dev.tonholo.s2c.parser.ast.css.syntax.node.AtRulePrelude -import dev.tonholo.s2c.parser.ast.css.syntax.node.Block -import dev.tonholo.s2c.parser.ast.css.syntax.node.CssLocation -import dev.tonholo.s2c.parser.ast.css.syntax.node.Declaration -import dev.tonholo.s2c.parser.ast.css.syntax.node.Prelude -import dev.tonholo.s2c.parser.ast.css.syntax.node.QualifiedRule -import dev.tonholo.s2c.parser.ast.css.syntax.node.Selector -import dev.tonholo.s2c.parser.ast.css.syntax.node.SelectorListItem -import dev.tonholo.s2c.parser.ast.css.syntax.node.StyleSheet -import dev.tonholo.s2c.parser.ast.css.syntax.node.Value -import kotlin.test.Test -import kotlin.test.assertContains -import kotlin.test.assertEquals -import kotlin.test.assertFailsWith +// Kotlin test imports +import kotlin.test.Test +import kotlin.test.assertContains +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +// Domain imports +import dev.tonholo.s2c.domain.xml.XmlNode +import dev.tonholo.s2c.domain.xml.XmlRootNode +import dev.tonholo.s2c.domain.xml.XmlTextNode + +// Parser imports +import dev.tonholo.s2c.parser.ast.css.CssParser +import dev.tonholo.s2c.parser.ast.css.consumer.CssConsumers +import dev.tonholo.s2c.parser.ast.css.syntax.AstParserException +import dev.tonholo.s2c.parser.ast.css.syntax.node.AtRule +import dev.tonholo.s2c.parser.ast.css.syntax.node.AtRulePrelude +import dev.tonholo.s2c.parser.ast.css.syntax.node.Block +import dev.tonholo.s2c.parser.ast.css.syntax.node.CssLocation +import dev.tonholo.s2c.parser.ast.css.syntax.node.Declaration +import dev.tonholo.s2c.parser.ast.css.syntax.node.Prelude +import dev.tonholo.s2c.parser.ast.css.syntax.node.QualifiedRule +import dev.tonholo.s2c.parser.ast.css.syntax.node.Selector +import dev.tonholo.s2c.parser.ast.css.syntax.node.SelectorListItem +import dev.tonholo.s2c.parser.ast.css.syntax.node.StyleSheet +import dev.tonholo.s2c.parser.ast.css.syntax.node.Value
27-61: Consider extracting common setup codeThe first two tests share similar setup code for creating the SvgStyleNode. Consider extracting this to a helper method or using @BeforeTest to reduce duplication.
+ private fun createSvgStyleNode(children: MutableSet<XmlNode> = mutableSetOf()) = SvgStyleNode( + parent = XmlRootNode(children = mutableSetOf()), + children = children, + attributes = mutableMapOf(), + ) + @Test fun `given a style tag with no children nodes - when resolving the css tree - then must throw an exception`() { - val style = SvgStyleNode( - parent = XmlRootNode(children = mutableSetOf()), - children = mutableSetOf(), - attributes = mutableMapOf(), - ) + val style = createSvgStyleNode()
63-285: Consider extracting test data to constantsThe CSS test data and expected trees could be moved to companion object constants to improve readability and maintainability.
companion object { private const val SIMPLE_CSS_RULE = "body { color: red; }" private const val MULTIPLE_CSS_RULES = "body { color: red; } p { margin: 0; }" // ... more constants }
287-306: Enhance error testingThe invalid CSS test case could be expanded to cover more error scenarios:
- Invalid selectors
- Invalid properties
- Invalid values
- Malformed media queries
@Test fun `given invalid selector - when resolving css tree - then must throw exception`() { val styleContent = "123invalid { color: red; }" // ... test implementation } @Test fun `given invalid property - when resolving css tree - then must throw exception`() { val styleContent = "body { invalid-property: red; }" // ... test implementation }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt(5 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt(1 hunks)svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Rule.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNodeTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt(1 hunks)svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/parser/ast/css/CssParserTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/CssNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/token/consumer/HashTokenConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Block.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/SimpleBlockConsumer.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/syntax/node/Rule.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNode.kt
- svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgGroupNode.kt
🔇 Additional comments (9)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/parser/ast/css/consumer/ValueConsumer.kt (4)
85-98: Improve unit extraction in parseDimensionToken
The current method of extracting the unit using takeLastWhile { char.isLetter() } may not handle all valid CSS units correctly.
133-165: Add safety check for maximum iterations in function parsing
The do-while loop could potentially run indefinitely with malformed input.
222-261: Add validation for hex color format
The method should validate that hex colors have the correct number of digits.
24-26: Well-structured CSS value consumer implementation
The class is well-organized with clear separation of concerns and comprehensive documentation.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/lexer/css/CssTokenizerTest.kt (5)
83-94: Kudos on testing multiple rules in a single string.
These lines effectively validate handling of consecutive CSS rule blocks. This approach ensures that edge cases involving whitespace and spacing between rules are thoroughly tested.
203-210: Good use of combined selectors with element, class, and ID.
Comprehensive coverage for complex selectors is vital. This ensures the tokenizer handles chaining correctly and preserves the correct token boundaries.
690-715: Impressive dynamic token construction.
The approach of building tokens programmatically for different offsets is comprehensive and robust. It ensures the code under test is thoroughly verified against varied inputs.
873-877: Assertion helper centralizes token comparison logic nicely.
The single assertion method promotes DRY principles by preventing repetitive comparison logic. Great approach.
911-995: Extensive coverage for numeric and dimension tokens.
The enumerations for numeric, dimension, and derivative forms ensure broad coverage. Testing scientific notation, sign variations, and unit suffixes fosters high confidence in the tokenizer’s accuracy.
svg-to-compose/src/commonTest/kotlin/dev/tonholo/s2c/domain/svg/SvgStyleNodeTest.kt
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
ComputedRuleto enhance CSS specificity handling.asNodemethods across various SVG node classes to accept computed rules.Bug Fixes
Tests
Documentation