Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Fix missing fmt argument in error message #2118

Merged
merged 3 commits into from
Nov 3, 2021

Conversation

dtolnay
Copy link
Contributor

@dtolnay dtolnay commented Nov 3, 2021

  • The first commit fills in a missing argument to {} in config.rs. Without this, the error message would literally be 'Your token has status "{}"' with curly braces in it instead of the token status.

  • The second commit closes an inconsistency between how two places were doing bail macro args: the one in delete.rs used single { assuming the string would not get put into std::fmt, while the one in put.rs used double {{ assuming that std::fmt would get applied to it. This change makes room for the bail macro to catch issues like this statically in a future version.

  • The third commit is done by rustfmt; I am guessing the long line changed by the second commit previously made rustfmt fail out without formatting anything in the whole match expression.

Without this, the error message would literally be 'Your token has
status "{}"' with curly braces in it instead of the token status.
This code was doing two inconsistent things with the bail macro args:
the one in delete.rs used single `{` assuming the string would *not* get
put into std::fmt, while the one in put.rs used double `{{` assuming
that std::fmt *would* get applied to it.

This change closes the inconsistency and makes room for the bail macro
to catch issues like this statically in a future version.
@dtolnay dtolnay requested a review from a team as a code owner November 3, 2021 15:31
Copy link
Contributor

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Thanks for the fixes! Excited to hear that you're thinking of preventing this statically :)

@jyn514 jyn514 merged commit db14f23 into cloudflare:master Nov 3, 2021
@nataliescottdavidson nataliescottdavidson mentioned this pull request Nov 9, 2021
@dtolnay dtolnay deleted the fmt branch November 21, 2021 01:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants