Skip to content

refactor(rtf): split the outline reader into per-construct methods (S3776) - #1399

Merged
dylanmtaylor merged 1 commit into
mainfrom
fix/dk-s3776
Oct 26, 2026
Merged

dylanmtaylor merged 1 commit into
mainfrom
fix/dk-s3776

Conversation

@dylanmtaylor

@dylanmtaylor dylanmtaylor commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Cognitive complexity of rtf_to_outline was 88 (limit 30). The reader's state now lives in a struct, with one method per construct (groups, control words, \u and ' escapes, symbols, text, flush). Behaviour is unchanged; deckcraft-format's 40 tests pass.

Summary by CodeRabbit

  • Refactor
    • Reorganized the internal handling of RTF outline parsing. The parser’s existing behavior and generated output remain unchanged, so no user-visible changes are expected. This update does not add or remove functionality.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between aa3a1ba and 3c2ea85.

Files selected for processing: 1
  • crates/format/src/rtf.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

rtf_to_outline now delegates its parsing loop and local state to an Outline struct and its methods. The parser handles RTF groups, control words and symbols, Unicode fallback, text extraction, indentation, and output flushing.

Changes: RTF outline parsing

Layer / File(s) Summary
Parser state and entry point
crates/format/src/rtf.rs
St and Outline hold parser and formatting state. rtf_to_outline initializes and runs the parser, flushes the final line, and finishes surrogate pairs.
Groups, controls, and Unicode handling
crates/format/src/rtf.rs
Parser methods handle groups, escapes, control words, Unicode units, fallback characters, and supported punctuation.
Text scanning and outline output
crates/format/src/rtf.rs
Parser methods decode hex escapes, handle control symbols, scan plain text, and flush trimmed lines with calculated indentation.

Priority: Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Refactor

Possibly related PRs

Suggested reviewers: nikitkara, aiongg

Merge Risk: Minimal · up to 3c2ea

No actionable issue is established for RTF import; the change is mergeable after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 69.23% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: refactoring the RTF outline reader into per-construct methods to address S3776.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@sonarqubecloud

sonarqubecloud Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

@dylanmtaylor
dylanmtaylor merged commit c93e0a0 into main Oct 26, 2026
11 checks passed
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