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

Use name.namespace.html as the canonical URL, not kind.name.html #55160

Open
scottmcm opened this issue Oct 17, 2018 · 32 comments
Open

Use name.namespace.html as the canonical URL, not kind.name.html #55160

scottmcm opened this issue Oct 17, 2018 · 32 comments
Labels
A-stability Area: `#[stable]`, `#[unstable]` etc. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@scottmcm
Copy link
Member

I just wanted to look at the ManuallyDrop docs again, and my browser helpfully remembered that it's at https://doc.rust-lang.org/std/mem/union.ManuallyDrop.html. But, despite the type being stable, that page no longer exists; it's now https://doc.rust-lang.org/std/mem/struct.ManuallyDrop.html.

Is the specific item kind actually important to have in the path? The namespace (type vs value vs ...) certainly is, but especially for things without public fields, making the struct-vs-union distinction so front-and-centre seems overall unhelpful.

@Havvy Havvy added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Oct 17, 2018
@GuillaumeGomez
Copy link
Member

For me it's a positive thing. I'd argue that you'd better not use direct URLs and just search or rustdoc links. The current URLs are clear and represent what they should.

@mikeyhew
Copy link
Contributor

@GuillaumeGomez I disagree with that, I think that direct links to documentation are useful, their use should be encouraged, and we should avoid breaking them if possible.

Having the extra information of struct, enum or type in the URL is kind of nice too, but the other thing is that switching from the current URLs to new ones would result in a lot of links breaking, so maybe there is some way to use redirects instead? Like if you went to https://doc.rust-lang.org/std/mem/union.ManuallyDrop.html or https://doc.rust-lang.org/std/mem/type.ManuallyDrop.html it would redirect you to https://doc.rust-lang.org/std/mem/struct.ManuallyDrop.html.

@GuillaumeGomez
Copy link
Member

I'm not sure we understood each other. To be clear: the current state of rustdoc URLs seems good and should be kept unless something way better is proposed.

@RalfJung
Copy link
Member

I agree with the OP that encoding the "kind" of type in the URL seems suboptimal. Rust has three namespaces (macro-level, fn/variable-level, type-level), so that needs to be encoded, but I see no good reason to encode more. type.ManuallyDrophtml has all the information needed to uniquely identify this type.

As another argument for why the current URLs are bad: this is one of the few cases where doing a semver-compatible change (changing a struct to an enum, for example) breaks docs links.

@nox
Copy link
Contributor

nox commented Sep 14, 2019

Changing a struct to an enum isn't backwards-compatible (one can match against Struct { .. }), but I support removing the type kind from URLs, for the same rationale as in #64454.

@bluss
Copy link
Member

bluss commented Sep 14, 2019

We have a bunch of redirects that only focus on the namespace an item belongs to.

For example the URL https://doc.rust-lang.org/stable/std/mem/ManuallyDrop.t.html with "t" for type, will redirect to the correct place and I assume this URL was there before the struct/union switch happened. Only problem is that it's not the canonical URL. (And, it would be more ideal if we didn't have lots of extra redirect files, there are enough small files already in a rustdoc site).

@RalfJung
Copy link
Member

@bluss wow, I had no idea these files even existed!

For use in rustc/libstd, if they should be useful we need a tidy pass that tells people to link to the "t" file instead of the "struct"/"enum"/"union" file.

@GuillaumeGomez
Copy link
Member

It was for RLS iirc. Not sure if removing it is fine but I'd like to remove it.

@jyn514
Copy link
Member

jyn514 commented Aug 25, 2020

I disagree with that, I think that direct links to documentation are useful, their use should be encouraged, and we should avoid breaking them if possible.

I have the same comment as in #71912 (comment): direct links should use intra-doc links, the links rustdoc generates are not stable.

@RalfJung
Copy link
Member

@jyn514 so say I am writing a blog post and mentioning some Rust types whose docs I want to reference. Obviously my site generator does not support intra-doc links. What do you propose I do?

This is something people already do -- anything that has a URL has people link to it. It's what makes the web so amazing. Please don't break that.

@GuillaumeGomez
Copy link
Member

URLs are stable, the DOM content isn't. However, since you always use a specific version of a crate, you can just link to the element of this specific version of the crate on docs.rs.

@RalfJung
Copy link
Member

URLs are not stable when a struct becomes an enum (which otherwise is semver-compatible if the struct had a private field).

@GuillaumeGomez
Copy link
Member

Then that's on the crate owner, not on rustdoc. We the URLs are defined as [type].[type name].html. If you change an item's type, then you change the URL. It's predictable. But again: always reference to a specific version of a crate and that shouldn't be an issue.

@RalfJung
Copy link
Member

The entire point of this issue is that there is no good reason to include struct vs union in the URL. The "item type" of all structs, unions and enums should be just type.

It's predictable, sure, but that does not make it sensible.

@GuillaumeGomez
Copy link
Member

So you would want to include a breaking change for something stable even before 1.0? You'd need very strong reasons for it to be done and I'm really not convinced...

@RalfJung
Copy link
Member

