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

Canonical Paths #132

Merged
merged 3 commits into from
Oct 15, 2017
Merged

Canonical Paths #132

merged 3 commits into from
Oct 15, 2017

Conversation

Havvy
Copy link
Contributor

@Havvy Havvy commented Oct 13, 2017

Defines canonical paths, as part of #129.

Note to reviewer(s): There's also two commits that just change the location of links and change where newlines are, so just focus on the giant green blob in the paths file.

@Havvy Havvy requested a review from steveklabnik October 13, 2017 19:43
src/paths.md Outdated
pub struct Struct; // ::a::Struct

pub trait Trait { // ::a::Trait
fn f(&self); // <::a::Trait>::f
Copy link
Contributor

Choose a reason for hiding this comment

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

<::a::Trait>::f -> ::a::Trait::f.
<::a::Trait>::f has another meaning - inherent associated item on the trait object type ::a::Trait.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going with the assumption you are right, but I thought ::a::Trait::f was syntactic sugar for <::a::Trait>::f.

Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

Looks good to me overall but one small note.

src/paths.md Outdated
the path component the item itself defines.

[Implementations] and [use declarations] do not have canonical paths, although
the associated items that implementations define do have them. Items defined in
Copy link
Member

Choose a reason for hiding this comment

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

i wonder if "associated items" is too overloaded a term here, it made me do a double-take at first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's technically extraneous, as implementations only define associated items. I removed it; does it flow better that way?

src/paths.md Outdated
The path prefix for modules is the canonical path to that module. For bare
implementations, it is the canonical path of the item being implemented
surrounded by angle (`<>`) brackets. For trait implementations, it is the
cananonical path of the item being implemented followed by `as` followed by the
Copy link
Contributor

Choose a reason for hiding this comment

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

cananonical -> canonical

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks! ❤️

@steveklabnik steveklabnik merged commit 37cc523 into rust-lang:master Oct 15, 2017
@Havvy Havvy deleted the follow-the-path branch August 21, 2018 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants