-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
rustdoc: allow moving methods from deref below trait impls via a new unstable command line argument #92273
rustdoc: allow moving methods from deref below trait impls via a new unstable command line argument #92273
Conversation
r? @CraftSpider (rust-highfive has picked a reviewer for you, use r? to override) |
I'm on vacation the next month or so. I'm a little hesitant to add more configuration to rustdoc; this seems sufficiently niche I don't think we should stabilize it and we already have far too many unstable flags. |
I generally agree with that assessment as well, I don't think we need many more unstable flags |
I did add it as an unstable one for that reason. I would not be concerned if it basically remained "perma-unstable" as various things in Rust often do. Is there a better manner in which you suggest I might be able to re-implement what #83826 did for the two months or so it was merged? To be clear, the current state of things is that
made on #85618 is not really true as far as I can tell. "Methods From Deref" are still currently rendered in their entirety directly after "Methods", no matter what. |
We relanded the Deref PR recently: #90183
This is exactly my point, rustdoc really don't need more perma-unstable flags. I don't think #83826 was a bad change; maybe talk with @jsha about whether there's some compromise you can come to there, not sure whether I agree #85618 is actually a regression. Maybe we can put methods from the first deref before traits and methods from all other deref impls after? |
Well, that unfortunately wouldn't help my motivating case for this PR (and for #83826) at all, which is having a struct that implements I agree that this rendering behavior does make sense in the specific case of I certainly understand where you're coming from when you say "this is exactly my point, Stuff like how |
Rustdoc has a lot of bugs and edge cases, and having many different ways to configure rustdoc doesn't help since it increases complexity. This seems like a fairly niche use case, so I'd also rather not add the flag, even if (or especially if) it's perma-unstable. At some point, it might make sense to allow configuring rustdoc in a more uniform way, but having lots of one-off flags isn't the best way to do it IMO. |
I'd at least somewhat argue that the difference here is that this exact functionality was previously made the always-on default for |
☔ The latest upstream changes (presumably #97239) made this pull request unmergeable. Please resolve the merge conflicts. |
Closing this based on the above discussion that there is no way forward for this pr to land. Thanks |
This PR has been open for a long time, and based on the discussion, it seems very unlikely that we will move forward with it. But in any case, thanks for working on this! Don't hesitate to come talk to us if you have more ideas. |
Basically, with this applied, running
cargo rustdoc -- -Z unstable-options --show-deref-methods-last
will generate docs with "Methods From Deref" as the last thing on the sidebar and main page body, as opposed to it coming right after "Methods".I feel like this is a suitably unintrusive way of achieving the same thing that #83826 did before being reverted, as it does not change how docs are rendered at all in any scenario where the new flag is not explicitly passed.
@jyn514, since they originally provided feedback related to #83826.