8000 fix: include payload in Error Display impl (#426) by chiliec · Pull Request #428 · emoon/rust_minifb · GitHub
[go: up one dir, main page]
Skip to content

fix: include payload in Error Display impl (#426) - #428

Merged
emoon merged 1 commit into
emoon:masterfrom
chiliec:fix/display-includes-payload
Aug 18, 2026
Merged

fix: include payload in Error Display impl (#426)#428
emoon merged 1 commit into
emoon:masterfrom
chiliec:fix/display-includes-payload

Conversation

@chiliec
@chiliec chiliec commented Aug 18, 2026
Copy link
Copy Markdown
Contributor

What

Closes #426.

The Display impl for Error printed only the variant name and discarded the String payload:

Error::MenuExists(_)   => write!(formatter, "Menu already exists"),
Error::WindowCreate(_) => write!(formatter, "Failed to create window"),
Error::UpdateFailed(_) => write!(formatter, "Failed to Update"),

So a detailed message — for example the buffer-size explanation built in check_buffer_size (src/lib.rs) — never reached err.to_string(), println!("{}", err), or Box<dyn Error> reporting; it was only visible via {:?}.

Fix

Bind and include the payload in Display:

Error::MenuExists(ref e)   => write!(formatter, "Menu already exists: {e}"),
Error::WindowCreate(ref e) => write!(formatter, "Failed to create window: {e}"),
Error::UpdateFailed(ref e) => write!(formatter, "Failed to Update: {e}"),

Debug is left unchanged (it already includes the payload).

Tests

Added a unit test asserting to_string() includes the payload for each variant.

Verified RED→GREEN: with the old Display, the test fails (left: "Failed to Update", right: "Failed to Update: buffer too small"); with the fix it passes.

Validation (real results)

  • cargo test --lib display_includes_payload: passes.
  • cargo fmt --check: clean.

@chiliec
fix: include payload in Error Display impl
9b1170f 
The Display impl for Error printed only the variant name and dropped the
String payload for MenuExists, WindowCreate and UpdateFailed. Detailed
messages (e.g. the buffer-size explanation from check_buffer_size) were
therefore lost via to_string(), `{}`, and Box<dyn Error> reporting, and
only visible through {:?}.

Include the payload in Display so the detail reaches error reporting.

Closes emoon#426
@emoon
emoon merged commit 71ace73 into emoon:master Aug 18, 2026
4 checks passed
@emoon
emoon commented Aug 18, 2026
Copy link
Copy Markdown
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Display impl for Error discards the detail message

2 participants

@chiliec @emoon
0