Use rustc_lexer for rustdoc syntax highlighting by matklad · Pull Request #75775 · rust-lang/rust

matklad · 2020-08-21T16:06:27Z

r? @ghost

matklad · 2020-08-21T16:07:06Z

This is not ready for review yet, but

r? @GuillaumeGomez

just to give a heads up

GuillaumeGomez · 2020-08-21T16:11:19Z

It looks like a nice start after a quick read. Don't forget to put back tests. :p

matklad · 2020-08-21T16:12:48Z

This bit is now gone, and, for example, && will always produce two spans now. I wonder if that is important? We need a real parser to do this precisely, but an approximate gluing heuristic won't be hard to add, if we need that.

For now it's fine, but please open an issue about it so it's not forgotten.

Hmm, I wonder if this would fix rust-lang/docs.rs#484.

Good question, that'd be nice!

@matklad could you add a test/screenshot of how this highlights assert!(self.length < N && index <= self.length);?

Added the test. There are two spans for <= and one span for && but they all are ops, so the end result looks fine.
On a meta note, I would be surprised if this doesn't regress some corner cases, but I think it should be fine to fix them as they come up, until we complete #75981.

Looks great, thanks!
On a meta note, I would be surprised if this doesn't regress some corner cases, but I think it should be fine to fix them as they come up, until we complete #75981.
I agree, and I don't expect this to be any more broken than the existing code.

matklad · 2020-08-21T16:15:15Z

@GuillaumeGomez that particular test is deliberately removed, see the commit message in b48b66c.

I do plan to add more unit-tests here, using #75773

matklad · 2020-08-27T11:00:50Z

This is ready for review @GuillaumeGomez !

matklad · 2020-08-27T11:37:22Z

Moved the tests, will open the issue about more precise highlighting shortly

GuillaumeGomez · 2020-08-27T11:42:48Z

Looks good to me now! However, considering this is a big PR, I'd like to have another reviewer from @rust-lang/rustdoc to check if I didn't miss anything. :)

jyn514

Can you post a screenshot of what the sample file looks like when highlighted?

matklad · 2020-08-27T12:57:10Z

@jyn514 thanks for the review, comments addressed (unless I've missed something)!

Filed #75981 for further improvements. FYI, here's the html bit of rust-analyzer's syntax highlighting. Here the bulk of actual highlighting.

Screenshot it #75775 (comment)

jyn514 · 2020-08-27T13:01:13Z

Looks great, thanks!

ollie27 · 2020-08-27T13:41:44Z

 }
