rustdoc: simplify URLs #35020
rustdoc: simplify URLs #35020
Conversation
|
Nice work! However he'll have to do a huuuuuuuuuuuuuuuuuge update in doc urls. Poor us. :'( |
|
A few questions:
Overall, 👍 |
And what happens if it's a module? :-/ |
|
@GuillaumeGomez a module is kind of a special case in that it gets |
|
@GuillaumeGomez also, there are redirects in place, so you the update shouldn't be too bad |
|
1: yes |
|
Awesome, thanks @nrc! Some thoughts of mine:
Overall I very much like the direction this is going! |
|
The Travis test is probably spurious, apparently. |
|
|
@nrc the linkchecker lives in Also yeah I wasn't thinking of eliding namespaces aggressively, we'd still always have |
|
A few problems I've noticed so far:
pub const v: u32 = 0;
pub fn constant() {}
|
|
re the case sensitivity issue, what does Rustdoc do at the moment? E.g., if there are two structs in the same module called |
|
Yeah it just silently fails: #25879. This change does make that problem a bit worse of course. |
|
@alexcrichton what do you think about "v/t" vs "value/type" ? |
|
@nrc I prefer "v/t" as it's not something we really want to draw attention to in the URL, but I don't feel too strongly |
|
They still need a guess, I don't think it becomes easier, it just becomes more confusing ( The ideal way is to make |
It does. The compiler knows the namespace of every name, so any tool which uses the compiler does too. In fact the namespace is part of the language so any tool which wants to correctly interpret a Rust program must know the namespace of a name. Rustdoc's categorisation on the other hand is totally ad hoc - the compiler has no concept of the distinctions, any tool must basically repeat Rustdoc's analysis to work out which category an item belongs too, which means you must have access to the definition of the item, as well as the use (c.f., the namespace which is inferable from the context of the use). |
|
If this is only for tools then what about one of the following less disruptive alternatives:
|
|
☔ The latest upstream changes (presumably #35086) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Has anyone considered always combining the various namespaces into one page to clean up the URL? |
|
@eddyb: That sounds a bit problematic however (but I really love the idea!). It means rewriting the current architecture and increase folders number (after all each '/' is going inside a directory except if you added rewrite rules or wrote the server program). |
|
I think we should continue to improve via @alexcrichton's or @eddyb's proposals, but as a follow-up. This PR reduces the number of options for a name from 10-ish to 3, in the future we can hopefully reduce that to 1, but for now this actually solves painful problems. |
|
@GuillaumeGomez |
|
@nrc The danger is that if this gets into stable, it will have permanent effects. |
|
@eddyb nothing that can't be fixed with a redirect, right? |
|
@nrc Sure, it just means that you either keep the redirects there forever or you end up with links from an interim period. At least rewrite rules can handle both status quo -> this PR and this PR -> my proposal. |
|
@ollie27 why do you think this PR is particularly disruptive. Nearly all existing URLs will continue to work, but you get better URLs too. Seems like a win to me. To be clear, I hate the current URL scheme, I see no reason not to change it other than backwards compatability, which I believe this PR addresses (or will address when the bugs are rolled out). |
|
It's disruptive because people will need to learn a new scheme. I also don't think The current scheme is far from perfect but we can't keep changing it and creating redirects. We'll have to support them basically forever and they can create file name conflicts like I've pointed out unless we're very careful. Adding over 2,000 files to the Do either of my suggestions satisfy your use case? |
Also, the new URLs reflect naming as it actually is in Rust, this is how the language is defined (or would be if we had a proper definition) and how the compiler classifies names. The current URLs are totally arbitrary, just some classification that only Rustdoc knows about and isn't defined anywhere, isn't documented, and isn't used by anything else. |
| // page. If a page exists already, do not overwrite it. We | ||
| // don't want to replace a real rustdoc page with a redirect. | ||
| // In particular, the redirect for a primitive module would | ||
| // overwrite the doc page for the primitive itself. |
There was a problem hiding this comment.
If these redirects don't necessarily exist, is there much point in them existing at all?
The problem with primitive types is not only with module redirects. The following documents fine currently but doesn't with this PR.
pub struct bool;
#[doc(primitive = "bool")]
mod _bool {}I think it might make sense to put primitives in their own "namespace" because they're special anyway so presumably can't be treated like other types anyway.
There was a problem hiding this comment.
If these redirects don't necessarily exist, is there much point in them existing at all?
I would have thought that 2% of links breaking is better than 100% of links breaking.
The following documents fine currently but doesn't with this PR.
This seems to be a totally hypothetical question - is there any reason for anyone to actually write this?
I think it might make sense to put primitives in their own "namespace" because they're special
I thought about this, but it kind of ruins the whole idea - you can't just look up a type, you need to know if that type is a primitive or not - how do you know that? There is no way to tell, you need to look at a magic list that will go out of date as soon as we add i128 or whatever.
Furthermore, they're not special, they're only special to Rustdoc, which has an ugly hack to support documenting them, that should be an implementation detail, not exposed in the URLs.
|
People still have to synthesise the URLs when writing documentation. Neither of my suggestions were to not change the URLs, my point was that we don't need to change the canonical URLs if the only reason is so that tools can generate links. In fact both of the suggestions make it easier to change them later on without breaking tools perhaps to one of the nicer looking suggestions above. The old URLs have to be kept around for backwards compatibility anyway so the maintenance burden of them is exactly the same as my suggestions. I think a change like this deserves a proper RFC. It's essentially adding a new public API which will need to be supported. We'll also need a document outlining the new scheme in detail which is required if it's supposed to be synthesisable. A problem with the current URLs is that the same page can have multiple methods with the same name on the same page. Currently they get different ids e.g. primitive.bool.html#method.fmt and primitive.bool.html#method.fmt-1 but these can't be guessed nor are they stable (if you change their order in the source code the ids change). This change actually makes the problem worse by adding struct fields and enum variants to the same namespace so now for example Other issues with the current PR:
I don't know if you're aware of it but #34023 is an issue that I think should be fixed with a new URL scheme. Also cc. #25226 |
|
/cc @rust-lang/tools |
|
I am sympathetic to the notion that the current scheme is easier for humans to understand than the proposed. People think about structs and enums, not the type and value namespaces. I'd imagine that a large portion of Rust programmers don't even know what a namespace is in Rust. Some of the other critiques from @ollie27 I'm not so up to speed on, but I appreciate that they are thinking hard about this. |
This was unintentional, but I liked the change so I left it. The Thanks for the other points, I'll look at getting them fixed. |
|
@nrc what do you think of taking a perhaps more conservative route for now along the lines of what @ollie27 mentioned where there's a page which has JS to hyperlink everywhere else. That way we could leave URLs as they are today to perhaps have an RFC to get improved, but tools could immediately benefit by having much more machine-friendly generated links? |
|
@alexcrichton I think this is kind of a bad idea - we're adding a redirect and thus slowing things down for users, whilst preserving a crappy URL scheme for eternity (and adding a bunch of annoying code, which probably won't be kept up to date, rather than improving existing code). The current URLs are bad - they are exposing Rustdoc's internals in a misguided attempt at being easy to use, rather than doing things properly. We're locking down implementation decisions for Rustdoc with them since they tie us to current architectural decisions which should be purely implementation details rather than exposed in the interface. Writing an RFC for this seems like overkill, it will get bogged down in endless bike-shedding and we'll probably end up with something that no-one will ever implement. |
|
Hm I don't think we can objectively say that the current url scheme is "bad" because from an end-user perspective I agree with @ollie27 and @brson that I also don't think we need to preserve the current scheme for eternity, we just need a transition plan no matter what. I'd imagine that regardless of what we do we'll end up removing this scheme after a few months or a year. The current URLs I agree though are not great for machines to generate links to, but @ollie27's suggestion should solve that problem (easy to generate links), and we're not really worried about browsing speed when looking at these docs, it's not like you're downloading megabytes of JS to render a page. It seems like this is contentious enough that we at least want to have a broader discussion about what rustdoc URLs ideally should be. @ollie27 has a good point that changing it twice may be a good point, so if we're gonna change it we may as well choose a scheme that we all agree on at the current point in time. In the meantime though we can fix the machine-generated link problem in a relatively non-intrusive fashion. I agree though that an RFC probably isn't worth it, maybe just a discuss thread to get feedback. |
|
The trouble with the more conservative route is that it is easy to translate from the old URLs to the new ones, but hard to translate the new ones to the old, because there is missing context. To do so, the redirect page has to download a mapping from every single item (including nested items like fields and methods) to rustdoc type. That would be a huge file and thus a slow redirect (and of course it changes, so the client can't cache it, at least not easily). Forcing tools to download that mapping is a massive headache (How often should they do it? What if they don't have internet access? How to manage the memory?) I don't want to do that for IDEs, and no-one else will do it for any other tool, so you are pretty much guaranteeing that no tool will ever link to Rustdoc. |
|
I guess I can just add redirects for every one of the new URLs |
pulled out of rust-lang#35020
|
Closing in favour of #35236 (sadly). |
… ones cc rust-lang#35020 which does this properly
pulled out of rust-lang#35020
… ones cc rust-lang#35020 which does this properly
rustdoc: redirect URLs cc #35020 which does this properly r? @alexcrichton
rustdoc: remove the `!` from macro URLs and titles Because the `!` is part of a macro use, not the macro's name. E.g., you write `macro_rules! foo` not `macro_rules! foo!`, also `#[macro_import(foo)]`. (Pulled out of rust-lang#35020).
rustdoc: remove the `!` from macro URLs and titles Because the `!` is part of a macro use, not the macro's name. E.g., you write `macro_rules! foo` not `macro_rules! foo!`, also `#[macro_import(foo)]`. (Pulled out of rust-lang#35020).
Changes rustdoc URLs to name.namespace.html, e.g., Foo.t.html. These are easier for clients to guess since they only need to know the namespace, not the kind of item.
Old URLs are preserved as redirects to the new ones.
I also add redirects for modules, e.g.,
foo/bar/baz.t.html to foo/bar/baz/index.html, so modules are not an exception to the URL rule.
And removes the ! from macro URLs.
Closes #34271
r? @alexcrichton
cc @steveklabnik