[rustdoc] Move line numbers into the <code> directly

[GuillaumeGomez]

Fixes #84242.

This is the first for adding support for #127334 and also for another feature I'm working on.

A side-effect of this change is that it also fixes source code pages display in lynx since they're not directly in the source code.

To allow having code wrapping, the grid approach doesn't work as the line numbers are in their own container, so we need to move them into the code. Now with this, it becomes much simpler to do what we want (with CSS mostly). One downside: the highlighting became more complex and slow as we need to generate some extra HTML tags directly into the highlighting process. However that also allows to not have a huge HTML size increase.

You can test the result here and here.

The appearance should have close to no changes.

r? @notriddle

[rustbot]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

[yotamofek]

... I think?

Suggested change

	write!(out, "{extra}<a href=\"#{line}\" id={line} data-nosnippet>{line}</a>",).unwrap();
	write!(out, "{extra}<a href=\"#{line}\" id=\"{line}\" data-nosnippet>{line}</a>",).unwrap();

[GuillaumeGomez]

It's a number so I'm cheating here. That allows to get 2 characters x number of lines, so it becomes interesting for big crates.

[yotamofek]

Cool, didn't know that was valid in HTML5!

[GuillaumeGomez]

It's valid in all HTML as far as I know. 😉

[notriddle]

You can do that with the href, too. Unquoted attributes are allowed to contain hashes.

It would probably also be good to add a Rust comment pointing out what you're doing and why.

[TimNN]

According to https://caniuse.com/?search=user-select, this needs prefixes for Safari (and IE).

(I have confirmed that on Safari, for the two linked examples, the line numbers currently get selected).

[GuillaumeGomez]

Didn't know, thanks for that!

[GuillaumeGomez]

Finally fixed the tests and made tidy happy. ^^'

[notriddle]

This look really nice. It fixes the problem, and it also fixes #84242

                                                                 lib.rs - source
Files

   scrape_examples/
   lib.rs

1//@ run-flags:-Zrustdoc-scrape-examples
2/// # Examples
3///
4/// ```
5/// test();
6/// test();
7/// ```
8pub fn test() {}
9
10pub fn test_many() {}





Commands: Use arrow keys to move, '?' for help, 'q' to quit, '<-' to go back.
  Arrow keys: Up and Down to move.  Right to follow a link; Left to go back.
 H)elp O)ptions P)rint G)o M)ain screen Q)uit /=search [delete]=history list 

You can mention that in the PR description and we can finally close that issue out.

[notriddle]

You can do that with the href, too. Unquoted attributes are allowed to contain hashes.

It would probably also be good to add a Rust comment pointing out what you're doing and why.

[notriddle]

r=me after removing the quotes from the href attribute (assuming there's not some reason I wasn't aware of to keep them).

[GuillaumeGomez]

Wow nice, didn't know about the hashtag in the attributes. Thanks!

[GuillaumeGomez]

@bors r=notriddle rollup

[bors]

📌 Commit b594b9f has been approved by notriddle

It is now in the queue for this repository.

[Kobzol]

@rust-timer build 60306e7

[rust-timer]

Finished benchmarking commit (60306e7): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

	mean	range	count
Regressions ❌ (primary)	0.9%	[0.1%, 2.0%]	19
Regressions ❌ (secondary)	1.7%	[0.3%, 4.9%]	9
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.9%	[0.1%, 2.0%]	19

Max RSS (memory usage)

Results (primary 2.2%, secondary 2.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.2%	[2.2%, 2.2%]	2
Regressions ❌ (secondary)	2.2%	[2.2%, 2.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.2%	[2.2%, 2.2%]	2

Cycles

Results (primary 1.8%, secondary 2.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.8%	[1.8%, 1.8%]	1
Regressions ❌ (secondary)	2.5%	[1.5%, 4.0%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.8%	[1.8%, 1.8%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 789.14s -> 789.514s (0.05%)
Artifact size: 347.71 MiB -> 347.71 MiB (-0.00%)

[Kobzol]

This PR caused a number of doc regressions. I'll let you judge if it was fine, just wasn't sure if it was expected or not.

[notriddle]

I knew it would happen. The underlying cause is that this new format pushes more bytes of HTML than the old one. It's a hack that's imperfect both for producing non-bloated HTML and for a11y. This change also makes #136991 possible (which is ultimately what pushed it over the edge).

• We want small HTML. To reduce HTML bloat to zero, we could have used JavaScript to add line numbers, but that would require JS to make line-number-anchored URLs work
• We want to produce correct results in as many browsers as possible (including browsers with JS disabled, text browsers with no CSS support, and screen readers). To achieve perfect results, we could have used tables, but that would require syntax highlighting regions that span multiple lines to be declared separately on each line

The new version pushed in this PR does an imperfect job of making HTML both small and accessible at the same time. This is vividly shown in these Lynx screenshots:

Before	After
1 2 3 4 5 6 7 8 9 10 //@ run-flags:-Zrustdoc-scrape-examples /// # Examples /// /// ``` /// test(); /// test(); /// ``` pub fn test() {} pub fn test_many() {}	1//@ run-flags:-Zrustdoc-scrape-examples 2/// # Examples 3/// 4/// ``` 5/// test(); 6/// test(); 7/// ``` 8pub fn test() {} 9 10pub fn test_many() {}

First of all, the visible-on-screen line numbers are also where the anchors for links are found, so if someone shares a URL pointing at a particular line, the "After" version will jump you to that line. So that's much better than what we did before. Too bad there isn't any way to fix this without making the HTML bigger, because we have to mark up each line separately instead of doing it all at once.

The "Before" screenshot is clearly busted, but the "After" screenshot is also imperfect. Notice how the word pub on line 10 isn't in the same on-screen column as line 8? They're aligned in the actual source file, but Lynx doesn't know that.