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

Allow # to appear in rustdoc code output. #41785

Merged
merged 1 commit into from
May 7, 2017

Conversation

Mark-Simulacrum
Copy link
Member

@Mark-Simulacrum Mark-Simulacrum commented May 6, 2017

"##" at the start of a trimmed rustdoc line is now cut to "#" and then
shown. If the user wanted to show "##", they can type "###".

I'm somewhat concerned about the potential implications for users, since this does make a potentially backwards-incompatible change. Previously, ## had no special handling, and now we do change it. However, I'm not really sure what we can do here to improve this, and I can't think of any cases where ## would likely be correct in a code block, though of course I could be wrong.

Fixes #41783.

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)


// @has issue_41783/struct.Foo.html
// @has - '# <span class="ident">single'
// @has - '#<span class="attribute"># <span class="ident">double</span>'
Copy link
Member Author

Choose a reason for hiding this comment

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

If there's a better way to do this, I'd be happy to hear about it.

@GuillaumeGomez
Copy link
Member

Ok, so it seems to be a good idea but I don't agree with how you implemented it or the idea of the feature. From my point of view, if a line starts with #, then you have to check the next character. If it's not #, then you don't print the line. As simple as this. Your way of doing it seems a bit overkill.

@Mark-Simulacrum Mark-Simulacrum force-pushed the issue-41783 branch 3 times, most recently from 74b29b6 to ba840c5 Compare May 6, 2017 21:05
} else if trimmed.starts_with("# ") {
Some(&trimmed[2..])
Line::Hidden("")
} else if trimmed.starts_with("#") {
Copy link
Member

Choose a reason for hiding this comment

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

This code won't work, switch the two conditions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Err, that was a mistake either way, it should be "# ".

let trimmed = s.trim();
if trimmed == "#" {
Some("")
Line::Hidden("")
} else if trimmed.starts_with("# ") {
Copy link
Member

Choose a reason for hiding this comment

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

Something more intelligent in here could be to put this if condition after the next one, and replace starts_with("# ") with starts_with("#"). So you can write:

#extern crate foo;

As well as:

# extern crate foo;

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label May 7, 2017
"##" at the start of a trimmed rustdoc line is now cut to "#" and then
shown. If the user wanted to show "##", they can type "###".
@Mark-Simulacrum
Copy link
Member Author

r? @GuillaumeGomez

I wasn't able to make #text work since that conflicts with #[attr]. However, I think I did the other things you requested.

@GuillaumeGomez
Copy link
Member

Well, it could be a future improvement. We'll see. For now it's good, thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented May 7, 2017

📌 Commit ffe12b1 has been approved by GuillaumeGomez

@bors
Copy link
Contributor

bors commented May 7, 2017

⌛ Testing commit ffe12b1 with merge ced823e...

bors added a commit that referenced this pull request May 7, 2017
Allow # to appear in rustdoc code output.

"##" at the start of a trimmed rustdoc line is now cut to "#" and then
shown. If the user wanted to show "##", they can type "###".

I'm somewhat concerned about the potential implications for users, since this does make a potentially backwards-incompatible change. Previously, `##` had no special handling, and now we do change it. However, I'm not really sure what we can do here to improve this, and I can't think of any cases where `##` would likely be correct in a code block, though of course I could be wrong.

Fixes #41783.
@bors
Copy link
Contributor

bors commented May 7, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: GuillaumeGomez
Pushing ced823e to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants