-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add a generator for an "error index" webpage. #25062
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
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Here's a preview: http://sproul.io/error-index/error-index.html |
I think my dependency on the stage 2 compiler is definitely wrong, as running |
RUSTC_EXE = $(HBIN2_H_$(CFG_BUILD))/rustc | ||
error-index: doc/error-index.html | ||
|
||
doc/error-index.html: src/etc/error-index-generator.rs $(RUSTC_EXE) | doc/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these dependencies aren't quite right because this needs to depend on libraries like libsyntax and librustdoc from stage2. I think you'll want to depend on CSREQ2_T_$(CFG_BUILD)_H_$(CFG_BUILD)
here.
Nice! Could you upload a preview of what this looks like as well? |
I also think it may be worth looking into |
There's a preview here: http://sproul.io/error-index/error-index.html I'll add the panic and look into the tools array now. |
I wonder if we could take a slightly different route: create a single big markdown file with all the error descriptions and then use a single (We'd still need a tool to create the markdown file.) |
@huonw: It's really easy to adjust the generator to do that (and I tried it), but I decided against it because it basically prevents us from adding cool features like filtering and searching. For filtering specifically, I think you need the wrapper divs to group the headers and descriptions together. Then you can hide unused errors for example with a tiny bit of JS: $(".error-unused").hide(); The wrapper divs are problematic because if you put markdown inside, |
Ah, good point. That could be addressed via some client-side javascript, but it may not be worth it. |
Now that I'm typing |
Yay! Wrapped my head around the build-system some more and made it a tool. |
@alexcrichton: I just verified that the |
I did something like this before, but made them on individual pages, rather than one big page. I think this is a good start though, 👍 |
@steveklabnik: I hadn't seen your approach but I just read through #16619 and may recycle some of your ideas. I particularly like the form of your error messages, with the summary and the examples 😄 |
Nice! The build system changes look great to me. Do you think the errors without descriptions should be listed at all? They do take up quite a bit of space :( |
@@ -288,3 +292,9 @@ doc/style/index.html: $(RUSTBOOK_EXE) $(wildcard $(S)/src/doc/style/*.md) | doc/ | |||
@$(call E, rustbook: $@) | |||
$(Q)rm -rf doc/style | |||
$(Q)$(RUSTBOOK) build $(S)src/doc/style doc/style | |||
|
|||
error-index: doc/error-index.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This rule doesn't seem to be included from anywhere, perhaps it could be added to the docs
rule so this is prepared automatically with the rest of the documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah! Good point, I'll fix that.
I also forget how we do this in rustdoc, but rustdoc at least has where when you click on a title it'll highlight it (also when you link to a title it'll highlight what you linked to on the page), but that doesn't seem to be happening here? Perhaps a JS or CSS include is missing from the main rustdoc abilities? Also, could this generate a markdown file and then use rustdoc to render that instead? That'd help keep it somewhat consistent with the book as well. |
Including Regarding the generated markdown, I don't think it's ideal, as I explained a few comments back: #25062 (comment) |
Perfect!
Wow I read that comment and then totally didn't connect the two together, my mistake! |
@alexcrichton: Great! I've added some CSS to hide the errors without descriptions by default. |
Are we ok to land this @alexcrichton? |
…xcrichton I've added backticks in a few places to ensure correct highlighting in the HTML output (cf rust-lang#25062). Other changes include: * Remove use of `1.` and `2.` separated by a code block as this was being rendered as two separate lists beginning at 1. * Correct the spelling of successful in two places (from "succesful"). Other changes are a result of reflowing text to stay within the 80 character limit.
…xcrichton I've added backticks in a few places to ensure correct highlighting in the HTML output (cf rust-lang#25062). Other changes include: * Remove use of `1.` and `2.` separated by a code block as this was being rendered as two separate lists beginning at 1. * Correct the spelling of successful in two places (from "succesful"). Other changes are a result of reflowing text to stay within the 80 character limit.
This PR adds a program which uses the JSON output from #24884 to generate a webpage with descriptions of each diagnostic error. The page is constructed by hand, with calls to `rustdoc`'s markdown renderers where needed. I opted to generate HTML directly as I think it's more flexible than generating a markdown file and feeding it into the `rustdoc` executable. I envision adding the ability to filter errors by their properties (description, no description, used, unused), which is infeasible using the whole-file markdown approach due to the need to wrap each error in a `<div>` (markdown inside tags isn't rendered). Architecturally, I wasn't sure how to add this generator to the distribution. For the moment I've settled on a separate Rust program in `src/etc/` that gets compiled and run by a custom makefile rule. This approach doesn't seem too hackish, but I'm unsure if my usage of makefile variables is correct, particularly the call to `rustc` (and the `LD_LIBRARY_PATH` weirdness). Other options I considered were: * Integrate the error-index generator into `rustdoc` so that it gets invoked via a flag and can be built as part of `rustdoc`. * Add the error-index-generator as a "tool" to the `TOOLS` array, and make use of the facilities for building tools. The main reason I didn't do this was because it seemed like I'd need to add lots of stuff. I'm happy to investigate this further if it's the preferred method.
The handling of |
It doesn't have backticks in the source, should be alright as |
Fixed in #25331 |
This PR adds a program which uses the JSON output from #24884 to generate a webpage with descriptions of each diagnostic error.
The page is constructed by hand, with calls to
rustdoc
's markdown renderers where needed. I opted to generate HTML directly as I think it's more flexible than generating a markdown file and feeding it into therustdoc
executable. I envision adding the ability to filter errors by their properties (description, no description, used, unused), which is infeasible using the whole-file markdown approach due to the need to wrap each error in a<div>
(markdown inside tags isn't rendered).Architecturally, I wasn't sure how to add this generator to the distribution. For the moment I've settled on a separate Rust program in
src/etc/
that gets compiled and run by a custom makefile rule. This approach doesn't seem too hackish, but I'm unsure if my usage of makefile variables is correct, particularly the call torustc
(and theLD_LIBRARY_PATH
weirdness). Other options I considered were:rustdoc
so that it gets invoked via a flag and can be built as part ofrustdoc
.TOOLS
array, and make use of the facilities for building tools. The main reason I didn't do this was because it seemed like I'd need to add lots of stuff. I'm happy to investigate this further if it's the preferred method.