fix(theme): tolerate unknown spacing tokens instead of throwing

[thejesh23]

Fixes #155

Summary

WindThemeData.getSpacing throws ArgumentError for any multiplier that isn't 'full', a container key, or numeric. The padding, margin, position, and flex-gap parsers each pass the raw regex-captured value directly, so tokens like p-primary, m-foo, top-abc, gap-x-blue — natural typos after using bg-primary/text-primary — throw at build time and produce a red screen. This violates the parser contract at wind_parser.dart:369 (unknown className is dropped with a debug warning); the sibling sizing_parser.dart already pre-validates with double.tryParse.

Adds WindThemeData.tryGetSpacing(String) — same behaviour as getSpacing but returns null for unrecognised tokens. Switches the four throwing callers to use it and drop the token via their existing if (value == null) continue; guards. getSpacing is unchanged.

Definition of done

• dart analyze — please run in CI (no toolchain on audit machine).
• dart format . — please run in CI.
• flutter test — added coverage:
  - test/theme/wind_theme_data_test.dart — tryGetSpacing matches getSpacing for known tokens; returns null for unknown.
  - test/parser/parsers/padding_parser_test.dart — p-primary/mx-foo/py-bar no longer throw.
  - test/parser/parsers/margin_parser_test.dart — m-primary/mx-foo/my-bar no longer throw.
  - test/parser/parsers/position_parser_test.dart — top-abc/left-foo/inset-x-bar no longer throw.
  - test/parser/parsers/flexbox_grid_parser_test.dart — gap-blue/gap-x-primary/gap-y-foo no longer throw.
• ./tool/coverage.sh 90 — new tryGetSpacing is fully exercised and each patched parser gains a dedicated test, so coverage should hold at ≥ 90 %. Please confirm in CI.

Post-change sync

This is a pure defect fix; no new widget/parser/token/theme field is added. doc/, example/lib/pages/, skills/wind-ui/, and README.md are unchanged per the guidance. CHANGELOG updated under [Unreleased] > Fixed.

Notes

I could not run flutter test/dart analyze/dart format locally (no Flutter toolchain on the audit machine). Please rely on CI. All edits were made against HEAD aa07198 and follow the parser conventions in .github/instructions/parsers.instructions.md.

Summary by CodeRabbit

• Bug Fixes

  • Unknown theme spacing tokens in margin, padding, position, and gap utilities are now safely ignored instead of causing errors.
  • Valid spacing tokens continue to resolve as before, including numeric values and full.

• Tests

  • Added coverage for unknown spacing tokens across supported utility parsers and theme spacing lookups.

• Documentation

  • Updated the changelog with the corrected behavior.

[coderabbitai]

Warning

Review limit reached

@anilcancakir, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 29 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 96135d1a-a0ca-41b8-a1a6-c8a54dd485ef

📥 Commits

Reviewing files that changed from the base of the PR and between 433a222 and 0c8f511.

📒 Files selected for processing (1)

• lib/src/theme/wind_theme_data.dart

📝 Walkthrough

Walkthrough

Unknown theme-spacing tokens now resolve to null through tryGetSpacing and are dropped by padding, margin, position, and flex-gap parsers. Tests cover valid and invalid spacing lookups plus parser behavior, and the changelog documents the fix.

Changes

Theme spacing token handling

