Add support for <style> tags
#99
Conversation
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.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (40)
WalkthroughThe pull request introduces multiple changes across several files, primarily focusing on the addition and modification of dependencies, plugins, and the implementation of a CSS parsing system. Key updates include the addition of a new Gradle plugin and dependencies, enhancements to the Changes
Tip You can generate walkthrough in a markdown collapsible section to save space.Enable the 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
| override val transform: SvgTransform? by attribute<String?, SvgTransform?> { | ||
| var transform = it | ||
| if (viewportX != 0f || viewportY != 0f) { | ||
| // Apply translation if viewBox has an offset | ||
| transform = "translate(${-viewportX}, ${-viewportY})" | ||
| attributes[ATTR_TRANSFORM] = transform | ||
| } | ||
| transform?.let(::SvgTransform) | ||
| } |
There was a problem hiding this comment.
Preserve existing transform attributes when applying viewport offset
In the transform property, if viewportX or viewportY is not zero, the code overwrites any existing transform attribute with a translation. This means any transformations specified in the SVG root node will be lost. Consider combining the existing transform with the new translation to preserve all transformations.
Apply this diff to combine the transformations:
if (viewportX != 0f || viewportY != 0f) {
// Apply translation if viewBox has an offset
- transform = "translate(${-viewportX}, ${-viewportY})"
+ val translation = "translate(${-viewportX}, ${-viewportY})"
+ transform = if (transform != null) {
+ "$translation $transform"
+ } else {
+ translation
+ }
attributes[ATTR_TRANSFORM] = transform
}This change ensures that existing transform attributes are preserved and combined with the viewport offset.
📝 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.
| override val transform: SvgTransform? by attribute<String?, SvgTransform?> { | |
| var transform = it | |
| if (viewportX != 0f || viewportY != 0f) { | |
| // Apply translation if viewBox has an offset | |
| transform = "translate(${-viewportX}, ${-viewportY})" | |
| attributes[ATTR_TRANSFORM] = transform | |
| } | |
| transform?.let(::SvgTransform) | |
| } | |
| override val transform: SvgTransform? by attribute<String?, SvgTransform?> { | |
| var transform = it | |
| if (viewportX != 0f || viewportY != 0f) { | |
| // Apply translation if viewBox has an offset | |
| val translation = "translate(${-viewportX}, ${-viewportY})" | |
| transform = if (transform != null) { | |
| "$translation $transform" | |
| } else { | |
| translation | |
| } | |
| attributes[ATTR_TRANSFORM] = transform | |
| } | |
| transform?.let(::SvgTransform) | |
| } |
| @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(), | ||
| ) | ||
| val expected = floatArrayOf(0f, 0f, 300f, 150f) | ||
| val actual = svgNode.viewBox | ||
| assertContentEquals(expected, actual) | ||
| } |
There was a problem hiding this comment.
Test case appears to be identical to the missing viewBox test.
The test case for incomplete viewBox is exactly the same as the missing viewBox test, but it doesn't actually test an incomplete viewBox scenario. The test should include a malformed viewBox attribute to properly test the handling of incomplete data.
Consider modifying the test to actually test an 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(
+ "viewBox" to "0 0 300", // Incomplete viewBox with missing height
),
)
val expected = floatArrayOf(0f, 0f, 300f, 150f)
val actual = svgNode.viewBox
assertContentEquals(expected, actual)
}📝 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.
| @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(), | |
| ) | |
| val expected = floatArrayOf(0f, 0f, 300f, 150f) | |
| val actual = svgNode.viewBox | |
| assertContentEquals(expected, actual) | |
| } | |
| @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( | |
| "viewBox" to "0 0 300", // Incomplete viewBox with missing height | |
| ), | |
| ) | |
| val expected = floatArrayOf(0f, 0f, 300f, 150f) | |
| val actual = svgNode.viewBox | |
| assertContentEquals(expected, actual) | |
| } |
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)
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.
TBD
Summary by CodeRabbit
New Features
Bug Fixes
Tests