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

rustdoc: remove macro redirects #35705

Closed
nrc opened this issue Aug 16, 2016 · 6 comments
Closed

rustdoc: remove macro redirects #35705

nrc opened this issue Aug 16, 2016 · 6 comments
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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

@nrc
Copy link
Member

nrc commented Aug 16, 2016

PR #35234 changed macro URLs to remove the !. It left redirects in so as not to break existing URLs. At some point, we should remove these redirects so as not to accumulate too much cruft. Obviously this will require some message before we do so.

See FIXME in render.rs

@nrc nrc added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Aug 16, 2016
@ollie27
Copy link
Member

ollie27 commented Aug 16, 2016

Personally I don't see much reason to ever remove the redirects. For example I would expect https://doc.rust-lang.org/std/macro.assert!.html to continue to work basically forever,

@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-cleanup Category: PRs that clean code up or issues documenting cleanup. label Jul 25, 2017
@Mark-Simulacrum
Copy link
Member

Nominating for discussion. It's been almost two years; I think we should consider this issue again.

@QuietMisdreavus
Copy link
Member

@rust-lang/rustdoc, what do you think?

I personally don't see the harm in keeping it. Do we have crates with many more macros than the standard library, where having these redirect pages is a problem? I'm with @ollie27, keeping the link alive is more important to me than removing these small files from the output.

@GuillaumeGomez
Copy link
Member

I don't know why it'd cause issues so ok for me.

@QuietMisdreavus
Copy link
Member

Based on the rousing discussion here, I think the consensus can be described as "it's okay to keep them around". Before i close this issue, though, we should remove the FIXME that it refers to:

// If the item is a macro, redirect from the old macro URL (with !)
// to the new one (without).
// FIXME(#35705) remove this redirect.
if item_type == ItemType::Macro {
let redir_name = format!("{}.{}!.html", item_type, name);
let redir_dst = self.dst.join(redir_name);
let redirect_out = try_err!(File::create(&redir_dst), &redir_dst);
let mut redirect_out = BufWriter::new(redirect_out);
try_err!(layout::redirect(&mut redirect_out, file_name), &redir_dst);
}

Just to keep things neat.

QuietMisdreavus added a commit to QuietMisdreavus/rust that referenced this issue Jul 18, 2018
Based on the discussion in rust-lang#35705, the rustdoc team has determined that macro redirects are here to stay.

Closes rust-lang#35705
@QuietMisdreavus
Copy link
Member

A PR to remove that comment is live at #52510.

kennytm added a commit to kennytm/rust that referenced this issue Jul 18, 2018
… r=nrc

rustdoc: remove FIXME about macro redirects

Based on the discussion in rust-lang#35705, the rustdoc team has determined that macro redirects are here to stay.

Closes rust-lang#35705
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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
Development

No branches or pull requests

6 participants