Skip to content
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

initial work for cargo doc for markdown #2256

Closed
wants to merge 2 commits into from

Conversation

steveklabnik
Copy link
Member

The beginnings of #739

This handles the building of a docdirectory, but not cargo test yet.

Does this look like it's on the right track?

@rust-highfive
Copy link

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@steveklabnik
Copy link
Member Author

/cc @chriskrycho

@chriskrycho
Copy link

👍

One of the things I was thinking about with this was including the --markdown-css bit. Maybe pull it in from Cargo.toml? (Honestly, that was the biggest thing that was blocking me going on it, because I just didn't even know where to start on that.)

@alexcrichton
Copy link
Member

Looks like it's mostly along the right lines to me, thanks @steveklabnik! Some thoughts:

  • This may want to recurse deeply to probe copy the entire filesystem structure
  • Executing rustdoc will want to be tweaked, but you can see examples when running doctests for cargo test
  • Tests :)
  • It may be worth putting this in the backend to get parallelized
  • We'll probably want configuration keys for the project to do things like:
    • Change the root folder
    • Disable this functionality entirely
    • etc

@steveklabnik
Copy link
Member Author

Great! I wanted to hear from you before I did more work, that all seems reasonable.

This may want to recurse deeply to probe copy the entire filesystem structure

Should I use the walkdir crate for this? I forget what Cargo's policies on including external crates is.

It may be worth putting this in the backend to get parallelized

I don't know enough about Cargo's architecture to know what this would look like, any pointers?

@alexcrichton
Copy link
Member

Nah I'd just recurse manually, it should be a pretty simple recursive traversal.

For now I guess we can avoid parallelization, would be good to at least get the framework and tests written first. To land this, however, I think we may need that (and I can help)

@steveklabnik
Copy link
Member Author

(and I can help)

That would be rad, if you have the time, since you know Cargo way better than me.

@alexcrichton
Copy link
Member

Sure, let's get the main bulk of stuff set up first (e.g. get the interface and all the tests working), and then after that we can do all that.

Another reason I just thought of for using the parallelization backend is that it also tracks dirtiness by default (e.g. doesn't re-run rustdoc unless it needs to), which is likely desirable.

@steveklabnik
Copy link
Member Author

Hm, I thought removing the unwraps would help here. Need to dig deeper.

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to reopen if this is filled out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants