Repository navigation
Conversation
| .current_dir(&self.root) | ||
| .arg("test") | ||
| .arg("--doc") | ||
| .args(["--message-format=json", "--config", "build.rustdoc='echo'"]); |
There was a problem hiding this comment.
The key magical incantation is using cargo with the right arguments. Those are message-format set to json, as well as the RUSTDOC command set to echo, which outputs two sets of information about the compile process to our captured stdout.
| #[non_exhaustive] | ||
| pub struct RustConfig { | ||
| /// Rust edition used in playground | ||
| /// DEPRECATED |
There was a problem hiding this comment.
Please don't.
There was a problem hiding this comment.
Well this has some consequences for good and bad
Good: philosophy of TIOWTDI: There Is One Way To Do It. Everyone places it in the same file.
Bad: you need an extra file two extra files just to declare the edition.
| let chapters = book | ||
| .iter() | ||
| .filter_map(get_chapter) | ||
| .map(|(chapter, path)| (path.clone(), chapter.name.clone(), chapter.content.clone())) |
There was a problem hiding this comment.
Unless I missed something, I don't think you need to clone anything here.
There was a problem hiding this comment.
| .map(|(chapter, path)| (path.clone(), chapter.name.clone(), chapter.content.clone())) | |
| book.items.into_iter().filter_map(get_chapter).map(|(chapter, path)| (path, chapter.name, chapter.content)) |
Like that?
Because the items field is public.
Doesn't make that much of a difference, because this piece is not performance-critical. Thanks,
| let output = cargo.output().ok()?; | ||
|
|
||
| if !output.status.success() { | ||
| // failed = true; |
There was a problem hiding this comment.
Why commented?
There was a problem hiding this comment.
@GuillaumeGomez That leftover shouldn't be there. I am removing it, because this line would be for tests, not cargo.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
|
@GuillaumeGomez Thanks for the review. There is indeed a bunch of changes, which are all necessary because they allow us to keep old deprecated or remaining functionality while adding the new functionality through Cargo manifests. Some parts are there just to massage the arguments into correctness. Half of the changes just kind of are for testing, documentation etc. So they may seem unrelated while they are all necessary. |
|
@GuillaumeGomez Some of this code is there to, for example, permit you to jump to error location in your editor. This works perfectly well unless you include program content inline. I checked. Which is also why there is some extra code. |
Except it's not what this feature is about. Remove it. |
|
@GuillaumeGomez I don't see why an improvement should be removed if all it takes is a few lines. It should be okay to refactor or make small functional changes to the same method you are currently working on. |
|
No, please keep your PRs focused on one topic. If you have other improvements in mind you are welcome to open separate PRs. |
|
@GuillaumeGomez Okay, sure, thanks. What I am thinking this code will break in the future in case someone changes the arguments from cargo to rustdoc, so it needs some way to ensure the cargo project does not break this. What I am saying there is already one instance of whitespace in the arguments and we need some way to ensure no further whitespace appears within command line arguments, or that they are successfully removed. |
|
This one is |
|
Then it's not the right approach. You can't really guess what the correct way to parse is unless you reimplement the binary CLI. |
|
@GuillaumeGomez Mmmmmm, yes. I think I can fix this. With a few tiny changes to cargo. When new cargo version is released, the code would work on top of it, and otherwise keep the echo. |
|
@GuillaumeGomez I just don't want to add hundreds of lines of code here. This change is a net addition of just 230 lines. |
Fixes #706
Fixes #394
Supersedes #2503
Supersedes #3137
Allows doc snippets to freely depend on external crates.
The key magical incantation is using
cargowith the right arguments. Those aremessage-formatset tojson, as well as theRUSTDOCcommand set toecho, which skips the compile process and outputs two sets of information about the compile process to our captured stdout.Hopefully this is not too much of a hack, and saves a couple hundred lines of effort.