Skip to content

fix: make Style non-exhaustive - #270

Open
nightcityblade wants to merge 1 commit into
vyncint:mainfrom
nightcityblade:fix/issue-259
Open

fix: make Style non-exhaustive#270
nightcityblade wants to merge 1 commit into
vyncint:mainfrom
nightcityblade:fix/issue-259

Conversation

@nightcityblade

Copy link
Copy Markdown

What & why

Mark Style as non-exhaustive while the crate is pre-1.0, so future terminal attributes can be represented without another breaking change. The API docs and changelog explain how downstream comparisons should construct a style from Style::default().

A dedicated builder is intentionally omitted: default plus field assignment remains concise and avoids adding API surface solely for test expectations.

Closes #259.

Checklist

  • Linked an issue (or explained above why none exists)
  • Tests added/updated for the change (existing style integration suite covers field access and comparisons)
  • cargo fmt --all and cargo clippy --workspace --all-targets --all-features are clean
  • All commits are signed off (git commit -s) — see CONTRIBUTING.md §5
  • No AI attribution trailers (no AI co-authors, "Generated with" footers, or bot identities) — see CONTRIBUTING.md §6
  • CHANGELOG.md updated under [Unreleased] (user-facing changes only)
  • Snapshot changes (if any) were reviewed with cargo insta review, not blind-accepted (N/A: no snapshots changed)

@vyncint vyncint left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Reviewed and verified. Correct, and the reasoning in the PR body is the right call.

What I checked beyond CI:

  • Built an external consumer crate against this branch. Style { … } is refused with E0639: cannot create non-exhaustive struct using struct expression, and so is the functional-update form Style { bold: true, ..Style::default() } — which is the one people reach for first and the reason the doc paragraph has to name the alternative explicitly. It does.
  • The documented migration compiles: let mut s = Style::default(); s.fg = …; s.bold = true;.
  • No integration test or fixture constructs a Style literal, so nothing outside the crate needed converting. The in-crate literals are unaffected by the attribute.

Declining the builder is the right answer and I am glad you said so out loud rather than leaving it unmentioned. Two lines of assignment is not a hardship, and a builder would be API surface that exists only for test expectations.

This is also the last cheap moment to make this change, which is what the issue was really about — after 1.0 it costs a major.


One thing to fix, and it is the same in all five of your PRs

The CHANGELOG entry landed inside the published ## [0.9.0] - 2026-09-05 section rather than under ## [Unreleased]. Easy to miss: [Unreleased] was empty when you branched, so the nearest heading with content under it belongs to 0.9.0.

It matters for two reasons. 0.9.0 is already on crates.io and its GitHub Release notes were generated from that section, so the entry now describes a release that does not contain the change. And docs/RELEASING.md cuts the next release by moving [Unreleased] down into a new version heading — an entry parked in [0.9.0] would never appear in any release notes at all.

Move it up under ## [Unreleased], adding the ### Added / ### Changed / ### Fixed sub-heading there if it does not exist yet. main has moved on by a commit since you branched, so a rebase is wanted anyway:

git fetch origin && git rebase origin/main

Thanks for these — five well-scoped fixes with tests and docs, and the PR bodies say what you decided and why. That is exactly the shape this repository asks for.

Signed-off-by: nightcityblade <nightcityblade@gmail.com>
@nightcityblade

Copy link
Copy Markdown
Author

Rebased onto current main and moved the Style entry under UnreleasedChanged in dcc23e9.

Validation: cargo fmt --check and cargo test --workspace --all-features pass.

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.

Style is the one public struct that is not #[non_exhaustive], and two attributes are already known to be missing

2 participants