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

fix(rustdoc): always use a channel when linking to doc.rust-lang.org #134807

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

poliorcetics
Copy link
Contributor

Closes #131971

I manually checked the resulting links

One issue is that this will create nightly/... links in places that formerly linked to stable, is that ok ? (the slice and array links in the search help notably)

@rustbot
Copy link
Collaborator

rustbot commented Dec 26, 2024

r? @fmease

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Dec 26, 2024
@rustbot
Copy link
Collaborator

rustbot commented Dec 26, 2024

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. r=me with CI fixed (just tidy line-length errors)

@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 29, 2024
@camelid camelid assigned camelid and unassigned fmease Dec 29, 2024
@GuillaumeGomez
Copy link
Member

I'm not sure it's the right fix: currently it's only targeting the same channel whereas the issue is about the same version as rustdoc when the doc is generated.

@camelid
Copy link
Member

camelid commented Dec 29, 2024

I'm not sure it's the right fix: currently it's only targeting the same channel whereas the issue is about the same version as rustdoc when the doc is generated.

Sure, this is more of an existing limitation of DOC_RUST_LANG_ORG_CHANNEL though, right? We could just change it to use the 1.x.x version instead of stable to resolve your concern right?

@GuillaumeGomez
Copy link
Member

Not sure... I suppose it could work, but on nightly, the item we're targeting might not be there when the current version is release 12 weeks later. Tricky problem.

@camelid
Copy link
Member

camelid commented Dec 29, 2024

Not sure... I suppose it could work, but on nightly, the item we're targeting might not be there when the current version is release 12 weeks later. Tricky problem.

I'm a little confused what the issue is. I'm pretty sure this PR is a strict improvement over the status quo, since we will match the version in more cases than before. In any case, these are only for a few custom links to things that don't change that much, like the slice primitive. It's not going to be going away.

@GuillaumeGomez
Copy link
Member

Yeah, maybe you're right. But in any case, it doesn't solve the issue which wants the version and not the channel. I'm fine merging as is but it's not the fix asked in the issue.

@camelid
Copy link
Member

camelid commented Jan 5, 2025

