Skip to content

Missing short summaries for reexported items in rustdoc #77783

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

Closed
ollie27 opened this issue Oct 10, 2020 · 6 comments · Fixed by #77790
Closed

Missing short summaries for reexported items in rustdoc #77783

ollie27 opened this issue Oct 10, 2020 · 6 comments · Fixed by #77790
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@ollie27
Copy link
Member

ollie27 commented Oct 10, 2020

For example the current nightly docs show:
image

The summary lines for the reexported modules is missing. They're also missing from the search index and sidebar.

This is a regression from #77519. cc @jyn514 @GuillaumeGomez

@ollie27 ollie27 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. C-bug Category: This is a bug. labels Oct 10, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 10, 2020
@ollie27
Copy link
Member Author

ollie27 commented Oct 10, 2020

Testing locally, this is fixed by simply removing the newly added DocFragmentKind::Divider but I don't know what that was added for so I don't know if that's the correct fix.

@jyn514
Copy link
Member

jyn514 commented Oct 10, 2020

this is fixed by simply removing the newly added DocFragmentKind::Divider but I don't know what that was added

This was added to distinguish between the attributes on the original item and attributes on the re-export: 8fbfdc5. Previously, collapse_docs would collapse all attributes into one giant attribute:

// crate1
/// Docs in original
pub struct S;

// crate2
/// Docs on re-export
pub use crate1::S;

// how rustdoc sees it: as if _both_ docs were in crate1
/// Docs in original
/// Docs on re-export
pub struct S;

After my change, it distinguishes between the same two in collapse_docs, so they're now separate attributes the way #[doc = "x"] and /// x are not combined:

if matches!(*curr_kind, DocFragmentKind::Include { .. }) || curr_kind != new_kind {

I think the issue is that doc_value is now returning the Divider even if there were no additional docs:

/// value found.
pub fn doc_value(&self) -> Option<&str> {
self.doc_strings.first().map(|s| s.doc.as_str())
}

There are two possible fixes:

  1. Don't add a divider if there were no new docs. I like this one because it's simple.
  2. Change doc_value to ignore the first attribute if it's a Divider and look for the first attribute which isn't. This has the drawback there might be other places using attrs, although off the top of my head I don't know where.

I'll try to get a fix in with 1. today.

@jyn514
Copy link
Member

jyn514 commented Oct 10, 2020

And good catch, thanks!

@jyn514 jyn514 added the A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate label Oct 10, 2020
@camelid
Copy link
Member

camelid commented Oct 10, 2020

Assigning P-high and removing I-prioritize as discussed in the prioritization working group.

@camelid camelid added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 10, 2020
@ollie27
Copy link
Member Author

ollie27 commented Oct 11, 2020

This was added to distinguish between the attributes on the original item and attributes on the re-export: 8fbfdc5.

Right, I figured that's what it was trying to do but the Divider is being placed at the beginning of the list of DocFragments:

doc_strings.borrow_mut().push(DocFragment::divider());
so it's not actually dividing anything. The result is that collapse_docs is still collapsing the docs as it did before so if a reexport has extra docs, all links are resolved relative to the local crate.

Rather than adding Divider couldn't collapse_docs check for differing parent_modules to decide whether to merge DocFragments? That seems like the simpler solution.

@jyn514
Copy link
Member

jyn514 commented Oct 12, 2020

Oops, you're right on both counts. I guess it didn't show up because of #77200.

couldn't collapse_docs check for differing parent_modules to decide whether to merge DocFragments

That sounds better, yeah, then we could get rid of Divider. Let me try that.

@bors bors closed this as completed in 1fe9b7f Oct 12, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate C-bug Category: This is a bug. P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants