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

[Rustdoc] Render tuple fields in structs correctly #80320

Closed
wants to merge 2 commits into from
Closed

[Rustdoc] Render tuple fields in structs correctly #80320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca664b0
Fix: render tuple fields in structs correctly
RDambrosio016 Dec 23, 2020
bf0fd2f
Chore: test for no rendering if the field is not public
RDambrosio016 Dec 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions src/librustdoc/html/render/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3079,21 +3079,22 @@ fn item_struct(
_ => None,
})
.peekable();
if let doctree::Plain = s.struct_type {
if let doctree::Plain | doctree::Tuple = s.struct_type {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't even know we could do that in rust! 😮

if fields.peek().is_some() {
let field_string =
if let doctree::Plain = s.struct_type { "Fields" } else { "Tuple Fields" };
write!(
w,
"<h2 id=\"fields\" class=\"fields small-section-header\">
Fields{}<a href=\"#fields\" class=\"anchor\"></a></h2>",
{}{}<a href=\"#fields\" class=\"anchor\"></a></h2>",
field_string,
document_non_exhaustive_header(it)
);
document_non_exhaustive(w, it);
for (field, ty) in fields {
let id = cx.derive_id(format!(
"{}.{}",
ItemType::StructField,
field.name.as_ref().unwrap()
));
for (idx, (field, ty)) in fields.enumerate() {
let field_name =
field.name.map_or_else(|| idx.to_string(), |sym| (*sym.as_str()).to_string());
let id = cx.derive_id(format!("{}.{}", ItemType::StructField, field_name));
write!(
w,
"<span id=\"{id}\" class=\"{item_type} small-section-header\">\
Expand All @@ -3102,7 +3103,7 @@ fn item_struct(
</span>",
item_type = ItemType::StructField,
id = id,
name = field.name.as_ref().unwrap(),
name = field_name,
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
ty = ty.print()
);
document(w, cx, field, Some(it));
Expand Down
11 changes: 11 additions & 0 deletions src/test/rustdoc/tuple_struct_fields.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// @has tuple_struct_fields/struct.Tooople.html
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a check like:

Suggested change
// @has tuple_struct_fields/struct.Tooople.html
// @has tuple_struct_fields/struct.Tooople.html
// @has - 'Tuple Fields'

to make sure the heading is correct?

Copy link
Contributor Author

@RDambrosio016 RDambrosio016 Jan 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think i added that at one point then removed it because it wasn't working (and the errors are less helpful than staring at a wall), but ill try again tomorrow 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and the errors are less helpful than staring at a wall)

Yeah, htmldocck errors could use some work.

// @has - //span '0: usize'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you write more complete paths please? It's much easier when an unwanted change occur to find out where it's from in such cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As i stated in an earlier comment, i tried that, but the test harness kept erroring and i could not figure out why, which is why i removed it

Copy link
Member

@GuillaumeGomez GuillaumeGomez Jan 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can help with that! :) If it's the same as a normal field, the path should look like this:

//section[@id="main"]/span[@id="structfield.0"] '0: usize'

Also, please add tests to ensure that "Tuple fields" header is there and that the fields are in the sidebar too.

// @has - 'Wow! i love tuple fields!'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same.

// @!has - 'I should be invisible'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


pub struct Tooople(
/// Wow! i love tuple fields!
pub usize,
/// I should be invisible
u8,
);