-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
rustdoc: Hide #text
in doc-tests
#84445
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
Not really, for example: let s = "hello
#because why not?"; |
@GuillaumeGomez that's already broken today if you use let s = "hello
# because why not?"; |
Really the problem is that rustdoc chose an ambiguous syntax, it needs to both use something that's both an invalid token normally and also follow string open-closing marks. The string thing might be possible but it will be a real pain. I don't this should be blocked on that improvement given that's there's other valid code that's already broken by the |
This comment has been minimized.
This comment has been minimized.
Since `#![attr]` and `#[attr]` are the only valid syntax that start with `#`, we can just special case those two tokens.
Can't we use the ast parser to get some help there? |
@GuillaumeGomez I think this may be possible, but it will be a lot of hard work for very little benefit. If you're worried about breakage, maybe it makes sense to do a crater run instead? I don't think the exact whitespace you can use with |
@bors try (in case you want a crater run) |
No, my problem is more like that this isn't a real fix, more like a patchwork over a few others. I don't mind merging as is. But maybe in the long run, we should try to take a long at this to have a "definitive" fix. |
Ok, I opened #84478 for the proper fix. I still think this is a good change in the meantime that makes the behavior less surprising. |
Fine by me, thanks for opening the issue and thanks for this PR! @bors: r+ rollup |
📌 Commit af6c320 has been approved by |
☀️ Test successful - checks-actions |
This change is not backwards compatible and has broken Rocket's test suite. Please revert. Rocket has: // This (long, contrived) form string...
# assert_form_parses! { Foo,
"[k:top_key][i][k:sub_key]name=Bobert&\
[k:top_key][i][k:sub_key]age=22&\
[k:top_key][i][sub_key]=1337&\
[top_key][7]name=Builder&\
[top_key][7]age=99",
// We could also set the top-level value's key explicitly:
// [top_key][k:7]=7
# "[k:top_key][i][k:sub_key]name=Bobert&\
# [k:top_key][i][k:sub_key]age=22&\
# [top_key][k:7]=7&\
# [k:top_key][i][sub_key]=1337&\
# [top_key][7]name=Builder&\
# [top_key][7]age=99",
# => Previously, the |
What bors did here was.. interesting. The cc @rust-lang/infra |
feels like rust-lang/homu#47 😢 |
…Gomez Revert "rustdoc: Hide `#text` in doc-tests" See discussion in rust-lang#84502 - I'm worried that rust-lang#84445 may cause a lot of breakages if this were to hit stable, so I think it's safer to revert and work on the known correct fix rust-lang#84478.
Since
#![attr]
and#[attr]
are the only valid syntax that start with#
, we can just special case those two tokens.Fixes #83284.