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.

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

Add doc comment to hiding portions of code example #50852

Merged
merged 2 commits into from
Jun 5, 2018

Conversation

mandeep
Copy link
Contributor

@mandeep mandeep commented May 18, 2018

fixes #50816

Not sure if this is all that's needed, but I think it's a good start. One thing to note is that the code block is a text block where it could possibly be a rust block.

@rust-highfive
Copy link
Collaborator

r? @steveklabnik

(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 May 18, 2018
# fn foo() {}
/// /// Some documentation.
/// # fn foo() {} // this function will be hidden
/// println!("Hello, World!");
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a println!?

Copy link
Member

Choose a reason for hiding this comment

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

yes, r=me with this line removed :)

Choose a reason for hiding this comment

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

LGTM!

# fn foo() {}
/// /// Some documentation.
/// # fn foo() {} // this function will be hidden
/// println!("Hello, World!");
Copy link
Member

Choose a reason for hiding this comment

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

yes, r=me with this line removed :)

@mandeep
Copy link
Contributor Author

mandeep commented May 18, 2018

The foo function is there to show how to hide code. I wanted to add the println to show how normal code would look like in addition to hidden code.

@GuillaumeGomez
Copy link
Member

Then you definitely need to update the next code example as well. But that seems really strange. Why would you put a doc comment on a hidden function? Doesn't make sense to me...

@mandeep mandeep force-pushed the fix-rustdoc-example-testing branch from 440944b to 8d74cf8 Compare May 18, 2018 15:10
@mandeep
Copy link
Contributor Author

mandeep commented May 18, 2018

The PR attempts to show users that one needs doc comments on all of their example code so that they don't write something like the following:

/// # Examples
/// /// Some documentation
# fn foo() {}
/// println("Hi");

@steveklabnik
Copy link
Member

@mandeep that makes sense, but I'd prefer the examples to have actual syntax, then. You need curlies, for example.

@mandeep
Copy link
Contributor Author

mandeep commented May 18, 2018

@steveklabnik Are curly braces needed somewhere besides the foo function?

@mandeep
Copy link
Contributor Author

mandeep commented May 20, 2018

I discussed this on IRC and there may have been confusion with my example in my comment that contains code. I've updated the example to match the commit of the PR.

@GuillaumeGomez
Copy link
Member

I have to admit that I still don't understand at all why there is a doc comment on a hidden function in an example. Or even why there is a doc comment in an example...

@pietroalbini
Copy link
Member

Ping from triage! What's the status on this?

@mandeep
Copy link
Contributor Author

mandeep commented Jun 3, 2018

@GuillaumeGomez maybe an example will help:

/// Find the barycentric coordinates of the given point with respect to the given triangle
///
/// # Examples
///
/// ```
/// let points =  vec![Vector3::new(0, 0, 0), Vector3::new(2, 2, 2), Vector3::new(0, 2, 2)]
/// let point = Point3::new(1.0, 1.0, 0.0);
/// let barycentric_coordinates: Point3<f64> = find_barycentric(&points, &point);
/// ```
///
fn find_barycentric(points: &Vec<Point3<f64>>, point: &Point3<f64>) -> Point3<f64> {
    let u = Vector3::new(points[2].x - points[0].x, points[1].x - points[0].x, points[0].x - point.x);
    let v = Vector3::new(points[2].y - points[0].y, points[1].y - points[0].y, points[0].y - point.y);

    let w = u.cross(&v);

    if (w.z).abs() < 1.0 {
        return Point3::new(-1.0, 1.0, 1.0);
    } else {
        return Point3::new(1.0 - (w.x + w.y) as f64 / w.z as f64, w.y as f64 / w.z as f64, w.x as f64 / w.z as f64);
    }

}

As for the hidden function, the docs being changed have a full example of hidden functions: https://doc.rust-lang.org/rustdoc/documentation-tests.html#hiding-portions-of-the-example

@GuillaumeGomez
Copy link
Member

I still don't get it. :-/

@mandeep
Copy link
Contributor Author

mandeep commented Jun 5, 2018

The use case is not immediately apparent, but since this omission is causing problems I think we need to add this commit. If you think more information is needed, let me know and I'd be happy to add more.

@GuillaumeGomez
Copy link
Member

Ok, then let's just move forward.

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jun 5, 2018

📌 Commit 8d74cf8 has been approved by GuillaumeGomez

@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 Jun 5, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 5, 2018
…, r=GuillaumeGomez

Add doc comment to hiding portions of code example

fixes rust-lang#50816

Not sure if this is all that's needed, but I think it's a good start. One thing to note is that the code block is a text block where it could possibly be a rust block.
bors added a commit that referenced this pull request Jun 5, 2018
Rollup of 7 pull requests

Successful merges:

 - #50852 (Add doc comment to hiding portions of code example)
 - #51183 (Update rustdoc book to suggest using Termination trait instead of hidden ‘foo’ function)
 - #51255 (Fix confusing error message for sub_instant)
 - #51256 (Fix crate-name option in rustdoc)
 - #51308 (Check array indices in constant propagation)
 - #51343 (test: Ignore some problematic tests on sparc and sparc64)
 - #51358 (Tests that #39963 is fixed on MIR borrowck)

Failed merges:
@bors bors merged commit 8d74cf8 into rust-lang:master Jun 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc does not clarify that examples need to be prepended with ///
7 participants