Skip to content

Commit

Permalink
Remove toggle for "undocumented items."
Browse files Browse the repository at this point in the history
Per discussion in rust-lang#84326. For trait implementations, this was
misleading: the items actually do have documentation (but it comes from
the trait definition).

For both trait implementations and trait implementors, this was
redundant: in both of those cases, the items are default-hidden by
different toggle at the level above.

Update tests: Remove XPath selectors that over-specified on details tag,
in cases that weren't testing toggles. Add an explicit test for toggles
on methods. Rename item-hide-threshold to toggle-item-contents for
consistency.
  • Loading branch information
jsha committed May 31, 2021
1 parent 84b1005 commit 10bafe1
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 54 deletions.
14 changes: 4 additions & 10 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1650,16 +1650,10 @@ fn render_impl(
);
}
}
if toggled {
w.write_str("<div class=\"impl-items\">");
w.push_buffer(default_impl_items);
if trait_.is_some() && !impl_items.is_empty() {
w.write_str("<details class=\"undocumented\"><summary></summary>");
close_tags.insert_str(0, "</details>");
}
w.push_buffer(impl_items);
close_tags.insert_str(0, "</div>");
}
w.write_str("<div class=\"impl-items\">");
w.push_buffer(default_impl_items);
w.push_buffer(impl_items);
close_tags.insert_str(0, "</div>");
w.write_str(&close_tags);
}

Expand Down
6 changes: 2 additions & 4 deletions src/test/rustdoc/assoc-consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,12 @@ impl Qux for Bar {
/// Docs for QUX1 in impl.
const QUX1: i8 = 5;
// @has - '//*[@id="associatedconstant.QUX_DEFAULT0"]' 'const QUX_DEFAULT0: u16'
// @!has - '//div[@class="impl-items"]/details[@open=""]//*[@class="docblock"]' "Docs for QUX_DEFAULT12 in trait."
// @has - '//div[@class="impl-items"]/details//*[@class="docblock"]' "Docs for QUX_DEFAULT12 in trait."
// @has - '//div[@class="impl-items"]//*[@class="docblock"]' "Docs for QUX_DEFAULT12 in trait."
const QUX_DEFAULT0: u16 = 6;
// @has - '//*[@id="associatedconstant.QUX_DEFAULT1"]' 'const QUX_DEFAULT1: i16'
// @has - '//*[@class="docblock"]' "Docs for QUX_DEFAULT1 in impl."
/// Docs for QUX_DEFAULT1 in impl.
const QUX_DEFAULT1: i16 = 7;
// @has - '//*[@id="associatedconstant.QUX_DEFAULT2"]' 'const QUX_DEFAULT2: u32'
// @!has - '//div[@class="impl-items"]/details[@open=""]//*[@class="docblock"]' "Docs for QUX_DEFAULT2 in trait."
// @has - '//div[@class="impl-items"]/details//*[@class="docblock"]' "Docs for QUX_DEFAULT2 in trait."
// @has - '//div[@class="impl-items"]//*[@class="docblock"]' "Docs for QUX_DEFAULT2 in trait."
}
9 changes: 3 additions & 6 deletions src/test/rustdoc/inline_cross/assoc-items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,15 @@ extern crate assoc_items;
// @has - '//*[@id="associatedconstant.ConstNoDefault"]' 'const ConstNoDefault: i16'
// @has - '//*[@class="docblock"]' 'dox for ConstNoDefault'
// @has - '//*[@id="associatedconstant.ConstWithDefault"]' 'const ConstWithDefault: u16'
// @!has - '//details[@open=""]/details/div[@class="docblock"]' 'docs for ConstWithDefault'
// @has - '//details/details/div[@class="docblock"]' 'docs for ConstWithDefault'
// @has - '//div[@class="docblock"]' 'docs for ConstWithDefault'
// @has - '//*[@id="associatedtype.TypeNoDefault"]' 'type TypeNoDefault = i32'
// @has - '//*[@class="docblock"]' 'dox for TypeNoDefault'
// @has - '//*[@id="associatedtype.TypeWithDefault"]' 'type TypeWithDefault = u32'
// @!has - '//details[@open=""]/details/div[@class="docblock"]' 'docs for TypeWithDefault'
// @has - '//details/details/div[@class="docblock"]' 'docs for TypeWithDefault'
// @has - '//div[@class="docblock"]' 'docs for TypeWithDefault'
// @has - '//*[@id="method.method_no_default"]' 'fn method_no_default()'
// @has - '//*[@class="docblock"]' 'dox for method_no_default'
// @has - '//*[@id="method.method_with_default"]' 'fn method_with_default()'
// @!has - '//details[@open=""]/details/div[@class="docblock"]' 'docs for method_with_default'
// @has - '//details/details/div[@class="docblock"]' 'docs for method_with_default'
// @has - '//div[@class="docblock"]' 'docs for method_with_default'
pub use assoc_items::MyStruct;

// @has foo/trait.MyTrait.html
Expand Down
3 changes: 1 addition & 2 deletions src/test/rustdoc/inline_cross/impl-inline-without-trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,5 @@ extern crate impl_inline_without_trait;