Layer / File(s)	Summary
Nullable spacing lookup lib/src/theme/wind_theme_data.dart, test/theme/wind_theme_data_test.dart	Adds tryGetSpacing, preserving valid spacing behavior while returning null for unknown tokens, with corresponding tests.
Parser handling and regression coverage lib/src/parser/parsers/*_parser.dart, test/parser/parsers/*_parser_test.dart, CHANGELOG.md	Updates padding, margin, position, and flex-gap parsing to drop unknown spacing tokens without throwing, with regression tests and changelog documentation.

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

Suggested reviewers: anilcancakir

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: unknown spacing tokens are tolerated instead of throwing.
Linked Issues check	✅ Passed	The PR adds tryGetSpacing and updates padding, margin, position, and flex-gap parsers to drop unknown tokens, matching #155.
Out of Scope Changes check	✅ Passed	The changes stay focused on the spacing-token crash fix, associated tests, and changelog update.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[anilcancakir]

Thanks for the thorough writeup, @thejesh23. The diagnosis is exactly right: getSpacing throwing inside build() violates the "unknown className is dropped with a debug warning" contract that wind_parser.dart and the sibling sizing_parser already honor, and a color-name typo like p-primary producing a red screen is a real trap.

I verified the fix end to end on a maintainer machine (the CI-equivalent Definition of Done gate you could not run locally):

• dart analyze (lib + test): no issues.
• dart format --set-exit-if-changed: 0 changed.
• flutter test: 1671 pass, 1 pre-existing skip.
• ./tool/coverage.sh 90: 94.6%, above the 90% floor.

A few things I checked beyond the four listed callers:

• Completeness. The other two getSpacing callers, sizing_parser.dart and text_parser.dart (line-height branch), both pre-validate with double.tryParse before calling, so they were already safe. No crash site is left behind.
• No silent regression. tryGetSpacing mirrors getSpacing byte for byte except for the throw, and the fraction case (p-1/2) was already going through the double.tryParse -> null -> throw path before this PR, so nothing that used to resolve now silently drops.
• Regression tests are genuine. Before the fix parser.parse(...) threw, so each new returnsNormally assertion fails on the old code and passes on the new one.

I pushed one follow-up commit on top (refactor(theme): fold getSpacing into tryGetSpacing): getSpacing now delegates to tryGetSpacing and throws only when it returns null, instead of keeping a parallel copy of the full/container/numeric branches. Behavior is identical and backward compatibility holds, it just removes the drift risk between the two lookups. I also dropped an em-dash from the tryGetSpacing docstring to match the repo's punctuation convention. Both stay within your intent of keeping getSpacing's observable behavior unchanged.

Approving. Thank you for the clean, well-tested contribution.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

This PR fixes a runtime crash where spacing-related utility parsers could throw ArgumentError during build() when given unknown (often typo) tokens like p-primary or gap-x-blue. It introduces a non-throwing spacing lookup on WindThemeData and updates the affected parsers to drop unknown spacing tokens instead of crashing, aligning behavior with Wind’s “unknown tokens are ignored” contract.

Changes:

• Added WindThemeData.tryGetSpacing(String) (returns null for unrecognized tokens) and refactored getSpacing to delegate to it while preserving existing throwing behavior.
• Updated padding, margin, position, and flex-gap parsing to use tryGetSpacing so unknown spacing tokens no-op instead of throwing.
• Added targeted regression tests ensuring unknown spacing tokens do not throw, and documented the fix in CHANGELOG.md.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
lib/src/theme/wind_theme_data.dart	Adds tryGetSpacing and refactors getSpacing to share logic while keeping getSpacing’s throwing contract.
lib/src/parser/parsers/padding_parser.dart	Switches theme spacing resolution to tryGetSpacing to safely ignore unknown padding tokens.
lib/src/parser/parsers/margin_parser.dart	Switches theme spacing resolution to tryGetSpacing to safely ignore unknown margin tokens.
lib/src/parser/parsers/position_parser.dart	Switches theme spacing resolution to tryGetSpacing to safely ignore unknown offset tokens.
lib/src/parser/parsers/flexbox_grid_parser.dart	Switches gap theme spacing resolution to tryGetSpacing to safely ignore unknown gap tokens.
test/theme/wind_theme_data_test.dart	Adds tests asserting tryGetSpacing matches getSpacing for valid inputs and returns null for unknown tokens.
test/parser/parsers/padding_parser_test.dart	Adds regression test ensuring unknown padding tokens don’t throw and don’t apply padding.
test/parser/parsers/margin_parser_test.dart	Adds regression test ensuring unknown margin tokens don’t throw and don’t apply margin.
test/parser/parsers/position_parser_test.dart	Adds regression test ensuring unknown position tokens don’t throw and don’t apply offsets.
test/parser/parsers/flexbox_grid_parser_test.dart	Adds regression test ensuring unknown gap tokens don’t throw and don’t apply gaps.
CHANGELOG.md	Documents the behavior change under [Unreleased] > Fixed.