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

rustdoc: Record crate name instead of using None #83399

Merged
merged 2 commits into from
Mar 24, 2021
Merged

rustdoc: Record crate name instead of using None #83399

merged 2 commits into from
Mar 24, 2021

Conversation

camelid
Copy link
Member

@camelid camelid commented Mar 23, 2021

Fixes #83365.

r? @jyn514

@camelid camelid added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Mar 23, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 23, 2021
@jyn514
Copy link
Member

jyn514 commented Mar 23, 2021

Looks great :) r=me with CI passing.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@camelid
Copy link
Member Author

camelid commented Mar 23, 2021

Uh... why are 38 tests failing? That's weird...

@camelid
Copy link
Member Author

camelid commented Mar 23, 2021

Maybe the change in how crate names are used is messing up URLs or something?

@jyn514
Copy link
Member

jyn514 commented Mar 23, 2021

I ran into something like this in #78082 - see if this is adding an extra directory named after the top-level crate. There was somewhere in html/render that overrode this later.

@camelid camelid added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 23, 2021
@camelid
Copy link
Member Author

camelid commented Mar 23, 2021

I ran into something like this in #78082 - see if this is adding an extra directory named after the top-level crate.

Yep, it is doing that. Do you remember where the place in html/render was?

@jyn514
Copy link
Member

jyn514 commented Mar 23, 2021 •
edited
Loading

Not off the top of my head, sorry. I think I changed it as part of #78082 - you could try reading through the diff, but it's rather large.

@camelid
Copy link
Member Author

camelid commented Mar 23, 2021 •
edited
Loading

I do see this change, but haven't found the change that I need yet...

Found this: 1697ada

And this: 5e2793b

@camelid
Copy link
Member Author

camelid commented Mar 23, 2021 •
edited
Loading

Okay, it looks like the bug is in whatever's populating cache.paths. (No need to respond, just recording this so I don't forget.)

...cache.paths is populated based on cache.stack...

@camelid
Don't push the crate name onto the Cache.stack
c9ae359 
Now that we record the crate's name in its `clean::Item`, pushing the
crate name onto the `stack` causes duplicate paths. E.g., the URL
generated for the path `::foo::bar::baz` would be something like

    ../foo/foo/bar/baz

With this commit, the URL is corrected to

    ../foo/bar/baz
@camelid
Copy link
Member Author

camelid commented Mar 23, 2021

Fixed the bug. Will push up commit tomorrow.

@camelid camelid removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 23, 2021
@camelid camelid mentioned this pull request Mar 23, 2021
Merged
@jyn514
Copy link
Member

jyn514 commented Mar 23, 2021

@bors r+

@bors
Copy link
Contributor

bors commented Mar 23, 2021

📌 Commit c9ae359 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 Mar 23, 2021
@Dylan-DPC-zz Dylan-DPC-zz mentioned this pull request Mar 24, 2021
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2021
@bors
Auto merge of rust-lang#83432 - Dylan-DPC:rollup-4z5f6al, r=Dylan-DPC
db492ec 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#83051 (Sidebar trait items order)
 - rust-lang#83313 (Only enable assert_dep_graph when query-dep-graph is enabled.)
 - rust-lang#83353 (Add internal io::Error::new_const to avoid allocations.)
 - rust-lang#83391 (Allow not emitting `uwtable` on Android)
 - rust-lang#83392 (Change `-W help` to display edition level.)
 - rust-lang#83393 (Codeblock tooltip position)
 - rust-lang#83399 (rustdoc: Record crate name instead of using `None`)
 - rust-lang#83405 (Slight visual improvements to warning boxes in the docs)
 - rust-lang#83415 (Remove unnecessary `Option` wrapping around `Crate.module`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3d645a3 into rust-lang:master Mar 24, 2021
@rustbot rustbot added this to the 1.53.0 milestone Mar 24, 2021
@camelid camelid deleted the rustdoc-priv-link-from-crate branch March 25, 2021 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jyn514 jyn514

Labels
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 prints <unknown> as name of crate root
6 participants
@camelid @jyn514 @rust-log-analyzer @bors @rust-highfive @rustbot