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

Convert documentation to intra doc links, add default whitepoint for Lab/Lch, make code fixups #190

Merged
merged 2 commits into from Nov 21, 2020

Conversation

okaneco
Copy link
Contributor

@okaneco okaneco commented Nov 19, 2020

Add default D65 WhitePoint to Lab and Lch
Collapse if else statements in color_difference.rs
Use !is_empty instead of len > 0 comparisons in gradient.rs
Use strip_prefix instead of manually stripping, avoids manual indexing
Remove extraneous clones, into_iters, and closures
Simplify matching on single condition into an if let
Remove format! in favor of to_string
Fix deprecated image functions in examples
Change some as casts to use direct from casts when applicable

Closes #177

Add default D65 WhitePoint to Lab and Lch
Collapse if else statements in color_difference.rs
Use !is_empty instead of `... > 0` comparisons in gradient.rs
Use strip_prefix instead of manually stripping, avoids manual indexing
Remove extraneous clones, into_iters, and closures
Simplify matching on single condition to if let
Remove format! in favor of to_string
Fix deprecated image functions in examples
Change some as casts to use direct `from` casts when applicable
@okaneco okaneco changed the title Convert documentation to intra doc links, make some fixups and clippy changes Convert documentation to intra doc links, add default whitepoint for Lab/Lch, make code fixups Nov 19, 2020
@okaneco
Copy link
Contributor Author

okaneco commented Nov 19, 2020

I noticed the white point issue after submitting the PR, it was a quick fix so I force pushed. The commits are separated by the intra doc changes and fixups for easier review.

Copy link
Owner

@Ogeon Ogeon left a comment

Choose a reason for hiding this comment

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

Oh, yes, this is great! I have been waiting for this feature for so long now. 😁 Time for even more cross-references in the future! And thanks a lot for the cleanup pass, too. Lots of nice changes there.

@@ -21,19 +21,19 @@ use crate::{
};

/// Linear HSL with an alpha component. See the [`Hsla` implementation in
/// `Alpha`](struct.Alpha.html#Hsla).
/// `Alpha`](crate::Alpha#Hsla).
Copy link
Owner

Choose a reason for hiding this comment

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

Oh, do anchors work too? That would be amazing!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and no 😅
That's using the span id= on the impl Alpha block for Hsl, so anchors work but they don't seem to be so fine-grained to refer to what we want.

///<span id="Hsla"></span>[`Hsla`](crate::Hsla) implementations.

In the Alpha doc, the anchor for the Hsl implementation is palette/struct.Alpha.html#impl-1.

I tried searching briefly but couldn't find anything about this. It might need to be suggested as an issue/feature request for rustdoc. It'd be great to have the anchor be the type name instead of impl-1, which would mean no more markdown/html spans.

Otherwise, it's an amazing feature and makes cross-linking so much easier. When you use a path that doesn't exist, you get something like a lint warning but the doc build still goes through. I used cargo doc --no-deps --no-default-features --features std with an optional --open to quickly iterate on builds.

Copy link
Owner

Choose a reason for hiding this comment

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

Alright, so it's basically the same old issue. 😄 I had even forgotten about it. It's good that they work at all! I don't know if it can be improved that much more in rustdoc, considering you can have multiple impl for types and type compositions. But I wouldn't complain if it did. 🙂

Anyway, let's merge.

@Ogeon
Copy link
Owner

Ogeon commented Nov 21, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 21, 2020

Build succeeded:

@bors bors bot merged commit 91ff1d4 into Ogeon:master Nov 21, 2020
@okaneco okaneco deleted the intradocs branch November 21, 2020 11:34
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.

Forgot default white point in alpha type aliases
2 participants