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.

Sign up for GitHub

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

Jump to bottom

Don't report disambiguator error if link would have been ignored #83865

Merged
merged 1 commit into from
Apr 5, 2021
Merged

Don't report disambiguator error if link would have been ignored #83865

merged 1 commit into from
Apr 5, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Apr 4, 2021

Fixes #83859.

This prevents us from warning on links such as <hello@example.com>.
Note that we still warn on links such as <hello@localhost> because
they have no dots in them. However, the links will still work, even
though a warning is reported.

r? @jyn514

@camelid camelid added A-diagnostics Area: Messages for errors, warnings, and lints A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 4, 2021
@rust-highfive
Copy link
Collaborator

Some changes occurred in intra-doc-links.

cc @jyn514

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 4, 2021
@camelid camelid added the A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. label Apr 4, 2021
@jyn514 jyn514 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2021
@camelid
Don't report disambiguator error if link would have been ignored
45ccd50 
This prevents us from warning on links such as `<hello@example.com>`.
Note that we still warn on links such as `<hello@localhost>` because
they have no dots in them. However, the links will still work, even
though a warning is reported.
@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 4, 2021
@jyn514
Copy link
Member

jyn514 commented Apr 4, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Apr 4, 2021

📌 Commit 45ccd50 has been approved by jyn514

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2021
@jyn514 jyn514 mentioned this pull request Apr 4, 2021
Closed
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 5, 2021
@Dylan-DPC
Rollup merge of rust-lang#83865 - camelid:disamb-err-fix, r=jyn514
32f3dac 
Don't report disambiguator error if link would have been ignored

Fixes rust-lang#83859.

This prevents us from warning on links such as `<hello@example.com>`.
Note that we still warn on links such as `<hello@localhost>` because
they have no dots in them. However, the links will still work, even
though a warning is reported.

r? ``@jyn514``
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 5, 2021
@Dylan-DPC
Rollup merge of rust-lang#83865 - camelid:disamb-err-fix, r=jyn514
1069434 
Don't report disambiguator error if link would have been ignored

Fixes rust-lang#83859.

This prevents us from warning on links such as `<hello@example.com>`.
Note that we still warn on links such as `<hello@localhost>` because
they have no dots in them. However, the links will still work, even
though a warning is reported.

r? `@jyn514`
This was referenced Apr 5, 2021
Closed
Closed
jyn514 pushed a commit to jyn514/rust that referenced this pull request Apr 5, 2021
Rollup merge of rust-lang#83865 - camelid:disamb-err-fix, r=jyn514
baaa85a 
Don't report disambiguator error if link would have been ignored

Fixes rust-lang#83859.

This prevents us from warning on links such as `<hello@example.com>`.
Note that we still warn on links such as `<hello@localhost>` because
they have no dots in them. However, the links will still work, even
though a warning is reported.

r? ```@jyn514```
@jyn514 jyn514 mentioned this pull request Apr 5, 2021
Closed
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Apr 5, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2021
@bors
Auto merge of rust-lang#83880 - Dylan-DPC:rollup-hz9ph0e, r=Dylan-DPC
39eee17 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#81922 (Let `#[allow(unstable_name_collisions)]` work for things other than function)
 - rust-lang#82483 (Use FromStr trait for number option parsing)
 - rust-lang#82739 (Use the beta compiler for building bootstrap tools when `download-rustc` is set)
 - rust-lang#83650 (Update Source Serif to release 4.004)
 - rust-lang#83826 (List trait impls before deref methods in doc's sidebar)
 - rust-lang#83831 (Add `#[inline]` to IpAddr methods)
 - rust-lang#83863 (Render destructured struct function param names as underscore)
 - rust-lang#83865 (Don't report disambiguator error if link would have been ignored)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3ca197e into rust-lang:master Apr 5, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 5, 2021
@camelid camelid deleted the disamb-err-fix branch April 6, 2021 02:05
@petrochenkov
Copy link
Contributor

@camelid @jyn514
How is <hello@localhost> a documentation link?
I assumed that doc links must include square brackets, as the documentation at https://doc.rust-lang.org/rustdoc/linking-to-items-by-name.html says.
The main optimization if #94857 relies on this.

What other syntax doc links may have?

@jyn514
Copy link
Member

jyn514 commented Apr 15, 2022

Doclinks can have basically arbitrary syntax - they can be arbitrary markdown or html. The documentation at https://doc.rust-lang.org/rustdoc/linking-to-items-by-name.html describes what paths you can use inside a markdown link context, but does not describe the syntax for a markdown link itself. There's a reference here: https://daringfireball.net/projects/markdown/syntax#link
and automatic links are described here: https://daringfireball.net/projects/markdown/syntax#autolink

@jyn514
Copy link
Member

jyn514 commented Apr 15, 2022

@petrochenkov but yes you are correct, in this case it is not a intra-doc link because hello is not a valid disambiguator.

@jyn514
Copy link
Member

jyn514 commented Apr 15, 2022

I think a conservative estimate is link.contains(&['[', '<']). Without <, it can't be an automatic or HTML link, and without [ it can't be a shortcut or reference link.

@petrochenkov
Copy link
Contributor

a conservative estimate is link.contains(&['[', '<'])

Ok, that's also good enough.

@petrochenkov petrochenkov mentioned this pull request Apr 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

Assignees

@jyn514 jyn514

Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.53.0
Development

Successfully merging this pull request may close these issues.

rustdoc is parsing email link literals in docs as intradoc links in the latest nightly
6 participants
@camelid @rust-highfive @jyn514 @bors @petrochenkov @rustbot