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

Struct.field should resolve for intra-doc-links #75437

Closed
jyn514 opened this issue Aug 12, 2020 · 10 comments
Closed

Struct.field should resolve for intra-doc-links #75437

jyn514 opened this issue Aug 12, 2020 · 10 comments
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-feature-request Category: A feature request, i.e: not implemented / a PR. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jyn514
Copy link
Member

jyn514 commented Aug 12, 2020

Found in #75432 (review). std::process::Child.stdout should resolve to the struct field, but instead gives a resolution error - you have to use Child::stdout instead. This is a break from the original RFC: https://github.com/rust-lang/rfcs/blob/68e17ace829e81239fe5020b7a6dbc22550b6d16/text/1946-intra-rustdoc-links.md#linking-to-fields

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name labels Aug 12, 2020
@jyn514
Copy link
Member Author

jyn514 commented Aug 12, 2020

Relevant code:

// Try looking for methods and associated items.


We're treating enum variants and struct fields the same.

@Manishearth
Copy link
Member

I'm actually kinda iffy on this, I prefer the current behavior and we get to avoid clashes with foo.com.

@jyn514
Copy link
Member Author

jyn514 commented Aug 12, 2020

Hmm, I think struct.field is more clear though since it's how you'd use it in rust code. But fortunately this only adds new behavior so it doesn't block stabilization, we can always choose to add it later.

@jyn514 jyn514 added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Aug 12, 2020
@Manishearth
Copy link
Member

Yeah I'm just worried about the possibilities of ambiguity. It was listed as an optional extension in the rfc and iirc this was part of why.

@GuillaumeGomez
Copy link
Member

I'm not a big fan of this syntax but it's not a strong position either...

@camelid
Copy link
Member

camelid commented Aug 14, 2020

struct.field is less surprising which is good, but I definitely understand that it's difficult given the potential for clashes (especially with non-.com, .org, .net domains, e.g. .name). Maybe rustdoc could assume that it's an intra-doc link, and if it doesn't resolve then link to the equivalent domain? Unfortunately there's potential for minor breaking changes, although the Rust style is to have uppercase struct names and much of the time people link to lowercase domains, so maybe it wouldn't be too much of a clash.

@jyn514
Copy link
Member Author

jyn514 commented Aug 14, 2020

@camelid the concern I think is that there's no way to detect the ambiguity without reaching out and making network requests, which no one wants. So there's the potential that rustdoc could silently link to something other than what you intended, which is against the spirit of intra-doc links.

@jyn514
Copy link
Member Author

jyn514 commented Aug 14, 2020

and as a corrollary, rustdoc couldn't warn if the link failed to resolve, because it would have to assume it was a network link.

@camelid
Copy link
Member

camelid commented Aug 14, 2020

Yeah, it's too bad that there's not a solution that doesn't involve ambiguity and isn't surprising :(

@jyn514 jyn514 added C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Aug 21, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 17, 2020

I think changes in this area should be a change to the intra-doc links RFC, not an implementation issue, so I'm going to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-feature-request Category: A feature request, i.e: not implemented / a PR. 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

4 participants