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 String documentation from Deref impl #32129

Closed
alexcrichton opened this issue Mar 8, 2016 · 15 comments · Fixed by #53052
Closed

Broken links in String documentation from Deref impl #32129

alexcrichton opened this issue Mar 8, 2016 · 15 comments · Fixed by #53052
Labels
C-bug Category: This is a bug. 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.

Comments

@alexcrichton
Copy link
Member

There are a bunch of links in the String documentation page which are broken. The reason for this is that most links are drawn from the documentation of the str type at /std/primitive.str.html but the String page is located at /std/string/struct.String.html. This mismatch in depth of paths causes a number of broken links, for example on the String::split method.

Not sure of the best way to fix this, but needs a tracking issue.

@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
@durka
Copy link
Contributor

durka commented Mar 8, 2016

It would be most awesome if there were some way to link to an item in Rustdoc, not just putting in a link URL, but that's perhaps out of scope for now. Maybe just some basic variable substitution, so you could write [foo]($htmlroot/bar/baz/foo.html)?

@alexcrichton
Copy link
Member Author

Yeah I suspect some basic substitution could work especially because we control the markdown rendering. I'd be a little wary to add API surface area to rustdoc, but it seems minorly well motivated?

@durka
Copy link
Contributor

durka commented Mar 9, 2016

It seems pretty much required to put in some facility for linking without relative URLs, because Deref copies and pastes things around.

@steveklabnik
Copy link
Member

It would be nice if we could follow the work CommonMark is doing here, so we're not totally just inventing a new thing.

http://talk.commonmark.org/t/generic-directives-plugins-syntax/444 seems to be at least one relevant discussion.

@mitaa
Copy link
Contributor

mitaa commented Mar 10, 2016

That seems like a nice long-term solution, that'd have to be implemented in hoedown however.

There is an old issue regarding this on their repo: hoedown/hoedown#99

@steveklabnik
Copy link
Member

yes, I would like to move from hoedown to pulldown-cmark, personally.

@frewsxcv
Copy link
Member

Opened an issue for moving to pulldown-cmark: #38400

@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

MaloJaffre commented Dec 13, 2017

This issue seems to block the testing of compiler docs: ref #46278

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.
kennytm added a commit to kennytm/rust that referenced this issue Aug 7, 2018
@ollie27
Copy link
Member

ollie27 commented Nov 7, 2018

This needs to be reopened. There are still broken links (e.g. #55666) and this issue can't be closed until the whitelist in linkchecker is removed:

// FIXME(#32129)
if file.ends_with("std/string/struct.String.html") ||
file.ends_with("interpret/struct.ImmTy.html") ||
file.ends_with("symbol/struct.InternedString.html") ||
file.ends_with("ast/struct.ThinVec.html") ||
file.ends_with("util/struct.ThinVec.html") ||
file.ends_with("util/struct.RcSlice.html") ||
file.ends_with("layout/struct.TyLayout.html") ||
file.ends_with("humantime/struct.Timestamp.html") ||
file.ends_with("log/index.html") ||
file.ends_with("ty/struct.Slice.html") ||
file.ends_with("ty/enum.Attributes.html") ||
file.ends_with("ty/struct.SymbolName.html") {
return None;
}

@Dylan-DPC-zz
Copy link

This is fixed, closing it. Thanks :)

@estebank
Copy link
Contributor

@Dylan-DPC I don't think it was (unless the nightly docs haven't been updated with the fix yet). In the String::split section, if you click on DoubleEndedIterator you end up in a 404.

@estebank estebank reopened this Dec 22, 2019
@ehuss
Copy link
Contributor

ehuss commented Dec 22, 2019

It might be useful to have a tracking issue for all of the causes of broken links (maybe reuse this one?), because there are dozens of broken links, and I think some are for different reasons. It'd be nice to have a goal to remove the linkchecker whitelist. I'd guess most of them are waiting for intra-rustdoc-links to handle all the appropriate scenarios? It's not really clear what needs to be done.

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 4, 2020

I think this was fixed in #74485? Should this be closed?

@ehuss
Copy link
Contributor

ehuss commented Nov 4, 2020

Considering that all the String links seem to be fixed, I think it is good to close. It looks like there are plenty of other tracking issues for the last remaining broken links (like #63351 and #74481), so I think those are adequately tracked. Thanks everyone for putting all the work into intra-doc-links!

@ehuss ehuss closed this as completed Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. 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.
Projects
None yet