+
+#[test]
+fn test_html_highlighting() {


It doesn't make sense to put this test here. Syntax highlighting doesn't have anything to do with running doctests which is what this file is a submodule of. I suggest keeping these tests in src/librustdoc/html/highlight/tests.rs.

I disagree with you for the location, however I think those tests should be put into a file on their own (like test/highlight.rs).

rustc_lexer is the lossless lexer, which is a better fit for approximate syntax highlighting. As a side-effect, we can now syntax-highlight even broken code.

matklad · 2020-08-27T16:37:52Z

Move tests back to where they originally were, and submitted #75989 to not be confused with this next time.

It's a unit-test in a sense that it only checks syntax highlighting. However, the resulting HTML is written to disk and can be easily inspected in the browser. To update the test, run with `--bless` argument or set `UPDATE_EXPEC=1` env var

See rust-lang#75773 and rust-lang#75775

GuillaumeGomez · 2020-08-29T11:58:20Z

Thanks!

@bors: r+

bors · 2020-08-29T11:58:21Z

📌 Commit b4f4db9 has been approved by GuillaumeGomez

bors · 2020-08-29T22:42:14Z

⌛ Testing commit b4f4db9 with merge ced37a5...

bors · 2020-08-30T00:47:30Z

☀️ Test successful - checks-actions, checks-azure
Approved by: GuillaumeGomez
Pushing ced37a5 to master...

rust-highfive assigned GuillaumeGomez Aug 21, 2020

matklad commented Aug 21, 2020
View reviewed changes

Comment threadsrc/librustdoc/html/highlight.rs Outdated

matklad commented Aug 21, 2020
View reviewed changes

matklad force-pushed the rustc-lexer-rustdoc-highlight branch 2 times, most recently from ab1987c to 1d083ceCompare August 21, 2020 17:27

petrochenkov self-assigned this Aug 21, 2020

petrochenkov added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 21, 2020

ollie27 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Aug 22, 2020

tesuji reviewed Aug 22, 2020
View reviewed changes

Comment threadsrc/librustdoc/html/highlight.rs Outdated

petrochenkov removed their assignment Aug 22, 2020

matklad force-pushed the rustc-lexer-rustdoc-highlight branch 3 times, most recently from 3788b43 to 63f6533Compare August 27, 2020 10:52

matklad changed the title ~~WIP: Use rustc_lexer for rustdoc syntax highlighting~~Use rustc_lexer for rustdoc syntax highlightingAug 27, 2020

matklad force-pushed the rustc-lexer-rustdoc-highlight branch from 63f6533 to 957511eCompare August 27, 2020 10:59

GuillaumeGomez reviewed Aug 27, 2020
View reviewed changes

Comment threadsrc/librustdoc/Cargo.toml Outdated

GuillaumeGomez reviewed Aug 27, 2020
View reviewed changes

Comment threadsrc/librustdoc/html/highlight/sample.html Outdated

GuillaumeGomez reviewed Aug 27, 2020
View reviewed changes

Comment threadsrc/librustdoc/html/highlight/sample.rs Outdated

GuillaumeGomez reviewed Aug 27, 2020
View reviewed changes

Comment threadsrc/librustdoc/html/highlight/tests.rs Outdated

matklad force-pushed the rustc-lexer-rustdoc-highlight branch 2 times, most recently from 5f18bab to f54359dCompare August 27, 2020 11:34

GuillaumeGomez approved these changes Aug 27, 2020
View reviewed changes

jyn514 reviewed Aug 27, 2020
View reviewed changes

Comment threadsrc/librustdoc/html/highlight.rs Outdated
Comment threadsrc/librustdoc/html/highlight.rs Outdated
Comment threadsrc/librustdoc/test/fixtures/sample.html Outdated

matklad force-pushed the rustc-lexer-rustdoc-highlight branch from f54359d to 22f709fCompare August 27, 2020 12:28

matklad mentioned this pull request Aug 27, 2020
Next-generation syntax highlighting for rustdoc #75981
Open

matklad force-pushed the rustc-lexer-rustdoc-highlight branch from 22f709f to fe9d84dCompare August 27, 2020 12:49

ollie27 reviewed Aug 27, 2020
View reviewed changes

Remove dependency on StringReader from rustdoc highlighter
1f95a91
rustc_lexer is the lossless lexer, which is a better fit for approximate syntax highlighting. As a side-effect, we can now syntax-highlight even broken code.

matklad force-pushed the rustc-lexer-rustdoc-highlight branch from fe9d84d to 21c4bd9Compare August 27, 2020 16:01

euclio reviewed Aug 27, 2020
View reviewed changes

Comment threadsrc/librustdoc/html/highlight/fixtures/sample.rs Outdated

matklad force-pushed the rustc-lexer-rustdoc-highlight branch from 21c4bd9 to 0b16ff3Compare August 27, 2020 16:35

Add expect test for rustdoc html highlighting
b4f4db9
It's a unit-test in a sense that it only checks syntax highlighting. However, the resulting HTML is written to disk and can be easily inspected in the browser. To update the test, run with `--bless` argument or set `UPDATE_EXPEC=1` env var

matklad force-pushed the rustc-lexer-rustdoc-highlight branch from 0b16ff3 to b4f4db9Compare August 27, 2020 16:52

matklad added a commit to matklad/rust that referenced this pull request Aug 29, 2020
Allow --bess ing expect-tests in tools
e6a2c82
See rust-lang#75773 and rust-lang#75775

matklad added the WG-parselib label Aug 29, 2020

bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2020

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 30, 2020

bors merged commit ced37a5 into rust-lang:masterAug 30, 2020

jyn514 mentioned this pull request Nov 15, 2020
rustdoc: DOS line endings cause extra newlines to show up in source view #76361
Closed

jyn514 mentioned this pull request May 5, 2021
Remove unneeded call to with_default_session_globals in rustdoc highlight #84953
Merged

cuviper added this to the 1.48.0 milestone May 2, 2024

Uh oh!

Uh oh!

Conversation

matklad commented Aug 21, 2020

Uh oh!

matklad commented Aug 21, 2020

Uh oh!

Uh oh!

GuillaumeGomez commented Aug 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matklad commented Aug 21, 2020

Uh oh!

Uh oh!

matklad commented Aug 27, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matklad commented Aug 27, 2020

Uh oh!

GuillaumeGomez commented Aug 27, 2020

Uh oh!

jyn514 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matklad commented Aug 27, 2020

Uh oh!

jyn514 commented Aug 27, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

matklad commented Aug 27, 2020

Uh oh!

GuillaumeGomez commented Aug 29, 2020

Uh oh!

bors commented Aug 29, 2020

Uh oh!

bors commented Aug 29, 2020

Uh oh!

bors commented Aug 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants