-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Copy more doc comments to JS/TS files, unescape comments #2070
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! I've just got a question about escaping but otherwise looks good to me
crates/cli-support/src/wit/mod.rs
Outdated
.collect::<Vec<_>>() | ||
.join("\n") | ||
} | ||
|
||
// Unescapes a quoted string. char::escape_debug() was used to escape the text. | ||
fn try_unescape(s: &str) -> Option<String> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm personally a bit confused on why we need a method like this, can you help me understand where this is coming from? Where do we acquire an escaped string from, and would it be possible to acquire an unescaped string instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasm-bindgen/crates/macro-support/src/parser.rs
Lines 1320 to 1324 in b9f78ab
TokenTree::Literal(lit) => { | |
// this will always return the quoted string, we deal with | |
// that in the cli when we read in the comments | |
Some(lit.to_string()) | |
} |
That's the code that gets a quoted + escaped string from proc_macro2
. I couldn't find an unescape function anywhere. If you know of one, I'll replace this func with that one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the un-quoting/etc happen around there? I don't think it should be happening in the CLI but rather at the source here if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been moved now. The old code used to unquote the string, but it's also been moved. Let me know if that's considered a breaking change and I'll restore it.
Thanks! |
Closes #2053
Closes #2052
Unescapes comments. Looks like it's coming from proc-macro2, it returns a quoted + escaped string. I couldn't find an unescape function so I had to write it myself.
Copies more comments from rust to js/ts files. I think all comments are copied now.