Generate error index with mdbook instead of raw HTML pages #101166

GuillaumeGomez · 2022-08-29T19:01:20Z

This is a follow-up of #100922.

This comes from a remark from @estebank who said that the search was a nice thing on the previous version and that it wasn't possible anymore. An easy way to come around this limitation was to use mdbook, which is what I did here.

Now some explanations on the code. I'll explain how I developed this and why I reached this solution. First I did it very basically by simply setting the source directory and the output directory, generated a SUMMARY.md manually which listed all error codes and that was it. Two problems arose from this:

A lot of new HTML files were generated at the top level
An index.html file was generated at the top-level (the summary in short).

So for 1., it's not great to have too many files at the top-level as it could create file conflicts more easily. And for 2., this is actually a huge issue because <doc.rust-lang.org> generates an index.html file with a links to a few different resources, so it should never be overwritten. Unfortunately, mdbook always generates an index.html file so the only solution I could see (except for sending them a contribution, I'll maybe do that later) was to temporaly move a potentially existing index.html file and then puts it back once done. For this last part, to ensure that we don't return before it has been put back, I wrapped the mdbook generation code inside render_html_inner which is called from render_html which in turn handle the "save" of index.html.

EDIT: mdbook completely deletes ALL the content in the target directory so I instead generate into a sub directory and then I move the files to the real target directory.

To keep compatibility with the old version, I also put the <script> @notriddle nicely provided in the previous PR only into the error-index.html file to prevent unneeded repetition. I didn't use mdbook additional-js option because the JS is included at the end of all HTML files, which we don't want for two reasons:

It's slow.
We only want it to be run in error-index.html (even if we also ensure in the JS itself too!).

Now the last part: why we generate the summary twice. We actually generate it once to tell mdbook what the book will look like and a second time because a create a new chapter content which will actually list all the error codes (with the updated paths).

EDIT: I removed the need for two summaries.

You can test it here.

r? @notriddle

GuillaumeGomez · 2022-08-29T19:57:24Z

Not sure why it failed. I saw a potential bug (returning too early from remove_file) so I fixed it.

Urgau · 2022-08-29T19:59:04Z

Looks good to me. 👍

Just wondering if the "Rust error codes index" chapter is really necessary since all the errors code are already listed on the left.

It might also be good to add the chapter (the error code) on the actual page, the only way to know which one you're looking is to look at the chapter list.

notriddle · 2022-08-29T20:44:36Z

src/tools/error_index_generator/book_config.toml

+boost-hierarchy = 2
+boost-paragraph = 1
+expand = true
+heading-split-level = 2


This one is a minor annoyance, the other one is the actual blocker:

searchindex.json is bigger than the original search-index.html, before it was split up into multiple pages.

$ du -h error-index.html searchindex.json 1.5M error-index.html 1.8M searchindex.json

I know heading-split-level = 0 helped before with reducing the size of the search index, and I suppose downloading the entire error index opt in is better than downloading it all no matter what, but it still seems disappointing, like we just undid all the gains of splitting into multiple pages and then some for anyone that wants to search the site.

To be noted: the size issue was a problem because it was an HTML page which made the rendering very slow and problematic on mobile devices. In this case, it's "better" considering it's not loaded by default and is not part of the DOM.

notriddle · 2022-08-29T20:47:06Z

src/tools/error_index_generator/main.rs

-
-        let mut id_map = IdMap::new();
-        let playground =
-            Playground { crate_name: None, url: String::from("https://play.rust-lang.org/") };


The part I consider really important is the lost functionality.

Stuff like this:

That "untested" indicator is gone. I know The Book has indicators for snippets that don't compile. Could that be copied here?

This is not really good looking imo. As for how, it's generated with this JS.

I think I'll do something closer to what rustdoc has.

GuillaumeGomez · 2022-08-29T21:40:05Z

While working on @Urgau's suggestion, I realized I could simplify the code a lot so I did.

I updated the online demo but didn't fix the missing attribute for ignore and compile_fail. Just to be sure: @notriddle if the missing attributes are put back, do you think it's worth it to merge this switch to mdbook or not? If not, let's just close this PR. Otherwise I'll add the missing attributes (with JS) tomorrow.

notriddle · 2022-08-29T21:45:51Z

I think @ehuss would be better able to say if adding these missing attributes to upstream mdBook would make sense.

Urgau · 2022-08-29T21:52:12Z

Not sure that it is a big deal but hidden code also regressed:

EDIT: This is because the code block isn't rust cf. https://rust-lang.github.io/mdBook/format/mdbook.html?highlight=ANCHOR#hiding-code-lines

EDIT 2: compile_fail, ignore, no_run are already supported but only with rust cf. https://rust-lang.github.io/mdBook/format/mdbook.html?highlight=ANCHOR#rust-code-block-attributes

GuillaumeGomez · 2022-08-30T09:54:16Z

@Urgau This is a big regression since we don't have syntax highlighting either. I'll fix both at once by adding the rust attribute.

GuillaumeGomez · 2022-08-30T13:02:08Z

So I added something like what we have in rustdoc and it allowed me to discover a bug in mdbook so I opened rust-lang/mdBook#1883.