@GuillaumeGomez why did the original PR introduce DOC_RUST_LANG_ORG_CHANNEL in the first place? Should we just get rid of it and always use the Rustdoc version (or beta/nightly if it's an unstable version) instead?

@GuillaumeGomez
Copy link
Member

I think using a version unless it's beta/nightly would be the correct fix.

@camelid
Copy link
Member

camelid commented Jan 9, 2025

so @poliorcetics do you want to redo the PR to accomplish that? Or rather it would be a change on top of what you've already done: replace DOC_RUST_LANG_ORG_CHANNEL with DOC_RUST_LANG_ORG_VERSION or something like that, where the part after the domain is nightly, beta, or the stable version number.

@poliorcetics
Copy link
Contributor Author

poliorcetics commented Jan 10, 2025

Isn't that already being done ?

pub fn doc_rust_lang_org_channel(&self) -> String {
let channel = match &*self.config.channel {
"stable" => &self.version,
"beta" => "beta",
"nightly" | "dev" => "nightly",
// custom build of rustdoc maybe? link to the latest stable docs just in case
_ => "stable",
};
format!("https://doc.rust-lang.org/{channel}")
}

@camelid
Copy link
Member

camelid commented Jan 10, 2025

@poliorcetics you're right, and I just confirmed by looking at the rendering for existing uses of the constant. Guillaume and I must've gotten confused (the name of the constant maybe could be improved 😅).

Can you fix the tidy errors so I can approve?

@GuillaumeGomez
Copy link
Member

And maybe the name of the constant too please? :3

@camelid
Copy link
Member

camelid commented Jan 11, 2025

Yeah, let's do DOC_RUST_LANG_ORG_VERSION.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@camelid
Copy link
Member

camelid commented Jan 30, 2025

@poliorcetics just a gentle ping. This is so close to being ready. Pretty sure you just need to fix the redundant escape sequences issue and then I can r+.

Copy link
Member

@camelid camelid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@camelid
Copy link
Member

camelid commented Feb 3, 2025

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 3, 2025

📌 Commit a063cf5 has been approved by camelid

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 3, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2025
… r=camelid

fix(rustdoc): always use a channel when linking to doc.rust-lang.org

Closes rust-lang#131971

I manually checked the resulting links

One issue is that this will create `nightly/...` links in places that formerly linked to stable, is that ok ? (the `slice` and `array` links in the search help notably)
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#134777 (Enable more tests on Windows)
 - rust-lang#134807 (fix(rustdoc): always use a channel when linking to doc.rust-lang.org)
 - rust-lang#135621 (Move some std tests to integration tests)
 - rust-lang#135695 (Support raw-dylib link kind on ELF)
 - rust-lang#135836 (bootstrap: only build `crt{begin,end}.o` when compiling to MUSL)
 - rust-lang#136235 (Pretty print pattern type values with transmute if they don't satisfy their pattern)
 - rust-lang#136392 (bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros)
 - rust-lang#136462 (mir_build: Simplify `lower_pattern_range_endpoint`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2025
… r=camelid

fix(rustdoc): always use a channel when linking to doc.rust-lang.org

Closes rust-lang#131971

I manually checked the resulting links

One issue is that this will create `nightly/...` links in places that formerly linked to stable, is that ok ? (the `slice` and `array` links in the search help notably)
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#134777 (Enable more tests on Windows)
 - rust-lang#134807 (fix(rustdoc): always use a channel when linking to doc.rust-lang.org)
 - rust-lang#135621 (Move some std tests to integration tests)
 - rust-lang#135836 (bootstrap: only build `crt{begin,end}.o` when compiling to MUSL)
 - rust-lang#136235 (Pretty print pattern type values with transmute if they don't satisfy their pattern)
 - rust-lang#136392 (bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros)
 - rust-lang#136462 (mir_build: Simplify `lower_pattern_range_endpoint`)

r? `@ghost`
`@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Feb 3, 2025
… r=camelid

fix(rustdoc): always use a channel when linking to doc.rust-lang.org

Closes rust-lang#131971

I manually checked the resulting links

One issue is that this will create `nightly/...` links in places that formerly linked to stable, is that ok ? (the `slice` and `array` links in the search help notably)
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2025
Rollup of 8 pull requests

Successful merges:

 - rust-lang#134807 (fix(rustdoc): always use a channel when linking to doc.rust-lang.org)
 - rust-lang#134814 (Add `kl` and `widekl` target features, and the feature gate)
 - rust-lang#135836 (bootstrap: only build `crt{begin,end}.o` when compiling to MUSL)
 - rust-lang#136022 (Port ui/simd tests to use the intrinsic macro)
 - rust-lang#136235 (Pretty print pattern type values with transmute if they don't satisfy their pattern)
 - rust-lang#136309 (set rustc dylib on manually constructed rustc command)
 - rust-lang#136392 (bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros)
 - rust-lang#136462 (mir_build: Simplify `lower_pattern_range_endpoint`)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2025
… r=camelid

fix(rustdoc): always use a channel when linking to doc.rust-lang.org

Closes rust-lang#131971

I manually checked the resulting links

One issue is that this will create `nightly/...` links in places that formerly linked to stable, is that ok ? (the `slice` and `array` links in the search help notably)
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2025
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#134807 (fix(rustdoc): always use a channel when linking to doc.rust-lang.org)
 - rust-lang#134814 (Add `kl` and `widekl` target features, and the feature gate)
 - rust-lang#135836 (bootstrap: only build `crt{begin,end}.o` when compiling to MUSL)
 - rust-lang#136022 (Port ui/simd tests to use the intrinsic macro)
 - rust-lang#136309 (set rustc dylib on manually constructed rustc command)
 - rust-lang#136392 (bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros)
 - rust-lang#136462 (mir_build: Simplify `lower_pattern_range_endpoint`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2025
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#134807 (fix(rustdoc): always use a channel when linking to doc.rust-lang.org)
 - rust-lang#134814 (Add `kl` and `widekl` target features, and the feature gate)
 - rust-lang#135836 (bootstrap: only build `crt{begin,end}.o` when compiling to MUSL)
 - rust-lang#136022 (Port ui/simd tests to use the intrinsic macro)
 - rust-lang#136309 (set rustc dylib on manually constructed rustc command)
 - rust-lang#136462 (mir_build: Simplify `lower_pattern_range_endpoint`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit f8f31fa into rust-lang:master Feb 4, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Feb 4, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2025
Rollup merge of rust-lang#134807 - poliorcetics:ab/push-skpynvsmwkll, r=camelid

fix(rustdoc): always use a channel when linking to doc.rust-lang.org

Closes rust-lang#131971

I manually checked the resulting links

One issue is that this will create `nightly/...` links in places that formerly linked to stable, is that ok ? (the `slice` and `array` links in the search help notably)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc should link to its current version of the rustdoc book
7 participants