Skip to content

Remove many unnecessary manual link resolves from library #77832

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 2, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Oct 11, 2020

Now that #76934 has merged, we can remove a lot of these! E.g, this is
no longer necessary:

[`Vec<T>`]: Vec

cc @jyn514

@camelid camelid added A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools labels Oct 11, 2020
@rust-highfive
Copy link
Contributor

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 11, 2020
@camelid
Copy link
Member Author

camelid commented Oct 11, 2020

If anyone's interested, I used this kind of hacky Python script (gist):

import re
from pathlib import Path
import sys

pat = re.compile(r'\[`?(.*)<.*>(::([A-Za-z0-9_]+))?(\(\))?`?\]: \1(::\3)?')

for path in Path(sys.argv[1]).rglob('*.rs'):
  print(path)

  inf = open(path, 'r')
  lines = inf.readlines()
  inf.close()

  outf = open(path, 'w')
  prev_line_empty = False
  prev_prev_line_empty = False
  for line in lines:
    if not pat.search(line):
      if not (prev_line_empty and prev_prev_line_empty and line.strip('/ \n') == ''):
        outf.write(line)
      prev_prev_line_empty = prev_line_empty
      prev_line_empty = line.strip('/ \n') == ''
    else:
      print('match!')
      prev_prev_line_empty = prev_line_empty
      prev_line_empty = True  # empty because was not written to file
  outf.close()

There were a few spots where it was over-eager in removing blank lines, but they were easy to fix manually.

@jyn514
Copy link
Member

jyn514 commented Oct 11, 2020

r? @jyn514

@jyn514
Copy link
Member

jyn514 commented Oct 11, 2020

This is blocked until the next beta bump, I think, we intentionally test the standard library is documented with --stage 0 so library contributors don't have to build the compiler: #75368 (comment)

@camelid
Copy link
Member Author

camelid commented Oct 11, 2020

Oops, forgot :)

Oh well, at least the PR will be ready then.

@camelid camelid added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 11, 2020
@bors

This comment has been minimized.

@camelid camelid force-pushed the remove-manual-link-resolves branch from 4e12af1 to ea3da77 Compare November 22, 2020 21:26
@camelid
Copy link
Member Author

camelid commented Nov 22, 2020

Rebased. I think this is ready now!

@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Nov 22, 2020
Dylan-DPC-zz referenced this pull request in Dylan-DPC-zz/rust Nov 24, 2020
Accept '!' in intra-doc links

This will allow linking to things like `Result<T, !>`.

*See <https://github.com/rust-lang/rust/pull/77832#discussion_r528409079>.*

r? `@jyn514`
@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

@camelid I think this is ready to go once you rebase (which should fix the linkchecker error).

@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 Nov 29, 2020
@camelid
Copy link
Member Author

camelid commented Nov 29, 2020

Don't we have to wait for the next beta bump though?

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

@camelid I think I might have been wrong about that - since the links fail silently, they won't actually cause x.py doc --stage 0 to fail, the links will just be broken. Since I don't expect many people to be looking at every link in a stage 0 build, I think it's ok to have them broken for a cycle - doc.rust-lang.org/nightly/ will still have the correct links.

@camelid
Copy link
Member Author

camelid commented Nov 29, 2020

@jyn514 Actually they don't fail silently (at least not in CI):

 std/primitive.never.html:44: broken intra-doc link - [<code>Result&lt;String, !&gt;</code>]
std/primitive.never.html:51: broken intra-doc link - [<code>Result&lt;T, !&gt;</code>]
std/primitive.never.html:55: broken intra-doc link - [<code>Result&lt;T, !&gt;</code>]
std/primitive.never.html:56: broken intra-doc link - [<code>Result&lt;T, !&gt;</code>]
std/primitive.never.html:57: broken intra-doc link - [<code>Result&lt;!, E&gt;</code>]
thread 'main' panicked at 'found some broken links', src/tools/linkchecker/main.rs:109:9
std/primitive.never.html:81: broken intra-doc link - [<code>Result&lt;!, E&gt;</code>]

@jyn514
Copy link
Member

jyn514 commented Nov 29, 2020

@camelid I'm pretty sure that's from before #79321 landed, it's from a --stage 1 build, not a --stage 0 build.

@camelid
Copy link
Member Author

camelid commented Nov 29, 2020

@jyn514 I just told CI to re-run, so we'll see what happens :)

@camelid
Copy link
Member Author

camelid commented Nov 29, 2020

CI still failed with the same linkchecker errors.

@camelid
Copy link
Member Author

camelid commented Nov 29, 2020

Probably this should just wait until the next beta bump – it's not urgent, although it is annoying to have to wait another 5 weeks :/

@camelid camelid added S-blocked Status: Blocked on something else such as an RFC or other implementation work. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 4, 2020
@camelid
Copy link
Member Author

camelid commented Dec 26, 2020

Only a few more days and then we can finally merge this! 😂

@camelid camelid added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Dec 31, 2020
@camelid
Copy link
Member Author

camelid commented Dec 31, 2020

Okay, the beta bump happened! Re-running CI...

@camelid
Copy link
Member Author

camelid commented Dec 31, 2020

Hmm, I don't see the button to re-run the workflow. Maybe because it's too old.

@jyn514 do you want to r+?

@jyn514
Copy link
Member

jyn514 commented Dec 31, 2020

@camelid can you rebase and push manually to make sure the links actually work?

Now that rust-lang#76934 has merged, we can remove a lot of these! E.g, this is
no longer necessary:

    [`Vec<T>`]: Vec
@camelid camelid force-pushed the remove-manual-link-resolves branch from ea3da77 to 0506789 Compare December 31, 2020 19:55
@camelid
Copy link
Member Author

camelid commented Dec 31, 2020

@jyn514 CI passed. Should be ready to go!

@jyn514
Copy link
Member

jyn514 commented Dec 31, 2020

@bors r+ rollup

Thanks for sticking with this! I know waiting for the release trains is a pain.

@bors
Copy link
Collaborator

bors commented Dec 31, 2020

📌 Commit 0506789 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 Dec 31, 2020
@bors
Copy link
Collaborator

bors commented Jan 2, 2021

⌛ Testing commit 0506789 with merge 0876f59...

@bors
Copy link
Collaborator

bors commented Jan 2, 2021

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 0876f59 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 2, 2021
@bors bors merged commit 0876f59 into rust-lang:master Jan 2, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 2, 2021
@camelid camelid deleted the remove-manual-link-resolves branch January 2, 2021 23:09
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-intra-doc-links Area: Intra-doc links, the ability to link to items in docs by name C-cleanup Category: PRs that clean code up or issues documenting cleanup. merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants