-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Remove intra-doc link disambiguation prefix aliases #77277
Remove intra-doc link disambiguation prefix aliases #77277
Conversation
“Prim” is rustc jargon; everywhere else uses the normal English word “primitive”, including the file names like primitive.u8.html, which it’s nice to match. This only touches the `prim@` disambiguation prefix. rustc still refers to “prim” internally in some places, but being an outsider to rustc I’m unqualified to judge whether any change is called for there. (The uses are: PrimIntCast/prim-int-cast; PrimTy, short for PrimitiveType; and an identifier prim, most commonly of type PrimitiveType.)
Following on from what I think should be the fairly uncontroversial killing of the prim@ disambiguation prefix in favour of primitive@, we have a few more aliases that it may be worth evicting from rustdoc. - `function` is removed in favour of `fn`, which matches the keyword and the fn.*.html filenames. - `module` is removed in favour of `mod`, which matches the keyword. Modules become folders, so there’s no filename argument. I have kept two aliases at this time because I think more discussion is called for: - `method` is retained as an alias of `fn`; it matches the method.* fragment that methods are given. Well, unless they’re tymethod.*. Should tymethod therefore be allowed as an alias? As I say, discussion is called for. Also, subjectively people may prefer to call a thing a method than a function, though that argument could be applied to retaining the “function” and “module” spellings. - `constant` is retained as an alias of `const`; their filenames are currently of the form constant.*.html.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
r? @jyn514 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function
is removed in favour offn
, which matches the keywordmodule
is removed in favour ofmod
, which matches the keyword.
👍
constant
is retained as an alias ofconst
; their filenames are
currently of the form constant.*.html.
I don't think we need to be consistent with rustdoc file names, they're not really user-facing. const
seems fine to me (constant
was only added recently: #74430 (comment)). Actually it looks like it was never even documented.
method
is retained as an alias offn
; it matches the method.*
fragment that methods are given.
Hmm, I'm again not sure we should keep this ... Rust doesn't have methods, it has functions. The only thing that &self
does is let you use the self.f()
syntax in addition to normal Self::f(&self)
.
cc @rust-lang/rustdoc
"derive" => Kind(DefKind::Macro(MacroKind::Derive)), | ||
"type" => NS(Namespace::TypeNS), | ||
"value" => NS(Namespace::ValueNS), | ||
"macro" => NS(Namespace::MacroNS), | ||
"prim" | "primitive" => Primitive, | ||
"primitive" => Primitive, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also add a help message if someone tries to use the old names? I'd implement it something like this:
- Add a new
ResolutionError::UnsupportedDisambiguator
variant - Return the variant if someone tries to use the old names (and change the error type of the function to
ResolutionFailure
):
"primitive" => Primitive, | |
"primitive" => Primitive, | |
"module" | "function" | "prim" => return Err(ResolutionError::UnsupportedDisambiguator(prefix)), |
- Change
fn resolution_failure
to give a helpful error, something like
warning: unresolved link to `prim@char`
--> bad.rs:1:6
|
1 | //! [prim@char]
| ^^^^^^^^^ help: use `primitive` instead of `prim`: `primitive@char`
|
= note: the `prim` disambiguator has been removed, see #77277
There are examples in that function, but feel free to ask for help if you need it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can do this, but I’m a little surprised you’d suggest it: this is niche, previously-unstable functionality, and it wouldn’t surprise me if literally no one has used these aliases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with that reasoning is that people aren't using it as if it's unstable. module@
does not seem particularly niche to me and neither does function@
. Part of the reason I've been trying to stabilize intra-doc links is because people are treating it as if it's stable, because docs.rs uses the nightly compiler.
This is part of a larger discussion I want to have about rustdoc and nightly: rustdoc should be enforcing feature gates. Right now people don't know what's stable and what's unstable because very few people document locally, and those that do mostly use nightly and think everything must be fine on stable because they haven't added #![feature]
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the disambiguation prefixes, which I think it’s fair to say are niche; I had forgotten that intra-doc links as a whole were only now becoming stable!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had forgotten that intra-doc links as a whole were only now becoming stable!
Right, this is what I mean by rustdoc should enforce feature gates. See also #63305.
I meant the disambiguation prefixes, which I think it’s fair to say are niche
Well, either way I don't think we should remove functionality without pointing to how to fix this code. So I'd strongly prefer to add this help message.
I'm curious why you think so, in my mind they're entirely separate. |
RFC 2988 is relevant because I like having the prefixes match the filenames (especially when it’s hijacking link hrefs), which is also why I haven’t pulled But really, I just like consistency. You say rustdoc URLs aren’t user-facing, but I strongly disagree—and in fact I’d say that a large part of the purpose of RFC 2988 is that they are user-facing to at least some degree, even if you think that the precise spelling of the URLs shouldn’t be. But I manually type rustdoc URLs moderately regularly (more regularly than I would need to if I arranged better tooling for myself!). |
I do not think we should remove I think removing There is no cost to having these extra, they just make it less likely for people to go "what was the disambiguator again?". |
I don't have strong opinions either way on this, I'll defer to @Manishearth's judgement. |
☔ The latest upstream changes (presumably #76325) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:
|
This is a breaking change though since intra-doc links are stable on beta, right? |
We're allowed to break beta, but yes, if this wants to land it should be soon. I am against removing things other than |
@chris-morgan any updates on this? |
Intra-doc links will be stable in a week, so I don't think it makes sense to change this at this point. |
(That title is gnarly!)
Follow-up to #75815 (comment).
Read the commit messages for details and remaining questions about the
method
andconstant
aliases. rust-lang/rfcs#2988 is relevant to this consideration./cc @jyn514