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

Jump to bottom

Remove _ from UnpinStruct #70

Closed
wants to merge 1 commit into from
Closed

Remove _ from UnpinStruct #70

wants to merge 1 commit into from

Conversation

taiki-e
Copy link
Owner

@taiki-e taiki-e commented Sep 4, 2019

LucioFranco
LucioFranco approved these changes Sep 4, 2019
View reviewed changes
Copy link
Sponsor

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

👍 wonder if we could provide the option to name it to a custom name.

@taiki-e
Copy link
Owner Author

taiki-e commented Sep 4, 2019

@LucioFranco

👍 wonder if we could provide the option to name it to a custom name.

Looks like a good idea.

Probably like this?

#[pin_project(unpin_docs = MyUnpinStructName)]
struct Foo<T, U> {
    #[pin]
    pinned: T,
    unpinned: U,
}

(Note: Once rust-lang/rust#48054 is implemented and stabilized, these documents will not be generated, but the options themselves will be maintained for compatibility.)

@LucioFranco
Copy link
Sponsor

@taiki-e seems reasonable!

@seanmonstar
Copy link

I didn't even realize that it exports a new type. Could it be doc(hidden), or even better, not pub?

@taiki-e
Copy link
Owner Author

taiki-e commented Sep 4, 2019

@seanmonstar
I'm sorry. There is certainly a lack of documentation on this behavior.
This is intended to avoid the issue that cannot know the actual trait implementation bounds of the Unpin implementation from the document of generated code. And a way to opt-out of this was added in #62.

@taiki-e
Copy link
Owner Author

taiki-e commented Sep 4, 2019 •
edited
Loading

I am considering disabling this by default.
This is because the current way is that if the struct is declared in a public module, UnpinStruct is displayed, but if it is declared in a private module, there is a discrepancy that UnpinStruct does not appear in the document.

cc @Aaron1011

@taiki-e
Copy link
Owner Author

taiki-e commented Sep 4, 2019

It should also be a problem that items that are not declared by the user appear in the document without permission.

@taiki-e taiki-e mentioned this pull request Sep 4, 2019
Merged
@taiki-e
Copy link
Owner Author

taiki-e commented Sep 4, 2019

Closing this in favor of #71.

@taiki-e taiki-e closed this Sep 4, 2019
@taiki-e taiki-e deleted the docs branch September 4, 2019 21:02
bors bot added a commit that referenced this pull request Sep 4, 2019
@bors @taiki-e
Merge #71
b75e631 
71: Do not display UnpinStruct in the document by default r=taiki-e a=taiki-e

There are some problems as mentioned in #70 

This causes the problem that by default the actual Unpin bounds cannot be known from the document if the original struct/enum itself is public (see #53 (comment) and rust-lang/rust#63281 for more).

This can be enabled using `--cfg pin_project_show_unpin_struct` in RUSTFLAGS.

```toml
# in Cargo.toml
[package.metadata.docs.rs]
rustdoc-args = ["--cfg", "pin_project_show_unpin_struct"]
```

cc @Aaron1011 @seanmonstar @LucioFranco

Co-authored-by: Taiki Endo <te316e89@gmail.com>
@taiki-e taiki-e added the A-pin-projection Area: #[pin_project] label Jun 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LucioFranco LucioFranco LucioFranco approved these changes

Assignees
No one assigned
Labels
A-pin-projection Area: #[pin_project]
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@taiki-e @LucioFranco @seanmonstar