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

Open docs with opener crate #1485

Closed
wants to merge 1 commit into from
Closed

Conversation

Seeker14491
Copy link
Contributor

The idea is to pull the doc-opening functionality from Cargo and rustup out into its own crate, as discussed in rust-lang/cargo#5701 (comment).

As I mentioned in Cargo's pull request, the handling of the $BROWSER environment variable changes on Linux.

@pickfire
Copy link
Contributor

pickfire commented Nov 2, 2018

@Seeker14491 Why not use open crate instead, it is quite minimal and does not depend on failure. (did 1.0 major release)

@Seeker14491
Copy link
Contributor Author

An advantage of opener is better error messages on Mac and Linux. Additionally, the main motivation of making the opener crate was to unify cargo and rustup's documentation-opening code. Using two separate crates would defeat the purpose. It's been integrated into cargo, and seems to be working well.

However, a cargo issue has been opened where it seems we've decided we want to explicitly check the $BROWSER environment variable for the purpose of overriding the browser used to open the docs. This should probably be implemented before this pull request is merged.

@pickfire
Copy link
Contributor

pickfire commented Nov 4, 2018

@Seeker14491 Ah, I see that cargo already uses opener. There seems to be an issue to use $BROWSER in open since 2017 though. Byron/open-rs#8

I think both cargo and rustup uses way different crate, both are different projects, I don't see how it matters much to use different crate since the usage of rustup and cargo is very different.

Say, rustup uses rustup component add --toolchain nightly but cargo have cargo +nighty. As well, the frequency of usage between rustup differs quite a lot from cargo (it even have a fancy progress bar).

@nrc
Copy link
Member

nrc commented Jan 14, 2019

Closing for now due to lack of review bandwidth. I would like to share more code between Cargo and Rustup though.

@nrc nrc closed this Jan 14, 2019
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.

3 participants