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

Don't highlight # which does not start an attribute in rustdoc #43918

Merged
merged 2 commits into from
Aug 30, 2017

Conversation

mystor
Copy link
Contributor

@mystor mystor commented Aug 17, 2017

Currently when we highlight some macros for rustdoc (e.g. quote! from https://github.com/dtolnay/quote), we get really bad syntax highlighting, because we assume that every token between a # character and the next ] in the source must be an attribute.

This patch improves that highlighting behavior to instead only highlight after finding the [ token after the # token.

(NOTE: I've only run this patch against https://github.com/nrc/rustdoc-highlight so if it doesn't build on travis that's why - I don't have a recent rustc build on this laptop)

I'm guessing r? @steveklabnik

@mystor
Copy link
Contributor Author

mystor commented Aug 17, 2017

See the docs for synstructure for how bad the current highlighting can look (https://docs.rs/synstructure/0.5.2/synstructure/)

@QuietMisdreavus
Copy link
Member

Looks like the travis failure was in running this test, which was put in by #41783 and #41785. The test expects the behavior you're trying to change, so it may be prudent to change the test? I'm not too sure on the particulars.

r? @GuillaumeGomez

@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Aug 17, 2017

I would actually be a bit more strict on the check: "if the character after # isn't a whitespace, then it shouldn't be highlighted". What do you think of this?

@arielb1 arielb1 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 22, 2017
@carols10cents
Copy link
Member

Hi @mystor! friendly ping to keep this on your radar! Wdyt of @GuillaumeGomez's suggestion?

@mystor
Copy link
Contributor Author

mystor commented Aug 28, 2017

Sorry! I have been very busy over the last week-or-so due to travel for RustConf and then the subsequent catching up on work, so this patch slipped my mind.

I don't think I fully understand what @GuillaumeGomez is suggesting. From what I can tell, the test referenced by @QuietMisdreavus was testing hiding a line from rustdoc output with the # character.

Namely, currently the following happens:

# This will be hidden
# # This will also be hidden (note:  the space)
## This escapes the `#` at the beginnig of the line and has one # character
### This only escapes the first `#` character.
#### Again, the same.

The reason why this change affects this test was because when we read a non-escaped # character in the code highlighter we began an attribute, whether or not it was syntactically the start of an attribute. You can actually see that in the expected test output. That output was expected to produce partial HTML output, as it never parses a ] character to terminate the attribute.

With this change, none of these lines would be indented, as none of them form a valid attribute. A line like:

##[This is an attribute]

would be parsed as an attribute, and would have a complete attribute span tag around it.

@QuietMisdreavus
Copy link
Member

@GuillaumeGomez Wouldn't this just take out the attribute highlighting altogether? Or are you wanting to highlight # characters by themselves? I like the way this PR does it, since it specifically checks that something looks like an attribute before highlighting it as such. Otherwise it just doesn't do anything special.

@GuillaumeGomez
Copy link
Member

No, it's fine as is.

@QuietMisdreavus
Copy link
Member

Cool! This looks good, thanks for the PR!

@bors r+

@bors
Copy link
Contributor

bors commented Aug 29, 2017

📌 Commit 2f19383 has been approved by QuietMisdreavus

@arielb1 arielb1 added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 29, 2017
arielb1 pushed a commit to arielb1/rust that referenced this pull request Aug 29, 2017
…avus

Don't highlight # which does not start an attribute in rustdoc

Currently when we highlight some macros for rustdoc (e.g. `quote!` from https://github.com/dtolnay/quote), we get really bad syntax highlighting, because we assume that every token between a `#` character and the next `]` in the source must be an attribute.

This patch improves that highlighting behavior to instead only highlight after finding the `[` token after the `#` token.

(NOTE: I've only run this patch against https://github.com/nrc/rustdoc-highlight so if it doesn't build on travis that's why - I don't have a recent rustc build on this laptop)

I'm guessing r? @steveklabnik
arielb1 pushed a commit to arielb1/rust that referenced this pull request Aug 29, 2017
…avus

Don't highlight # which does not start an attribute in rustdoc

Currently when we highlight some macros for rustdoc (e.g. `quote!` from https://github.com/dtolnay/quote), we get really bad syntax highlighting, because we assume that every token between a `#` character and the next `]` in the source must be an attribute.

This patch improves that highlighting behavior to instead only highlight after finding the `[` token after the `#` token.

(NOTE: I've only run this patch against https://github.com/nrc/rustdoc-highlight so if it doesn't build on travis that's why - I don't have a recent rustc build on this laptop)

I'm guessing r? @steveklabnik
bors added a commit that referenced this pull request Aug 29, 2017
Rollup of 12 pull requests

- Successful merges: #43705, #43778, #43918, #44076, #44117, #44121, #44126, #44134, #44135, #44141, #44144, #44158
- Failed merges:
@bors bors merged commit 2f19383 into rust-lang:master Aug 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants