Escape control chars in source when printing diagnostics #8049
Conversation
| let found_len = res[base + found_pos..].chars().next().unwrap().len_utf8(); | ||
| res.to_mut().replace_range( | ||
| base + found_pos..base + found_pos + found_len, | ||
| REPLACEMENT_CHAR, | ||
| ); | ||
| base += found_pos + REPLACEMENT_CHAR.len(); |
There was a problem hiding this comment.
nitpick(non-blocking): I think it'd be easier to understand this code (including base += …) with base + found_pos factored out to a variable. Not a big deal, though.
| @@ -137,3 +137,29 @@ where | |||
| Some(&self.inner) | |||
| } | |||
| } | |||
|
|
|||
| pub(crate) fn replace_control_chars(s: &str) -> Cow<'_, str> { | |||
| const REPLACEMENT_CHAR: &str = "\u{FFFD}"; | |||
There was a problem hiding this comment.
nitpick: We should use std::char::REPLACEMENT_CHARACTER instead of using this ad-hoc constant.
There was a problem hiding this comment.
I wanted to, but couldn't find a way to go from char to &str without a buffer.
There was a problem hiding this comment.
Oh, hmm. I suppose you could use a stack-allocated buffer and char::encode_utf8?
I'd be satisfied with merely a (doc) comment pointing to REPLACEMENT_CHARACTER expressing the type woe you're working around, too.
There was a problem hiding this comment.
suggestion: Let's add a CHANGELOG entry. This isn't likely to be noticeable for most folks' shaders, but it's good to call out.
Replace control characters when emitting diagnostics in naga.
The motivation here is that my fuzzing runs were making my terminals unhappy. In most cases, in a browser, they won't go to a terminal so it won't matter, but this seems like the right thing to do regardless.
Testing
Adds a unit test and a
wgsl_errorstest.Squash or Rebase? Squash
Checklist
cargo fmt.taplo format.cargo clippy --tests. If applicable, add:--target wasm32-unknown-unknowncargo xtask testto run tests.CHANGELOG.mdentry.