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

Broken links in BTreeSet struct documentation #32130

Closed
alexcrichton opened this issue Mar 8, 2016 · 18 comments
Closed

Broken links in BTreeSet struct documentation #32130

alexcrichton opened this issue Mar 8, 2016 · 18 comments
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@alexcrichton
Copy link
Member

There are broken links when looking at the BTreeSet structure. This happens because the structure can be viewed from two locations, both at varying depths of links so the relative links may or may not work:

@alexcrichton alexcrichton added A-docs T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Mar 8, 2016
@steveklabnik steveklabnik added A-docs and removed A-docs labels Mar 9, 2016
@mitaa
Copy link
Contributor

mitaa commented Mar 28, 2016

The same problem also causes the link [module-level documentation](index.html#insert-and-complex-keys) on the insert methods documentation on BTreeSet, BTreeMap and HashMap to sometimes refer to the wrong module (the immediate parent, instead of std::collections).

@phungleson
Copy link
Contributor

@alexcrichton is there anything left to be done here, i can help the rest

@alexcrichton
Copy link
Member Author

@phungleson if you can delete these lines and have this command work:

./x.py test src/tools/linkchecker

Then there's no more work to do! (other than deleting that block)

@phungleson
Copy link
Contributor

Thanks @alexcrichton seems like I have to make some fixes

I got a lot of broken link after removing those lines lol

@phungleson
Copy link
Contributor

While I am here, I wonder if I should format the docs here https://github.com/rust-lang/rust/blob/bae454edc5e18e81b831baf4a7647bf2dda620a8/src/libcollections/btree/map.rs to fit in 80 columns.

Currently it is quite hard to read properly.

image

@phungleson
Copy link
Contributor

phungleson commented Feb 12, 2017

Sorry for my noob question, every time, I change something and run ./x.py test src/tools/linkchecker the whole rustc get re-complied again and it took at least 15' to 30' to run the thing.

I wonder if there is any less painful way to make changes and test in rust?

@steveklabnik
Copy link
Member

steveklabnik commented Feb 12, 2017 via email

@alexcrichton
Copy link
Member Author

@phungleson you may want to pass --stage 0 as well, --stage 2 is the default and goes all the way to the end of the compiler.

Alternatively if you're just changing documentation then after you modify a file you can just use touch to move its mtime back in time.

@phungleson
Copy link
Contributor

Hey thanks guys, --stage 0 is not super fast (5-10') but probably ok right now, have been looking into this for a while.

Wonder why struct.BTreeMap.html and similar files appear in 2 places? This is impossible to create correct relative links.

I think it is not fixed yet? and what is the correct place things should be?

Thanks

image

@alexcrichton
Copy link
Member Author

Oh yeah I don't think this is fixed yet, hence the bug :)

@phungleson
Copy link
Contributor

@alexcrichton which one should we keep, btree_map or btree/map and do you know where in the code that decides where the files should go?

I can dig a bit but might be faster if you know where it is.

Thanks!

@alexcrichton
Copy link
Member Author

Nah unfortunately the details here are pretty out of cache for me :(

@steveklabnik steveklabnik added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 18, 2017
@Mark-Simulacrum Mark-Simulacrum added the C-bug Category: This is a bug. label Jul 24, 2017
@MaloJaffre
Copy link
Contributor

This problem also occurs in compiler docs (now built in #46278), where rustc_data_structures::sync::Lrc and rustc_data_structures::sync::RwLock are aliases to std types.

bors added a commit that referenced this issue Jan 1, 2018
Add compiler docs testing to CI.

Fixes #47025.
I don't know if `x86_64-gnu` is the right builder for this, but there seems to be time left on [Travis](https://travis-ci.org/rust-lang/rust/jobs/307488864).

Remaining problems blocking this PR:
- [x] broken links caused by rustdoc issues:
  - [x] `pub use self::Enum::...`: #46766 and #46767 (fixed by #47050, thanks @ollie27!)
  - [x] `impl Deref for DerefToStdType`: #32129 (ignored in linkchecker)
  - [x] `#[feature(decl_macro)]` and `use std::vec`: #47038 (ignored in linkchecker)
  - [x]  `rustc_data_structures::sync::{Lrc, RwLock}` aliases `std` types: #32130 (ignored in linkchecker)
- [x] markdown differences, in rust repository and in external crates, now failing the build with #46880 merged (all fixed)
- [x] multiple crate updates needed: `rand`, `log`, `parking_lot_core`, `flate2`
  - [x] submodule updates needed to deduplicate dependencies: `rust-installer`, ~`cargo`~ (done by #47052)
  - [x] #44953 test broken by `log` update (removed, this can be controversial)
- [x] Waiting `x86_64-gnu` build results ([done](https://travis-ci.org/rust-lang/rust/builds/323451069))

See individual commits for more details.
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 18, 2020
Use intra-doc links in `str` and `BTreeSet`

Fixes rust-lang#32129, fixes  rust-lang#32130

A _slight_ degradation in quality is that the `#method.foo` links would previously link to the same page on `String`'s documentation, and now they will navigate to `str`. Not a big deal IMO, and we can also try to improve that.
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 18, 2020
Use intra-doc links in `str` and `BTreeSet`

Fixes rust-lang#32129, fixes  rust-lang#32130

A _slight_ degradation in quality is that the `#method.foo` links would previously link to the same page on `String`'s documentation, and now they will navigate to `str`. Not a big deal IMO, and we can also try to improve that.
@bors bors closed this as completed in 0d669a9 Jul 18, 2020
@ehuss
Copy link
Contributor

ehuss commented Jul 18, 2020

Reopening until the exclusions in the link checker can be removed.

@ehuss ehuss reopened this Jul 18, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 19, 2020
More intra-doc links, add explicit exception list to linkchecker

Fixes the broken links behind rust-lang#32553

Progress on rust-lang#32130 and rust-lang#32129 except for a small number of links. Instead of whitelisting entire files, I've changed the code to whitelist specific links in specific files, and added a comment requesting people explain the reasons they add exceptions. I'm not sure if we should close those issues in favor of the already filed intra-doc link issues.
@jyn514
Copy link
Member

jyn514 commented Nov 5, 2020

Reopening until the exclusions in the link checker can be removed.

I wrote this in a commit message no one will read, so pasting here for visibility:

BTreeMap wants to link to the docs in std::collections, which are very detailed, and not alloc::collections, which is almost empty. But alloc is a dependency of std, so it's not possible to link to it across crates (#74481). What we could do instead is have the same documentation in both alloc::collections and std::collections, so that BTree can link to the alloc version. However the docs for collections in turn link to HashMap, which are only in std. So one way or another, some of the links will be broken.

(I suggested a while ago making HashMap available in alloc, but there wasn't a clear consensus on how to do that: https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Why.20is.20HashMap.20in.20std.20and.20not.20alloc.3F)

@jyn514
Copy link
Member

jyn514 commented Apr 8, 2021

Triage: this is almost working, these are the last broken links left in the standard library:

const LINKCHECK_EXCEPTIONS: &[(&str, &[&str])] = &[
// These try to link to std::collections, but are defined in alloc
// https://github.com/rust-lang/rust/issues/74481
("std/collections/btree_map/struct.BTreeMap.html", &["#insert-and-complex-keys"]),
("std/collections/btree_set/struct.BTreeSet.html", &["#insert-and-complex-keys"]),
("alloc/collections/btree_map/struct.BTreeMap.html", &["#insert-and-complex-keys"]),
("alloc/collections/btree_set/struct.BTreeSet.html", &["#insert-and-complex-keys"]),
];

@jyn514
Copy link
Member

jyn514 commented Apr 8, 2021

I'm going to mark this as a standard library bug; the rustdoc half of it is tracked at #74481.

@jyn514 jyn514 added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 8, 2021
@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Oct 30, 2021
@Dylan-DPC
Copy link
Member

Closing this as the exclusions are tracked in #74481

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants