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

Add experimental feature to link against local copy of std from cargo doc --open #7071

Closed
wants to merge 2 commits into from

Conversation

yaahc
Copy link
Member

@yaahc yaahc commented Jun 26, 2019

related to #6279

@rust-highfive
Copy link

r? @ehuss

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 26, 2019
@QuietMisdreavus
Copy link
Member

The interaction with rustdoc seems fine to me. I'll leave the proper review to Cargo-team people, though.

@ehuss
Copy link
Contributor

ehuss commented Jun 30, 2019

Thanks for helping to move this along! This isn't quite the direction I was thinking of though, and doesn't close 6279. I'd prefer to lay out a plan for how it will work overall before jumping into adding new flags.

There are a variety of cases that I'd like to see cargo cover for mapping dependencies:

  • Setting sysroot to local file (this PR).
    • How does this work with package managers? Do they install docs outside of sysroot?
  • Setting dependencies to the local target (the current behavior).
  • Setting dependencies to docs.rs.
    • How would that work with patched/replaced dependencies?
  • Setting dependencies to some other website.
  • Setting dependencies to some global shared folder.

I haven't thought about it much (hence the "needs design" part). I'm not sure if this is the kind of thing that should be defined in .cargo/config, or using command-line flags, or both.

@bors
Copy link
Contributor

bors commented Jul 26, 2019

☔ The latest upstream changes (presumably #7185) made this pull request unmergeable. Please resolve the merge conflicts.

@yaahc
Copy link
Member Author

yaahc commented Aug 8, 2019

@yaahc
Copy link
Member Author

yaahc commented Aug 9, 2019

  • Setting sysroot to local file (this PR).
    • How does this work with package managers? Do they install docs outside of sysroot?

Even with this part I don't feel like this PR fully addresses the problems. It makes crosslinking work but the two docs still act like otherwise unrelated webpages. I'd really like to see the search bar resolve items from both the locally generated crate / dependency docs and the std / core docs, not just have local items searchable but have clickable links to std/core items in the local doc pages.

I don't know how best to accomplish this though, particularly when you start tossing in other dependency mappings like to docs.rs. My expectation is that this would have to be accomplished via changes to rustdoc and even then I'm not sure all the information is there without having a local copy of std/core to look at. @QuietMisdreavus lmk what you think

  • Setting dependencies to the local target (the current behavior).

I don't think this is the current behavior, my understanding is that we always set std / core links to the nightly docs on rust-lang.org https://doc.rust-lang.org/nightly/

@QuietMisdreavus
Copy link
Member

I'd really like to see the search bar resolve items from both the locally generated crate / dependency docs and the std / core docs

I don't see a way to make this feasible without somehow embedding the search index from std and friends into everyone's search index, every time we ever generate docs. This would also severely clutter up the search index for regular crates who wouldn't want std to be searchable from their crate.

I could see it with a new flag to rustdoc, but making people add that manually seems like it would make it get less use. @rust-lang/rustdoc What do you think?

@GuillaumeGomez
Copy link
Member

Seems like a bad idea (to add into the search-index). However, linking to local std types seems like a nice thing.

@yaahc
Copy link
Member Author

yaahc commented Aug 9, 2019

@GuillaumeGomez when you say linking to local std types do you basically mean what's accomplished by this change? This already makes the std links in the docs point to the local copy of the std docs.

@QuietMisdreavus new flag sounds fine to me, I'm okay with it not being the default if most people wouldn't want it, who knows, maybe after using it I'll realize it was really a bad idea and not like the clutter either.

@GuillaumeGomez
Copy link
Member

@yaahc: Yep and I approve it.

@yaahc
Copy link
Member Author

yaahc commented Aug 9, 2019

ooh. cool 😅

@yaahc
Copy link
Member Author

yaahc commented Feb 24, 2020

I'm gonna close this because I doubt I'll be able to get around to this any time soon, but if someone else wants to come along and pick up where I left off they're more than welcome to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants