Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix formatting of control characters \x1a through \x1f #201

Merged

Conversation

joshtriplett
Copy link
Contributor

In the course of working on rust-lang/rust#134915 , I
discovered what appears to be a bug in BStr's Debug impl.

Due to an incorrect range, control characters \x1a through \x1f get
formatted as Unicode \u escapes rather than hex \x escapes.

Fix the range, and add a test.

Without the fix, the test fails like this:

  left: "\"\\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07\\x08\\t\\n\\x11\\x12\\r\\x14\\x15\\x16\\x17\\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f \\x7f\\x80\\x81\\xfe\\xff\""
 right: "\"\\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07\\x08\\t\\n\\x11\\x12\\r\\x14\\x15\\x16\\x17\\x18\\x19\\u{1a}\\u{1b}\\u{1c}\\u{1d}\\u{1e}\\u{1f} \\x7f\\x80\\x81\\xfe\\xff\""

In addition to the straightforward fix, simplify the logic using escape_ascii
from the standard library. (I've kept this as a separate commit in case you
prefer to just merge the first commit.)

Due to an incorrect range, control characters \x1a through \x1f get
formatted as Unicode \u escapes rather than hex \x escapes.

Fix the range, and add a test.

Without the fix, the test fails like this:

```
  left: "\"\\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07\\x08\\t\\n\\x11\\x12\\r\\x14\\x15\\x16\\x17\\x18\\x19\\x1a\\x1b\\x1c\\x1d\\x1e\\x1f \\x7f\\x80\\x81\\xfe\\xff\""
 right: "\"\\0\\x01\\x02\\x03\\x04\\x05\\x06\\x07\\x08\\t\\n\\x11\\x12\\r\\x14\\x15\\x16\\x17\\x18\\x19\\u{1a}\\u{1b}\\u{1c}\\u{1d}\\u{1e}\\u{1f} \\x7f\\x80\\x81\\xfe\\xff\""
```
Copy link
Owner

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

Nice, thanks! This LGTM with a rustfmt. :-)

Replace manual ranges of characters to escape with a call to
`escape_ascii` for anything in the ASCII range.
@joshtriplett joshtriplett force-pushed the fix-control-character-range branch from 93da841 to bbeda2b Compare January 2, 2025 13:47
@joshtriplett
Copy link
Contributor Author

@BurntSushi Done.

@BurntSushi BurntSushi merged commit 732fc99 into BurntSushi:master Jan 2, 2025
8 checks passed
@BurntSushi
Copy link
Owner

Thanks! This PR is on crates.io in bstr 1.11.2.

@joshtriplett joshtriplett deleted the fix-control-character-range branch January 2, 2025 14:36
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.

2 participants