Skip to content

Move root crate re-export docs to the root.#733

Merged
xStrom merged 1 commit intolinebender:masterfrom
xStrom:widgetext-doc
Mar 24, 2020
Merged

Move root crate re-export docs to the root.#733
xStrom merged 1 commit intolinebender:masterfrom
xStrom:widgetext-doc

Conversation

@xStrom
Copy link
Member

@xStrom xStrom commented Mar 22, 2020

WidgetExt shows up as a regular trait in the crate root docs right now, leading to broken links. This PR forces it to work the same way as Widget and WidgetId.

@cmyr
Copy link
Member

cmyr commented Mar 22, 2020

huh, I didn't know about these attributes. In my ideal world, we would inline all the druid types that are currently reexported; is there any reason this doesn't make sense?

@xStrom
Copy link
Member Author

xStrom commented Mar 22, 2020

We can inline them in the root, but then we should hide them in the widget module. Otherwise the docs will be available at two different URLs which will break the relative links. The rust doc linking system is extremely fragile right now, as you know.

@cmyr
Copy link
Member

cmyr commented Mar 22, 2020

That sounds okay to me; I dislike that these types aren't included in the main list of traits. Do you see any downside? If not, would you like to give that a try instead of this solution? If not I'm happy to merge this for now and then open an issue for that other change.

@cmyr cmyr added the S-waiting-on-author waits for changes from the submitter label Mar 22, 2020
@xStrom
Copy link
Member Author

xStrom commented Mar 22, 2020

Sure, I'll give it a go, probably tomorrow.

@xStrom xStrom removed the S-waiting-on-author waits for changes from the submitter label Mar 23, 2020
@xStrom
Copy link
Member Author

xStrom commented Mar 23, 2020

It's a no-go after all. #[doc(no_inline)] doesn't work at the source (e.g. pub trait Widget) and only works on re-exports by forcing them to show up as re-exports. #[doc(hidden)] hides the docs everywhere, even the re-exports despite those having the #[doc(inline)]. The rust docs system could use a lot of improvements!

So for now the best way to keep links working is to merge my original PR.

@cmyr
Copy link
Member

cmyr commented Mar 23, 2020

but what if we do,

#[doc(inline)]
pub use widget::{Widget, WidgetExt, WidgetId};

is the problem that the docs can be linked from two places? because I don't think thats totally the end of the world, they'll be the same docs.

@xStrom
Copy link
Member Author

xStrom commented Mar 23, 2020

Then there are two files trait.Widget.html and widget/trait.Widget.html. The problem is exactly that they have the same docs - they even have the same relative links! Which means that currently all links in trait.Widget.html would be broken. If we fix those then all links in widget/trait.Widget.html are broken.

@cmyr
Copy link
Member

cmyr commented Mar 23, 2020

hm, but can we just ensure that we never link to one of them?

@cmyr
Copy link
Member

cmyr commented Mar 23, 2020

I guess not. :/

@xStrom
Copy link
Member Author

xStrom commented Mar 23, 2020

How badly do we need Widget and WidgetId to be part of the public API in the widget module? One solution could be to move those two into a non-public sub-module (like WidgetExt) and then re-export it from the root crate with inline docs.

I'll give this a shot tomorrow and we'll see if it can be done and how it looks.

@xStrom xStrom added the S-waiting-on-author waits for changes from the submitter label Mar 23, 2020
@cmyr
Copy link
Member

cmyr commented Mar 23, 2020

sounds good.

@xStrom xStrom changed the title Force WidgetExt to show up as re-export in crate root docs. Move root crate re-export docs to the root. Mar 23, 2020
@xStrom xStrom added S-needs-review waits for review and removed S-waiting-on-author waits for changes from the submitter labels Mar 24, 2020
@xStrom
Copy link
Member Author

xStrom commented Mar 24, 2020

Okay, things are looking better now.

  • Widget, WidgetExt, WidgetId, Lens, LensExt, LensWrap docs live exclusively in the root now.
  • kurbo and piet re-exports are now also inlined with their docs.
  • Lens derive macro docs live only in the root.
  • widget::prelude module docs no longer link to doc pages with broken links.
  • Region docs are now properly available in the root.
  • Fixed a few random broken links I ran into.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

This looks like a very clear win, thanks! Afaict this shouldn't break anything; am I understanding correctly?

/// Wrap this widget in a [`Padding`] widget with the given [`Insets`].
///
/// [`Padding`]: struct.Padding.html
/// [`Insets`]: https://docs.rs/kurbo/0.5.4/kurbo/struct.Insets.html
Copy link
Member

Choose a reason for hiding this comment

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

👏

@xStrom
Copy link
Member Author

xStrom commented Mar 24, 2020

Yeah this is 99% docs orientated changes, as far as code is concerned all imports will continue to work as they did previously.

The only exception could be the druid_derive::Lens macro for which the export was moved from the widget module to the root to remove duplicated docs. However tests all seem to still be working, including one which uses this macro. In any case that's a one line change if it ends up needing to be reverted.

@xStrom xStrom merged commit e0a3c02 into linebender:master Mar 24, 2020
@xStrom xStrom removed the S-needs-review waits for review label May 15, 2020
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.

2 participants