Skip to content

refactor: remove unnecessary inline modifiers and deprecated indented function#209

Closed
rafaeltonholo wants to merge 1 commit intomainfrom
fix/warnings
Closed

refactor: remove unnecessary inline modifiers and deprecated indented function#209
rafaeltonholo wants to merge 1 commit intomainfrom
fix/warnings

Conversation

@rafaeltonholo
Copy link
Owner

@rafaeltonholo rafaeltonholo commented Mar 5, 2026

Summary by CodeRabbit

  • Refactor

    • Optimized indentation handling for generated code output.
    • Simplified parent traversal logic in node hierarchy.
    • Removed performance hints from utility functions to improve code maintainability.
  • Chores

    • Removed deprecated indentation helper function.

… function

Remove `inline` modifiers from functions that don't benefit from inlining, delete deprecated `String.indented()` in favor of `prependIndent()`, simplify `rootParent` lazy initialization loop, and clean up redundant `else` branch in `SvgElementNode.copy()`.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 5, 2026

Walkthrough

This PR refactors indentation handling by replacing a custom indented() extension with the standard prependIndent() function across multiple files, removes the inline modifier from several functions and extensions, removes deprecated code, and simplifies some control flow logic.

Changes

Cohort / File(s) Summary
Indentation Migration
ImageVectorNode.kt, ComposeBrush.kt, PathNodes.kt
Replaces custom .indented() calls with standard .prependIndent() for consistent Kotlin code generation formatting.
Inline Modifier Removal
PathCommand.kt, AvgNode.kt, ExitProgramException.kt, String.extension.kt
Removes inline modifier from extension and top-level functions without changing logic or signatures.
Deprecated Code Cleanup
String.extension.kt
Removes deprecated String.indented() extension function and inline modifier from String.removeTrailingZero().
Control Flow Adjustments
XmlNode.kt
Simplifies rootParent computation by replacing do-while with while-loop traversal of parent chain.
Node Copying Logic
SvgElementNode.kt
Removes when-branch fallback that previously preserved non-SvgChildNode/non-SvgElementNode types during copy operations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.79% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: removing unnecessary inline modifiers and the deprecated indented function across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/warnings

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

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt (1)

38-38: ⚠️ Potential issue | 🟡 Minor

prependIndent() indents every non-blank line, not just the first.

Kotlin's stdlib String.prependIndent(indent: String) prepends indentation to every non-blank line in a multi-line string. The previous indented() helper appears to have handled only the first line. This behavior difference will affect generated source formatting for multi-line fragments, including PathData blocks and gradient parameters already formatted before indentation.

If behavior-preserving refactoring is required, define a first-line-only helper for those embed sites instead of replacing the semantics globally.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt`
at line 38, The change to String.prependIndent(indentSize: Int) altered behavior
by using Kotlin's prependIndent which adds indentation to every non-blank line,
but callers (e.g., places that previously used indented() or expected only the
first line to be indented such as PathData and gradient parameter embedding)
relied on first-line-only indentation; revert this global semantics by restoring
the original first-line-only helper (or add a new function like
prependIndentFirstLine or indentedFirstLine) and update call sites that truly
need multi-line indentation to use the stdlib prependIndent, leaving functions
that embed single-line fragments (reference String.prependIndent and the
previous indented helper name) to call the first-line-only helper so generated
source formatting remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt`:
- Line 38: The change to String.prependIndent(indentSize: Int) altered behavior
by using Kotlin's prependIndent which adds indentation to every non-blank line,
but callers (e.g., places that previously used indented() or expected only the
first line to be indented such as PathData and gradient parameter embedding)
relied on first-line-only indentation; revert this global semantics by restoring
the original first-line-only helper (or add a new function like
prependIndentFirstLine or indentedFirstLine) and update call sites that truly
need multi-line indentation to use the stdlib prependIndent, leaving functions
that embed single-line fragments (reference String.prependIndent and the
previous indented helper name) to call the first-line-only helper so generated
source formatting remains unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 81cdbf99-63ab-48e6-9a04-4245a249804f

📥 Commits

Reviewing files that changed from the base of the PR and between dfa4693 and a265ff7.

📒 Files selected for processing (9)
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/ImageVectorNode.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/PathCommand.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/PathNodes.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/avg/AvgNode.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/compose/ComposeBrush.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgElementNode.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/xml/XmlNode.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/error/ExitProgramException.kt
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/extensions/String.extension.kt
💤 Files with no reviewable changes (1)
  • svg-to-compose/src/commonMain/kotlin/dev/tonholo/s2c/domain/svg/SvgElementNode.kt

@rafaeltonholo rafaeltonholo deleted the fix/warnings branch March 8, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant