Skip to content

feat(config): add CSS custom-properties scraper - #114

Merged
aram-devdocs merged 3 commits into
mainfrom
codex/30-feat-config-css-custom-props-scraper
Apr 25, 2026
Merged

feat(config): add CSS custom-properties scraper#114
aram-devdocs merged 3 commits into
mainfrom
codex/30-feat-config-css-custom-props-scraper

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Spec

Fixes #30

Summary

  • Adds scrape_css_properties (in plumb-config) that walks a list of CSS files, finds :root { ... } blocks at the top level or wrapped in a single @media / @supports at-rule, and surfaces every --name: value; declaration as a typed CssPropertyScrape.
  • Values are classified into Color (hex/rgb/rgba/hsl/hsla normalized to #rrggbb / #rrggbbaa), Px(u32), Rem(f32), Em(f32), or Other(String) so callers (e.g. plumb init) can fold them into [spacing]/[color]/[type]/[radius].
  • Malformed CSS surfaces as ConfigError::CssParse with a miette span pointing at the offending region.

Hand-rolled scanner (brace + semicolon state machine, skips comments + quoted strings) — chosen over cssparser to keep the dep tree narrow given the narrow scope (:root discovery only).

Test plan

  • just check passes (fmt + clippy, no warnings)
  • just test passes on my machine
  • just determinism-check passes
  • cargo deny check passes
  • New tests added — crates/plumb-config/tests/css_props_scraper.rs (13 cases: top-level :root, @media-wrapped :root, @supports-wrapped :root, multiple :root blocks, comments + quoted strings (semicolon inside string), rem surfaces as Rem, em surfaces as Em warning, Other for font stacks / unitless, rgb() → hex normalization, malformed CSS → CssParse with span, missing file → Read, ignores non-:root selectors, ignores non-custom declarations).
  • Docs updated — N/A (internal helper; no user-facing prose yet, surfaces via future plumb init).
  • Humanizer skill run on any docs/src/** prose — N/A (no docs/src/** changes).

Screenshots / terminal output

N/A — no user-visible output yet. The function returns Vec<CssPropertyScrape> for downstream consumers.

Breaking change?

  • No
  • Yes — describe the migration path

Anything reviewers should double-check

  • The :root selector match is strict (:root only, not :root, html). Issue spec is explicit about scope; expanding to selector lists is a follow-up.
  • One level of at-rule wrapping is supported (@media (...)/@supports (...)); deeper nesting is tolerantly skipped rather than erroring, matching the spec's "one level deep is enough".
  • HSL → RGB conversion uses standard chroma/intermediate math; tested via hsl(0, 100%, 50%) == #ff0000.

🤖 Generated with Claude Code

Adds `scrape_css_properties` that walks a list of CSS files, finds
`:root { ... }` blocks (top-level or wrapped in a single `@media` /
`@supports` at-rule), and surfaces every `--name: value;` declaration as
a typed `CssPropertyScrape`. Values are classified into hex/rgb/hsl
colors (normalized to `#rrggbb`/`#rrggbbaa`), `Px(u32)`, `Rem(f32)`,
`Em(f32)`, or `Other(String)` so callers like `plumb init` can fold them
into the matching config sections.

Hand-rolled scanner (skip comments, skip strings, brace depth) — chosen
over `cssparser` to keep the dep tree narrow given the limited scope.
Malformed input surfaces as `ConfigError::CssParse` with a miette span.

Closes #30

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

I now have everything I need. The diff is entirely in plumb-config (4 files: css_props.rs, lib.rs hunk, one integration test file, one fixture). Let me write up the review.


PR #114plumb-config: CSS custom-properties scraper

Files changed: crates/plumb-config/src/css_props.rs (new, 893 loc), lib.rs (18 lines), tests/css_props_scraper.rs (246 loc), tests/fixtures/tokens.css (32 loc).


1. Determinism

No violations found.

  • No HashMap/HashSet in output paths — results are Vec<CssPropertyScrape>, emitted in input order (file-then-source-position).
  • No SystemTime::now, Instant::now, rand, or env reads inside the parser.
  • f32 appears in ScrapedValue::Rem(f32) and ScrapedValue::Em(f32), but only as stored classification data, never as a sort key or stable output field.
  • The public doc at css_props.rs:80–82 correctly transfers the sort responsibility to callers ("Callers that want a stable global ordering across runs MUST sort files before calling"), which is the right contract for a config crate.

2. Workspace layering

Clean. plumb-config depends on plumb-core (correct per the hierarchy); the new module adds no new Cargo.toml dependencies. #![forbid(unsafe_code)] is on the crate root and nothing in css_props.rs uses unsafe.

3. Error handling

No unwrap()/expect() in library code — the crate root has #![deny(clippy::unwrap_used, clippy::expect_used)] and both .unwrap_or(...) calls in the new module are default-value forms, not the banned direct form. ConfigError::CssParse is a well-formed thiserror/miette variant with a proper path, message, source_code, and #[label] span.

4. Test coverage

Excellent. 11 integration tests cover: top-level :root, :root-in-@media, :root-in-@supports, multiple :root blocks, comments and quoted strings, rem/em/Other variants, RGB-to-hex normalization, malformed CSS error shape, missing-file error shape, non-root selector filtering, non-custom property filtering. Two unit tests cover classify_value and strip_inline_comments. tempfile is already a dev-dep.

5. Documentation

All three re-exported public items (CssPropertyScrape, ScrapedValue, scrape_css_properties) and every public field/variant carry doc comments. scrape_css_properties has a # Errors section. ConfigError::CssParse and its fields are documented. #![deny(missing_docs)] is enforced crate-wide.


Punch list

css_props.rs:27#![allow(clippy::redundant_pub_crate)] is misidentified in the adjacent comment. The items in this module are pub (not pub(crate)) and are re-exported via pub use in lib.rs, so pub is the correct scope. The redundant_pub_crate lint fires on pub(crate) in private modules, not on pub; there are no pub(crate) items in the file. The allow is either dead or guarding against a future accidental addition of pub(crate). The comment ("pub(crate) is the right scope but pedantic flags the redundancy") is backwards — pub is the right scope here. Low severity; harmless but misleading.

css_props.rs:776Parser::slice returns unwrap_or("") on an invalid UTF-8 slice. The accompanying comment explains the invariant correctly (cursor positions land only on ASCII delimiters, so byte boundaries are sound). That said, a debug_assert!(std::str::from_utf8(&self.bytes[start..end]).is_ok()) above the call would catch the invariant break cheaply in debug builds instead of silently producing an empty string that callers would then miscategorize as an empty declaration. Low severity.

Neither finding is a blocker.


Verdict: APPROVE

@aram-devdocs aram-devdocs changed the title feat(config): CSS custom-properties scraper feat(config): add CSS custom-properties scraper Apr 25, 2026
…ents

The previous loop pushed `bytes[i] as char` for every non-comment byte,
which truncates non-ASCII codepoints. Replace with a string-slice copy
of each non-comment run so multi-byte values (e.g. unicode font names
in `--font: "…"`) round-trip correctly.

Addresses Claude code-review BLOCKER on #114.
Address claude-code-review feedback on PR #114:

- Rename misleading `lower` binding in `strip_func` to `bytes`; the
  binding was never lowercased and bytes were re-computed on the
  next line.
- Reconcile `Parser::slice` comment with code. The empty-string
  fallback is the actual defensive no-op for an unreachable branch
  (cursor positions only land on ASCII delimiters by construction);
  drop the misleading `from_utf8_lossy` suggestion.
- Document output ordering on `scrape_css_properties`: results are
  emitted in input-file order, then by source position. Callers using
  `glob`/`read_dir` MUST sort first to keep output deterministic
  across runs.
@aram-devdocs
aram-devdocs merged commit b6dbde1 into main Apr 25, 2026
23 of 24 checks passed
aram-devdocs added a commit that referenced this pull request Apr 25, 2026
Brings in #112 (color/palette-conformance rule) and #114 (CSS
custom-properties scraper). Resolves a trivial conflict in
plumb-config/src/lib.rs (both branches added a new mod + pub use;
combine them) and corrects the CHANGELOG entry to reflect the
lowered MAX_NESTING (256 -> 64) from the prior review-feedback
commit.
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.

feat(config): CSS custom-properties scraper

1 participant