Skip to content

Add a cargo-doc command #254

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

Merged
merged 1 commit into from
Jul 28, 2014
Merged

Add a cargo-doc command #254

merged 1 commit into from
Jul 28, 2014

Conversation

alexcrichton
Copy link
Member

This is blocked until rust-lang/rust#15939 lands, but in the meantime I figured I could get some eyes on this to make sure I'm sane.

Closes #130

pub fn doc(manifest_path: &Path,
options: &mut DocOptions) -> CargoResult<()> {
let mut src = PathSource::for_path(&manifest_path.dir_path());
let _ = fs::rmdir_recursive(&manifest_path.dir_path().join("doc"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems... bad. Someone creates doc/... with a variety of handwritten docs (not realising that this is cargo's build directory), and runs cargo doc, losing everything.

Maybe it should be target/doc too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured I'd duplicate what rust does in that doc is the build directory while src/doc is where handwritten documentation goes. Currently the build output of cargo doc (libraries and such) is placed in target/doc, but that could move to target/doc-build if we want.

I also figured that "target" implies "build artifacts from the compiler" less so than "documentation"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is specifically the rmdir, since it is silently killing the whole directory; and it is very reasonable and fairly common to have a ./doc directory with handwritten docs. e.g.

And there's a pile of projects using ./docs too. About half of the projects I randomly clicked on in Github's "showcases" had either ./doc or ./docs as (AFAICT) human written docs.

I think it's a really bad idea to force-remove the directory, destroying people's work, and it seems suboptimal to put machine generated output into it by default (although I agree that "target" seems a little weird for documentation).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(We could encourage people to use ./src/doc, but having a command that mercilessly punishes deviation/mistakes by deleting it seems like a bad idea.)

@alexcrichton
Copy link
Member Author

I've updated with build output in target/doc-build and documentation output in target/doc.

@apoelstra
Copy link

A minor thing is that I don't think doc shows up in cargo help with this.

@sfackler
Copy link
Member

This doesn't handle doc tests, correct?

@alexcrichton
Copy link
Member Author

@sfackler, not currently, but that's what I'd like to work on next for rustdoc integration.

bors added a commit that referenced this pull request Jul 28, 2014
This is blocked until rust-lang/rust#15939 lands, but in the meantime I figured I could get some eyes on this to make sure I'm sane.

Closes #130
@bors bors merged commit 2c5af68 into rust-lang:master Jul 28, 2014
@alexcrichton alexcrichton deleted the cargo-doc branch July 28, 2014 20:38
bors added a commit to alexcrichton/cargo that referenced this pull request Sep 2, 2014
This is blocked until rust-lang/rust#15939 lands, but in the meantime I figured I could get some eyes on this to make sure I'm sane.

Closes rust-lang#130
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.

cargo doc command
6 participants