GuillaumeGomez · 2022-08-30T20:12:12Z

I think I fixed all issues now.

notriddle · 2022-08-31T16:34:49Z

src/tools/error_index_generator/book_config.toml

+additional-css = ["extra.css"]
+additional-js = ["extra-info.js"]


I notice that the error-index.html in https://doc.rust-lang.org/nightly/error-index.html sits at the top level of the docs directory, and, as far as I can tell, this means that the CSS and JavaScript assets created by mdBook are going to sit at the top level of the docs directory when this merges. For example, this will create https://doc.rust-lang.org/nightly/extra.css.

That makes me worried about potentially creating more filename conflicts, like the index.html conflict that you already fixed.

Can we give these two files a more descriptive name than extra? For example, name them error-index.css and error-index.js?

Alternatively, can we make mdBook put all the error-index related assets in a subdirectory? For example, we could make error_codes be the book output directory, including putting the error-index.html in there, and just leave the old top-level error-index.html as a redirect to point inside the book?

Can we give these two files a more descriptive name than extra? For example, name them error-index.css and error-index.js?

Very good idea!

Alternatively, can we make mdBook put all the error-index related assets in a subdirectory? For example, we could make error_codes be the book output directory, including putting the error-index.html in there, and just leave the old top-level error-index.html as a redirect to point inside the book?

That would break all the current links no? But otherwise, it can be done indeed.

Not necessarily.

The way I would consider doing it is,

Make error_codes itself the output directory. error-index.html becomes error_codes/index.html. The error_codes/E\d\d\d.html files are not moved at all by this change.

At the path where the old error-index.html used to be, put a mostly-empty redirect, including the #E\d\d\d JavaScript code, but tweaked so that in the absence of a window.location.hash, it instead redirects to error_codes/index.html

GuillaumeGomez · 2022-08-31T18:32:02Z

@notriddle I renamed the two files. Just waiting for a confirmation before moving the whole error index into a sub folder.

GuillaumeGomez · 2022-08-31T19:00:58Z

I moved the book into a sub-folder. I also updated the online demo. No need to handle mdbook weird things now.

GuillaumeGomez · 2022-09-01T12:25:02Z

I removed the 404.html file to prevent the broken links error (and we don't need it in any case so...).

notriddle · 2022-09-01T14:01:36Z

@bors r+

bors · 2022-09-01T14:01:40Z

📌 Commit f5857d5 has been approved by notriddle

It is now in the queue for this repository.

… r=notriddle Generate error index with mdbook instead of raw HTML pages This is a follow-up of rust-lang#100922. This comes from a remark from `@estebank` who said that the search was a nice thing on the previous version and that it wasn't possible anymore. An easy way to come around this limitation was to use `mdbook`, which is what I did here. Now some explanations on the code. I'll explain how I developed this and why I reached this solution. First I did it very basically by simply setting the source directory and the output directory, generated a `SUMMARY.md` manually which listed all error codes and that was it. Two problems arose from this: 1. A lot of new HTML files were generated at the top level 2. An `index.html` file was generated at the top-level (the summary in short). So for `1.`, it's not great to have too many files at the top-level as it could create file conflicts more easily. And for `2.`, this is actually a huge issue because <doc.rust-lang.org> generates an `index.html` file with a links to a few different resources, so it should never be overwritten. <s>Unfortunately, `mdbook` **always** generates an `index.html` file so the only solution I could see (except for sending them a contribution, I'll maybe do that later) was to temporaly move a potentially existing `index.html` file and then puts it back once done. For this last part, to ensure that we don't return *before* it has been put back, I wrapped the `mdbook` generation code inside `render_html_inner` which is called from `render_html` which in turn handle the "save" of `index.html`.</s> EDIT: `mdbook` completely deletes ALL the content in the target directory so I instead generate into a sub directory and then I move the files to the real target directory. To keep compatibility with the old version, I also put the `<script>` `@notriddle` nicely provided in the previous PR only into the `error-index.html` file to prevent unneeded repetition. I didn't use `mdbook` `additional-js` option because the JS is included at the end of all HTML files, which we don't want for two reasons: 1. It's slow. 2. We only want it to be run in `error-index.html` (even if we also ensure in the JS itself too!). <s>Now the last part: why we generate the summary twice. We actually generate it once to tell `mdbook` what the book will look like and a second time because a create a new chapter content which will actually list all the error codes (with the updated paths).</s> EDIT: I removed the need for two summaries. You can test it [here](https://rustdoc.crud.net/imperio/error-index-mdbook/error-index.html). r? `@notriddle`

… r=notriddle Generate error index with mdbook instead of raw HTML pages This is a follow-up of rust-lang#100922. This comes from a remark from ``@estebank`` who said that the search was a nice thing on the previous version and that it wasn't possible anymore. An easy way to come around this limitation was to use `mdbook`, which is what I did here. Now some explanations on the code. I'll explain how I developed this and why I reached this solution. First I did it very basically by simply setting the source directory and the output directory, generated a `SUMMARY.md` manually which listed all error codes and that was it. Two problems arose from this: 1. A lot of new HTML files were generated at the top level 2. An `index.html` file was generated at the top-level (the summary in short). So for `1.`, it's not great to have too many files at the top-level as it could create file conflicts more easily. And for `2.`, this is actually a huge issue because <doc.rust-lang.org> generates an `index.html` file with a links to a few different resources, so it should never be overwritten. <s>Unfortunately, `mdbook` **always** generates an `index.html` file so the only solution I could see (except for sending them a contribution, I'll maybe do that later) was to temporaly move a potentially existing `index.html` file and then puts it back once done. For this last part, to ensure that we don't return *before* it has been put back, I wrapped the `mdbook` generation code inside `render_html_inner` which is called from `render_html` which in turn handle the "save" of `index.html`.</s> EDIT: `mdbook` completely deletes ALL the content in the target directory so I instead generate into a sub directory and then I move the files to the real target directory. To keep compatibility with the old version, I also put the `<script>` ``@notriddle`` nicely provided in the previous PR only into the `error-index.html` file to prevent unneeded repetition. I didn't use `mdbook` `additional-js` option because the JS is included at the end of all HTML files, which we don't want for two reasons: 1. It's slow. 2. We only want it to be run in `error-index.html` (even if we also ensure in the JS itself too!). <s>Now the last part: why we generate the summary twice. We actually generate it once to tell `mdbook` what the book will look like and a second time because a create a new chapter content which will actually list all the error codes (with the updated paths).</s> EDIT: I removed the need for two summaries. You can test it [here](https://rustdoc.crud.net/imperio/error-index-mdbook/error-index.html). r? ``@notriddle``

… r=notriddle Generate error index with mdbook instead of raw HTML pages This is a follow-up of rust-lang#100922. This comes from a remark from ```@estebank``` who said that the search was a nice thing on the previous version and that it wasn't possible anymore. An easy way to come around this limitation was to use `mdbook`, which is what I did here. Now some explanations on the code. I'll explain how I developed this and why I reached this solution. First I did it very basically by simply setting the source directory and the output directory, generated a `SUMMARY.md` manually which listed all error codes and that was it. Two problems arose from this: 1. A lot of new HTML files were generated at the top level 2. An `index.html` file was generated at the top-level (the summary in short). So for `1.`, it's not great to have too many files at the top-level as it could create file conflicts more easily. And for `2.`, this is actually a huge issue because <doc.rust-lang.org> generates an `index.html` file with a links to a few different resources, so it should never be overwritten. <s>Unfortunately, `mdbook` **always** generates an `index.html` file so the only solution I could see (except for sending them a contribution, I'll maybe do that later) was to temporaly move a potentially existing `index.html` file and then puts it back once done. For this last part, to ensure that we don't return *before* it has been put back, I wrapped the `mdbook` generation code inside `render_html_inner` which is called from `render_html` which in turn handle the "save" of `index.html`.</s> EDIT: `mdbook` completely deletes ALL the content in the target directory so I instead generate into a sub directory and then I move the files to the real target directory. To keep compatibility with the old version, I also put the `<script>` ```@notriddle``` nicely provided in the previous PR only into the `error-index.html` file to prevent unneeded repetition. I didn't use `mdbook` `additional-js` option because the JS is included at the end of all HTML files, which we don't want for two reasons: 1. It's slow. 2. We only want it to be run in `error-index.html` (even if we also ensure in the JS itself too!). <s>Now the last part: why we generate the summary twice. We actually generate it once to tell `mdbook` what the book will look like and a second time because a create a new chapter content which will actually list all the error codes (with the updated paths).</s> EDIT: I removed the need for two summaries. You can test it [here](https://rustdoc.crud.net/imperio/error-index-mdbook/error-index.html). r? ```@notriddle```

…llaumeGomez Rollup of 9 pull requests Successful merges: - rust-lang#97739 (Uplift the `let_underscore` lints from clippy into rustc.) - rust-lang#99583 (Add additional methods to the Demand type) - rust-lang#100147 (optimization of access level table construction) - rust-lang#100552 (rustc_target: Add a compatibility layer to separate internal and user-facing linker flavors) - rust-lang#100827 (Simplify MIR opt tests) - rust-lang#101166 (Generate error index with mdbook instead of raw HTML pages) - rust-lang#101294 (Fix rust-lang#100844 rebase accident) - rust-lang#101298 (rustdoc: remove unused CSS `#main-content > .since`) - rust-lang#101304 (Add autolabels for `A-query-system`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…illaumeGomez doc: fix redirected link in `/index.html` Fallout from rust-lang#101166

GuillaumeGomez added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools A-error-codes Area: Explanation of an error code (--explain) labels Aug 29, 2022

rust-highfive assigned notriddle Aug 29, 2022

rustbot added the T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) label Aug 29, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2022

GuillaumeGomez mentioned this pull request Aug 29, 2022

Remove unneeded type attribute for <script> rust-lang/mdBook#1881

Merged

GuillaumeGomez force-pushed the error-index-mdbook branch from 66fa321 to 9873eec Compare August 29, 2022 19:56

notriddle reviewed Aug 29, 2022

View reviewed changes