// @has 'foo/struct.MyStruct.html'
// @has - '//*[@id="method.my_trait_method"]' 'fn my_trait_method()'
// @!has - '//details[@open=""]/details/div[@class="docblock"]' 'docs for my_trait_method'
// @has - '//details/details/div[@class="docblock"]' 'docs for my_trait_method'
// @has - '//div[@class="docblock"]' 'docs for my_trait_method'
pub use impl_inline_without_trait::MyStruct;
22 changes: 7 additions & 15 deletions src/test/rustdoc/manual_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,10 @@ pub trait T {
// @has - '//*[@class="docblock"]' 'Docs associated with the S1 trait implementation.'
// @has - '//*[@class="docblock"]' 'Docs associated with the S1 trait a_method implementation.'
// @!has - '//*[@class="docblock"]' 'Docs associated with the trait a_method definition.'
// @!has - '//div[@class="impl-items"]/details[@open=""]//div[@class="docblock"]' 'Docs associated with the trait b_method definition.'
// @has - '//div[@class="impl-items"]/details//div[@class="docblock"]' 'Docs associated with the trait b_method definition.'
// @!has - '//div[@class="impl-items"]/details[@open=""]//div[@class="docblock"]' 'Docs associated with the trait c_method definition.'
// @has - '//div[@class="impl-items"]/details//div[@class="docblock"]' 'Docs associated with the trait c_method definition.'
// @has - '//div[@class="impl-items"]//div[@class="docblock"]' 'Docs associated with the trait b_method definition.'
// @has - '//div[@class="impl-items"]//div[@class="docblock"]' 'Docs associated with the trait c_method definition.'
// @!has - '//*[@class="docblock"]' 'There is another line'
// @!has - '//div[@class="impl-items"]/details[@open=""]//div[@class="docblock"]' 'Read more'
// @has - '//div[@class="impl-items"]/details//div[@class="docblock"]' 'Read more'
// @has - '//div[@class="impl-items"]//div[@class="docblock"]' 'Read more'
pub struct S1(usize);

/// Docs associated with the S1 trait implementation.
Expand All @@ -45,10 +42,7 @@ impl T for S1 {
// @has - '//*[@class="docblock"]' 'Docs associated with the S2 trait implementation.'
// @has - '//*[@class="docblock"]' 'Docs associated with the S2 trait a_method implementation.'
// @has - '//*[@class="docblock"]' 'Docs associated with the S2 trait c_method implementation.'
// @!has - '//details[open=""]/div[@class="docblock"]' 'Docs associated with the trait a_method definition.'
// @!has - '//details[open=""]/div[@class="docblock"]' 'Docs associated with the trait c_method definition.'
// @!has - '//div[@class="impl-items"]/details[@open=""]//div[@class="docblock"]' 'Docs associated with the trait b_method definition.'
// @has - '//div[@class="impl-items"]/details//div[@class="docblock"]' 'Docs associated with the trait b_method definition.'
// @has - '//div[@class="impl-items"]//div[@class="docblock"]' 'Docs associated with the trait b_method definition.'
pub struct S2(usize);

/// Docs associated with the S2 trait implementation.
Expand All @@ -65,11 +59,9 @@ impl T for S2 {
}

// @has manual_impl/struct.S3.html '//*[@class="trait"]' 'T'
// @has - '//details/div[@class="docblock"]' 'Docs associated with the S3 trait implementation.'
// @!has - '//details[@open=""]/div[@class="docblock"]' 'Docs associated with the S3 trait implementation.'
// @has - '//details[@open=""]/div[@class="docblock"]' 'Docs associated with the S3 trait b_method implementation.'
// @!has - '//div[@class="impl-items"]/details[@open=""]//div[@class="docblock"]' 'Docs associated with the trait a_method definition.'
// @has - '//div[@class="impl-items"]/details//div[@class="docblock"]' 'Docs associated with the trait a_method definition.'
// @has - '//div[@class="docblock"]' 'Docs associated with the S3 trait implementation.'
// @has - '//div[@class="docblock"]' 'Docs associated with the S3 trait b_method implementation.'
// @has - '//div[@class="impl-items"]//div[@class="docblock"]' 'Docs associated with the trait a_method definition.'
pub struct S3(usize);

/// Docs associated with the S3 trait implementation.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#![allow(unused)]

// @has 'item_hide_threshold/struct.PubStruct.html'
// @has 'toggle_item_contents/struct.PubStruct.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 0
pub struct PubStruct {
pub a: usize,
pub b: usize,
}

// @has 'item_hide_threshold/struct.BigPubStruct.html'
// @has 'toggle_item_contents/struct.BigPubStruct.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 1
// @has - '//details[@class="rustdoc-toggle type-contents-toggle"]' 'Show fields'
pub struct BigPubStruct {
Expand All @@ -26,7 +26,7 @@ pub struct BigPubStruct {
pub m: usize,
}

// @has 'item_hide_threshold/union.BigUnion.html'
// @has 'toggle_item_contents/union.BigUnion.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 1
// @has - '//details[@class="rustdoc-toggle type-contents-toggle"]' 'Show fields'
pub union BigUnion {
Expand All @@ -45,23 +45,23 @@ pub union BigUnion {
pub m: usize,
}

// @has 'item_hide_threshold/union.Union.html'
// @has 'toggle_item_contents/union.Union.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 0
pub union Union {
pub a: usize,
pub b: usize,
pub c: usize,
}

// @has 'item_hide_threshold/struct.PrivStruct.html'
// @has 'toggle_item_contents/struct.PrivStruct.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 0
// @has - '//div[@class="docblock type-decl"]' 'fields omitted'
pub struct PrivStruct {
a: usize,
b: usize,
}

// @has 'item_hide_threshold/enum.Enum.html'
// @has 'toggle_item_contents/enum.Enum.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 1
// @has - '//details[@class="rustdoc-toggle type-contents-toggle"]' 'Show fields'
pub enum Enum {
Expand All @@ -72,14 +72,14 @@ pub enum Enum {
}
}

// @has 'item_hide_threshold/enum.LargeEnum.html'
// @has 'toggle_item_contents/enum.LargeEnum.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 1
// @has - '//details[@class="rustdoc-toggle type-contents-toggle"]' 'Show variants'
pub enum LargeEnum {
A, B, C, D, E, F(u8), G, H, I, J, K, L, M
}

// @has 'item_hide_threshold/trait.Trait.html'
// @has 'toggle_item_contents/trait.Trait.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 0
pub trait Trait {
type A;
Expand All @@ -88,7 +88,7 @@ pub trait Trait {
fn bar();
}

// @has 'item_hide_threshold/trait.GinormousTrait.html'
// @has 'toggle_item_contents/trait.GinormousTrait.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 1
// @has - '//details[@class="rustdoc-toggle type-contents-toggle"]' 'Show associated items'
pub trait GinormousTrait {
Expand All @@ -111,7 +111,7 @@ pub trait GinormousTrait {
fn bar();
}

// @has 'item_hide_threshold/trait.HugeTrait.html'
// @has 'toggle_item_contents/trait.HugeTrait.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 1
// @has - '//details[@class="rustdoc-toggle type-contents-toggle"]' 'Show associated constants and methods'
pub trait HugeTrait {
Expand All @@ -133,7 +133,7 @@ pub trait HugeTrait {
fn bar();
}

// @has 'item_hide_threshold/trait.BigTrait.html'
// @has 'toggle_item_contents/trait.BigTrait.html'
// @count - '//details[@class="rustdoc-toggle type-contents-toggle"]' 1
// @has - '//details[@class="rustdoc-toggle type-contents-toggle"]' 'Show methods'
pub trait BigTrait {
Expand Down
18 changes: 18 additions & 0 deletions src/test/rustdoc/toggle-method.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#![crate_name = "foo"]

// Struct methods with documentation should be wrapped in a <details> toggle with an appropriate
// summary. Struct methods with no documentation should not be wrapped.
//
// @has foo/struct.Foo.html
// @has - '//details[@class="rustdoc-toggle method-toggle"]//summary//code' 'is_documented()'
// @has - '//details[@class="rustdoc-toggle method-toggle"]//*[@class="docblock"]' 'is_documented is documented'
// @!has - '//details[@class="rustdoc-toggle method-toggle"]//summary//code' 'not_documented()'
pub struct Foo {
}

impl Foo {
pub fn not_documented() {}

/// is_documented is documented
pub fn is_documented() {}
}
24 changes: 18 additions & 6 deletions src/test/rustdoc/toggle-trait-fn.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
#![crate_name = "foo"]

// Trait methods with documentation should be wrapped in a <details> toggle with an appropriate
// summary. Trait methods with no documentation should not be wrapped.
//
// @has foo/trait.Foo.html
// @!has - '//details[@class="rustdoc-toggle"]//code' 'bar'
// @has - '//code' 'bar'
// @has - '//details[@class="rustdoc-toggle"]//code' 'foo'
// @has - '//details[@class="rustdoc-toggle"]//summary//code' 'is_documented()'
// @!has - '//details[@class="rustdoc-toggle"]//summary//code' 'not_documented()'
// @has - '//details[@class="rustdoc-toggle"]//*[@class="docblock"]' 'is_documented is documented'
// @has - '//details[@class="rustdoc-toggle"]//summary//code' 'is_documented_optional()'
// @!has - '//details[@class="rustdoc-toggle"]//summary//code' 'not_documented_optional()'
// @has - '//details[@class="rustdoc-toggle"]//*[@class="docblock"]' 'is_documented_optional is documented'
pub trait Foo {
fn bar() -> ();
/// hello
fn foo();
fn not_documented();

/// is_documented is documented
fn is_documented();

fn not_documented_optional() {}

/// is_documented_optional is documented
fn is_documented_optional() {}
}

0 comments on commit 10bafe1

Please sign in to comment.