No we don't need a breaking change -- as @bluss wrote above, reasonably stable URLs already exist: https://doc.rust-lang.org/stable/std/mem/ManuallyDrop.t.html
But nobody knows about them because they are not used by default. So all it would take is swap the role of ManuallyDrop.t.html and struct.ManuallyDrop.html, making the latter a redirect and the former the target of all the automatically generated links.

@GuillaumeGomez
Copy link
Member

Well, we kinda want to remove this URL format since a while. It's still there because it's being used by RLS iirc. Also, this would be a regression in my point of view considering you lose information. And since it's not the default URL format, it's definitely a breaking change.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

So you would want to include a breaking change for something stable

The point of discussion IMO is whether or not it is stable. Currently, it's stable with respect to rustdoc versions, but not with respect to the code being documented. Either

a) The file names should be predictable across versions being documented, in which case we should recommend ManuallyDrop.t.html, or
b) The file names are unstable and no one should depend on them, in which case we should change them every release so people don't depend on them by accident.

The current middle ground is sort of the worst of both worlds because it looks like it's stable, but can break for changes that do not break semver.

@RalfJung
Copy link
Member

RalfJung commented Aug 26, 2020

The information you lose is not semver-stable and irrelevant for API consumers. It's like having the line number in the URL -- sure that would be more inforamtion, but it's not good information to have in the URL. That's what this issue is all about -- please consult the arguments given above.

Also, how is this a breaking change if it doesn't break anything? All existing URLs keep working.

@RalfJung
Copy link
Member

RalfJung commented Aug 26, 2020

IMO the question is quite simple at a fundamental level -- what does it take to uniquely identify an item in Rust (assuming you already know the crate it is in)? You need its name, its full module path, and its "kind" -- which can be value, type, or macro. And that's it. So this information should be sufficient to construct a URL, and no other information should be contained in the URL. After all, a "Uniform Resource Locator" is all about being able to locate resources, such as the documentation about items in Rust, in a uniform and consistent way.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

@RalfJung what do you propose we do about all the existing URLs that point to enum.ManuallyUninit.html? Should we generate redirects for all pages in the type namespace? That seems like it would bloat the size of the docs quite a bit. But if we don't do that, then the URLs are still broken even if we make union.ManuallyUninit.html a redirect.

@jyn514 jyn514 added the A-stability Area: `#[stable]`, `#[unstable]` etc. label Aug 26, 2020
@RalfJung
Copy link
Member

If my understanding is correct, currently we generate two pages for every type:

  • [enum|struct|union].$name.html is the main page with all the fields and methods and stuff
  • $name.t.html redirects to the above.

What I am proposing is to swap the role of these two files, so every existing link keeps working:

  • $name.t.html is the main page with all the fields and methods and stuff
  • [enum|struct|union].$name.html redirects to the above.

This cannot break any link that currently works. It means changing e.g. union to enum still breaks people that use the "wrong" (deprecated) union.$name.html, but that's certainly better than not having the union.$name.html redirect! Currently we break all links on a union -> enum change; with the new scheme we only break old links -- but new links that people set will go to the .t.html page. So, I'd say this is a strict improvement over the status quo.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

I'm happy with that. It doesn't fix the existing links to union.ManuallyDrop.html, but it encourages good practice for future links without being too much of a burden on rustdoc.

@RalfJung
Copy link
Member

(I am not sure what ManuallyInit is. ManuallyDrop changed from union to struct, so if you mean the existing links to union.ManuallyDrop.html then I agree.)

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

I was actually thinking of MaybeUninit 😆 so not even close

@RalfJung
Copy link
Member

MaybeUninit is a union and always was, so I don't think it is relevant here.^^

@jyn514 jyn514 changed the title RustDoc: Including "struct" and "union" in URLs makes them fragile Use name.namespace.html as the canonical URL, not kind.name.html Aug 26, 2020
@ollie27
Copy link
Member

ollie27 commented Aug 26, 2020

The namespace based redirects were removed in #70563.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

It looks like that was mostly to fix wonky hash handling? The only comment about namespaces I see is #70563 (comment):

We should also fully revert #35236 not just part of it.

Is there any reason not to add the namespace redirects back?

@GuillaumeGomez
Copy link
Member

Why would we want it? It's a duplication of the current system. We removed it to stop having to maintain it and therefore reduce the amount of code.

@jyn514
Copy link
Member

jyn514 commented Aug 26, 2020

The entire issue is giving reasons why we would want it. #55160 (comment) has a good summary.

@RalfJung
Copy link
Member

Why would we want it?

Please read this issue.

@Kixunil
Copy link
Contributor

Kixunil commented Jan 20, 2022

@nox turns out you're right, I'm glad I double-checked it. I think this is a mistake in the language design and should be deprecated. It's very impractical and I was unknowingly relying on it not being the case.

The comment I originally intended to write, still relevant if the incompatibility gets fixed:

Somewhat less related but IMO if the type has no public fields people shouldn't even know what "kind" the type is. Whether it's a tuple struct or ordinary struct or an enum with private variants, if Rust ever gets those, should be completely irrelevant to the consumer because it has no observable effects. This information shouldn't be present in URL or docs or anywhere outside the source code. It's exactly like the analogy with having line numbers in the docs.

Edit: opened a discussion about this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-stability Area: `#[stable]`, `#[unstable]` etc. 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

10 participants