Skip to content
This repository has been archived by the owner. It is now read-only.

Support alternative frontends #191

Merged
merged 5 commits into from Oct 17, 2017

Conversation

Projects
None yet
3 participants
@euclio
Copy link
Contributor

euclio commented Oct 15, 2017

This PR extracts the ember-specific logic into a rustdoc-ember binary. Alternative frontends may be used by setting the RUSTDOC_FRONTEND environment variable.

I've also updated the README. Does the proposed interface for frontends make sense? I also considered sending data.json through stdin.

Fixes #52.

@euclio euclio force-pushed the euclio:frontend branch from 857e098 to 3df5ef6 Oct 15, 2017

@steveklabnik

This comment has been minimized.

Copy link
Owner

steveklabnik commented Oct 15, 2017

I am psyched about this! Will probably not get to a review until tomorrow though. Thanks!

@euclio euclio force-pushed the euclio:frontend branch from 3df5ef6 to a9f7f2b Oct 15, 2017

@mgattozzi

This comment has been minimized.

Copy link
Contributor

mgattozzi commented Oct 15, 2017

@euclio This is great!

@euclio euclio force-pushed the euclio:frontend branch from a9f7f2b to 2fd98a2 Oct 15, 2017

@steveklabnik
Copy link
Owner

steveklabnik left a comment

thanks a ton, overall this looks great! two inline questions for discussion

README.md Outdated
conforms to a particular interface.

- The frontend must allow a single positional argument. This argument will be
the path to the documentation JSON generated by the backend.

This comment has been minimized.

@steveklabnik

steveklabnik Oct 16, 2017

Owner

yeah, I wonder if maybe streaming over stdin isn't better. I feel like it's a tradeoff; my frontend would need to take that stdin and write it to a file anyway, whereas your implementation currently needs to read the file outputted.

I think I'm going to argue that it should be over stdin, because in that case, it only needs to write a file if it actually needs to write a file, otherwise we've left an extraneous file around. Thoughts?

This comment has been minimized.

@euclio

euclio Oct 17, 2017

Author Contributor

Yeah, I think stdin makes more sense. Certainly if we want to support streaming output.

@@ -0,0 +1,85 @@
#[macro_use]

This comment has been minimized.

@steveklabnik

steveklabnik Oct 16, 2017

Owner

what do you think about making this a workspace, with rustdoc-ember as a crate inside of it? This would let us get rid of the build script for the main rustdoc crate, which feels cleaner to me.

This comment has been minimized.

@euclio

euclio Oct 17, 2017

Author Contributor

I'd prefer a workspace as well, but I don't think it's possible to force all crates be built? I'm trying to avoid the situation where you only build the backend and then the frontend can't be found. Though, this does already happen if you just cargo run --bin rustdoc without cargo build. With a workspace you'll have to remember to cargo build --all instead of just cargo build. If that's acceptable I'm happy to make it into a separate crate.

This comment has been minimized.

@steveklabnik

steveklabnik Oct 17, 2017

Owner

It is, IMO.

@euclio euclio force-pushed the euclio:frontend branch from 3baada9 to 5d95021 Oct 17, 2017

euclio added some commits Oct 15, 2017

@euclio euclio force-pushed the euclio:frontend branch from 5d95021 to ca4b39c Oct 17, 2017

@euclio

This comment has been minimized.

Copy link
Contributor Author

euclio commented Oct 17, 2017

Rebased and ready for review.

@steveklabnik
Copy link
Owner

steveklabnik left a comment

Awesome, thank you! So pumped about this.

@steveklabnik steveklabnik merged commit 078db5b into steveklabnik:master Oct 17, 2017

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@steveklabnik steveklabnik referenced this pull request Oct 30, 2017

Closed

Tools news #28

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.