Anchors on listings

[mianlang]

I think I made it by the following four main steps, as the first four commits said:

1. add id to Listing with number attribute for cross reference
   Finally make my change in listing mod into effect by cargo install --locked --path packages/mdbook-trpl after mod updated.
   Now each rendered snippet automatically has an id attribute like id="listing-16-6" for referencing.

2. change listing reference from text to link
   Replace Listing[ \n](\d+[\d-]+) to [Listing $1](#listing-$1). Change 672 results in 72 files.

3. fix: prefix the actual file name if referenced listing is in another ch.md
   Using script generated by such prompt:

i have a folder of .md files, could you please give me a python script to achieve this:

1. find out all lines containing such pattern 1 #listing-([\d-]+). the matched part in parentheses will be used as $1 in the next step.
2. in the preceding lines, test if the same file contain pattern 2 number="$1". if not, find the actual file name which contains pattern 2, alias as $a, and change pattern 1 to pattern 3 $a#listing-$1, apply the change to file.

4. fix those grouped reference starts with Listings
   Manually, fix 3 results in 2 files.

Another two commits to fix linting and package tests:

5. fix lint errors
   Which is caused by omissions in step 3.

6. update tests after new attribute id added

Let me know if anything should get improved. @chriskrycho #921

[mianlang]

Sorry for not having test locally and failling the check.
I'll try to fix them. Looks like I should update test cases also for changes in mdbook-trpl. And broken link fragments may caused by omissions in step 3.

[mianlang]

Linting and package tests have passed. Not sure whether I should add a test for a listing without a number attribute, as there wasn't one before. Please let me know if anything needs to be changed.

[chriskrycho]

Thank you for doing this! I think the best move here is a slight variation on what you've done: Keep the figure id="..." the same (with one tweak suggested on the code itself, just a minor style nit), but instead of manually coding in the links throughout, which will impose a maintenance burden on contributors to keep up to date, let’s instead update the <figcaption> part of the output to link the headers, so we’ll end up with output like this:

<figcaption>
  <a href="listing-{number}">Listing {number}</a>: {caption}
</figcaption>

That will make it easy for people to link to using that link itself, and critically means that the only changes anyone will ever need to make to it will be updating the number at <Listing number="1-1" ...> and the text of the reference to it, not also the link to it. As silly as it might seem (why would anyone forget that?) I can tell you that my experience updating a bunch of listings lately is: it’s really easy to miss or mess up different parts of it, so the more we can get it to be right without manual checking the better that will be for us.

The other side of that is: we don’t really need the links inline in the text in the same way in the general case, though we might consider doing it in a few cases when linking across chapters. I say “might” because of the aforementioned “it’s hard to make sure you got the updates all correct” dynamic, and the link-checker will generally not help us here unless we happen to be linking to a numbered listing that we deleted along with all consecutive listings in that chapter—it’ll just end up pointing to the wrong listing. (That doesn’t happen very often, as you can probably imagine!) Net, you’ll want to do something like a git restore src --source main to get back all the content without the links.

Thanks again! I look forward to landing this with those tweaks in place.

[mianlang]

Great idea! I did considered keeping listings and refereces synced for easier maintenance, but I turned for some sort of service registry system and found it too complex.

Are you suggesting extending the Listing component so that it not only render the listing itself but also supports rendering a reference (an <a> tag with text and href) targeting it, controlled by a boolean attribute such as link? For example:

<!-- create a listing -->
<Listing number="1-2" file-name="src/main.rs" caption="Some *text*, yeah?">
    some snippets
</Listing>

<!-- later reference it -->
It should look similar to the code in Listing 1-2. <!-- original. can't navigate -->
It should look similar to the code in [Listing 1-2](#listing-1-2). <!-- delivered in PR, navigate to the listing -->
It should look similar to the code in <Listing number="1-2" link />. <!-- you suggested. am i right? -->

When we need to change, simply search and replace all number="1-2" with number="1-4" will do. A clever and cute idea.

I believe we can split this into two.

Firstly, what was happening and people complained here. I think the biggest problem is, text like Listing 1-2 lacks direct link, making navigation to the referring snippet frustrating. I was following the suggestion here and I think I delivered it overall. Whenever you want to refer to a listing, you'd be able to use its id.

Secondly, maintenance considerations. Adding a new reference is intuitive and including an extra target link is reasonable. Changing filename or the number of listing, may introduce additional overhead. Thorough search and confirmation before making such changes can mitigate. Implementing your suggestion would help a lot, replacing all number="1-2" being significantly easier than manually searching 1-2 and verifying one by one. Let me know if I've understood your thought correctly and if this aligns with your intent.

I'll resolve the conflicts and incorporate your suggestion on mod.rs (much appreciated from a Rust newcomer). I also think we might be able to merge this PR and discuss the maintenance considerations in a separate thread. But I'll leave the decision to you.

[mianlang]

There were 2 lint errors at the main HEAD, duplicate listing number number="20-40" (introduced by 56ec353 from #4272). I fixed it by correct the second one to 20-41, since I found the next was 20-42. I have no clue why that passed the lint check.

Since there are many conflicts and I find it hard to check and apply changes one by one, I re-executed those steps.

I discovered another benefit of this. After making the preceding changes, I ran the lint check, and it complained a broken link fragment: #listing-16-6. Upon verification, I found this to be true–the listing 16-6 was not delivered as a Listing component but was in a raw way. This can also happen when a listing gets changed or deleted. This serves as a safeguard to catch unintended changes and maintain link integrity.

[chriskrycho]

Are you suggesting extending the Listing component so that it not only render the listing itself but also supports rendering a reference (an <a> tag with text and href) targeting it, controlled by a boolean attribute such as link?

Ah, that’s an interesting idea, but no: the main thing I was suggesting was to make the Listing itself contain a link in its output, so that this input:

<Listing number="1-2" caption="Something interesting" file-name="src/main.rs">

```rust
fn main() {
    println!("Hello, world!");
}
```

</Listing>

Would generate output like this:

<figure class="listing">
<span class="file-name">Filename: src/main.rs</span>

```rust
fn main() {
    println!("Hello, world!");
}
```

<figcaption>
  <a href="listing-1-2">Listing 1-2</a>: Something interesting
</figcaption>
</figure>

Note the link here: <a href="listing-1-2">Listing 1-2</a>: Something interesting. That gives people an easy way to link to a listing from the listing, and also thereby helps a bit with accessibility of that “navigation” item.

This doesn’t fully solve the in-text linking issue that people originally called out, and that your PR, but it is a good starting point for making it easy to link to them.

Then there is separately a question of how to do in-text links to those—that is, the problem you set out to solve here in the first place! My very, very strong inclination is that we should absolutely not do that manually. As one of the main people who is maintaining this, maintenance is not a secondary concern at all, nor something to be addressed later. 😅 Rather the opposite, in fact!

One thing we could do, however, is create a preprocessor that takes the text and finds any instance of Listing\s(\d+-\d+) (note: with settings such that \s includes \n!) and replaces it with:

<a href="$1" class="listing-link">Listing $1</a>

That will be pretty robust in that as long as the text is correct, the link will be as well, and it means that neither contributors nor maintainers need to think about doing something funky there; we can just write a normal reference to a listing in text and have it be auto-linked in the web version while not needing to do any work for the nostarch version of the text—it just wouldn’t use the preprocessor at all. (That’s particularly helpful when we’re doing updates to the print version for a new print version, as we are late in the process of doing right now.) That should still also get us the upsides of the linkcheck feedback on those that you observed, which is nice!

Net, I think this is how we should proceed:

1. On this PR we only want the change to add the ID to the figure (which you already have) and the new link to the listing number in the figcaption.
2. Please go ahead and remove all the changes to add links inline. (Sorry, I would have waved you off from that ahead of time if I had known someone was thinking about doing it manually, for exactly these reasons!)
3. Please also split out the fix for listing duplicate listing numbers 20-40 into a separate little PR, seeing as it is unrelated.

Thanks for your patience as we iterate toward a good, easy-to-maintain solution here!

[mianlang]

Steps 1 and 2 are done. #4287 fixes step 3 and should be merged first, as this PR doesn't pass lint without that change.

Now I finally see the value of making a self–referencing <a> tag–once I click it, I can easily copy the link from the URL or the context menu, which makes referencing much more convenient.

I truly admire your ability to organize thoughts and pinpoint key issues, and I completely agree that maintainability should be the top priority. The preprocessor approach is certainly a cleaner solution.

Thanks for your patience and for your thoughtful, detailed review–it means a lot!

[chriskrycho]

This looks great! As for the careful and detailed review—you’re very welcome, and thanks again in turn for taking the time to make this really good. Let me know if you’d like to implement the other new preprocessor; otherwise I’m happy to do it. If you do, the existing examples should be a good guide about how to do it. I’ll take responsibility for landing that upstream in rust-lang/rust either way!

[mianlang]

I've rebased main to adopt that lint fix and passed checks.

I'd pretty much like to implement the new preprocessor, and I actually gave it a try–with some help from Copilot–and managed to get something working in #4289.

One major thing blocking me at the moment is that I have no idea how to handle cross-chapter references. If you have any ideas I could look at, that would be super helpful!