Merge #171

171: Remove remaining extern proc_macro, remove `strict` feature, and update documentation r=Ogeon a=okaneco

Remove `extern crate proc_macro` in `palette_derive`
Remove `strict` feature which denies warnings and missing docs
Add `RUSTFLAGS="-D warnings"` flag for CI to replace `strict`
Add documentation to show raw color type usage
Update CONTRIBUTING.md with some grammar fixes and remove unnecessary style sections

Closes #129 

Co-authored-by: okaneco <47607823+okaneco@users.noreply.github.com>

.travis.yml

@@ -7,6 +7,8 @@ rust:
   - stable
   - beta
   - nightly
+env:
+  - RUSTFLAGS="-D warnings"
 os:
   - linux
   - osx
@@ -18,11 +20,11 @@ branches:
     - /^\d+\.\d+$/
 script:
   - cd palette_derive
-  - cargo build -v --features strict
+  - cargo build -v
 
   - cd ../palette
-  - cargo build -v --features strict
-  - cargo test -v --features strict
+  - cargo build -v
+  - cargo test -v
   - bash ../scripts/test_features.sh
 
   - if [ "$TRAVIS_RUST_VERSION" = "nightly" ]; then

CONTRIBUTING.md

@@ -1,34 +1,35 @@
 # Contributing
 
-All sorts of contributions are welcome, no matter how huge or tiny. Just fork
+All sorts of contributions are welcome, no matter how large or small. Just fork
 the repository, implement your change and make a pull request. Don't worry if
 you are not sure how to implement the change, or if it's not yet done. You can
-still make an work-in-progress pull request where we can discuss it.
+still make a work-in-progress pull request where we can discuss it.
 
-Got a new shiny feature in store? Remember to explain it thoroughly and
-motivate why it should be added. This makes it easier to review and for
+Have a shiny new feature in store? Remember to explain it thoroughly and provide
+motivation for why it should be added. This makes it easier to review and for
 everyone to follow your reasoning.
 
 ## Testing
 
-Every pull request is automatically tested with the `strict` feature enables,
-so it's a good idea to also run your local tests with it, like this: `cargo
-test --features strict`. This will make sure that there are no warnings or
-missing documentation. All for the benefit of the user.
+Every pull request is automatically tested with continuous integration to deny
+warnings and any missing documentation. It's a good idea to run your local tests
+with `RUSTFLAGS="-D warnings" cargo test` and also to run `cargo check` and 
+`cargo build` with the compiler flag prepended. This will make sure that there
+are no warnings or missing documentation, all for the benefit of the user.
 
-There are also a number of examples in the `examples` directory, that should
-check that demonstrates things in a more "real" application. The output of
-these should also be checked if they may be affected by the change.
+There are a number of programs in the `examples` directory that that
+demonstrate applications of the library in more "real" code. The output of
+these should be checked to see if they are affected by changes made.
 `readme_examples.rs` shall contain the same code as in the `README.md`
 examples, and its output images shall be copied to the `gfx` directory (`cp
 examples/readme_*.png gfx`) if they have changed.
 
 ### Unit Tests
 
-New features should include appropriate unit tests to prevent future buggs.
-This is especially important when it comes to dynamic things, like propper
-conversion or valitation. The uni tests should be placed in a `test` module,
-in the same module as the code that is being tested. For example:
+New features should include appropriate unit tests to prevent future bugs.
+This is especially important when it comes to dynamic things, like proper
+conversion or data validation. The unit tests should be placed in a `test`
+module located within the same module as the code being tested. For example:
 
 ```rust
 struct Person {
@@ -64,11 +65,11 @@ case may, of course, be expanded to check for more than just the reported case.
 ## Commits
 
 Commits shall preferably be small and not contain too many different changes.
-This makes them easier to cherry pick if that happens to be necessary. The
-actual size of the commit depends on the change, so "follow your heart", but
-it's preferable to make sure that all the tests passes before committing.
+This makes them easier to cherry pick if necessary. The actual size of the
+commit depends on the change, so "follow your heart", but it's preferable to
+make sure that all the tests pass before committing.
 
-The commit messages themselves doesn't have to have any particular formatting
+The commit messages themselves don't need to have any particular formatting
 or syntax (except English). Just make them short, _descriptive_ (!), and tidy.
 Not like this:
 
@@ -87,18 +88,18 @@ them in past tense.
 ## Pull Requests
 
 The header of a pull request should follow the same rules. It should be short
-and describe the changes as good as possible. The PR description (or initial
+and describe the changes as well as possible. The PR description (or initial
 comment, depending on how you view it) should contain a relatively detailed
 description of the changes. Someone who doesn't know what's up should be able
 to look at it and understand what has changed. No secrets or surprises, even
 if they may be fun.
 
-Pull requests that closes issues has to mention it in the description. A
+Pull requests that close issues need to mention it in the description. A
 closed issue should be mentioned as "fixes #123", "closes #123", or [something
 similar][closing_commits]. This closes the issues automatically when the pull
 request is merged.
 
-Pull requests that breaks backwards compatibility should say so in the end of
+Pull requests that break backwards compatibility should say so in the end of
 the description, to make sure it's easy to find.
 
 Here is an example PR:
@@ -118,163 +119,16 @@ It's not much harder than that, depending on the size of the contribution.
 
 ## Code Style
 
-The code style is generally in line with the common guidelines for Rust. There is a `rustfmt.toml` in the project, so feel free to use automatic formatting. If you prefer not to, the following sections can be used as a style guide.
-
-### Braces
-
-Opening braces should be placed on the same line as the control structure it
-belongs to. `else` should be on the same line as the closing brace of `if`.
-
-```rust
-while x {
-
-}
-
-if a {
-
-} else if b {
-
-} else {
-
-}
-
-{
-    do_stuff_in_scope();
-}
-```
-
-### Indentation
-
-This rule is simple: _always_ indent with _four spaces_, and _only one step_
-per "level". No tabs, not two spaces, no visual indentation. The reasons for
-this are:
-
-1. Spaces are the Rust standard. Spaces are also predictable, so no surprise rightwards drift.
-2. Fixed length indentation is less confusing for text editors. Some of them can easily be thrown off by irregular indentation lengths.
-3. It's GIT friendly. Visual indentation depends on variable name lengths and other arbitrary things, so changing one of those things will affect more lines than necessary and cause noisy diffs.
+The code style is generally in line with the common guidelines for Rust. Running
+`rustfmt` or `cargo fmt` before committing is recommended.
 
 ### Long Lines
 
-The recommended line length is somewhere around 80 to 100 characters, but
-longer is allowed if it doesn't hurt readability. These are some guidelines
-for how to deal with long lines.
+The recommended line length is somewhere around 80 to 100 characters.
 
-#### Comments And Text
+#### Documentation, Comments, And Text
 
 Comments and the text in Markdown files are usually capped to 80 characters.
-This can be done automatically in some editors, like SublimeText (Alt+Q). It
+This can be done automatically in some editors like SublimeText (Alt+Q). Some
+editors allow for visual rulers to indicate an 80 character width. It
 doesn't hurt if this isn't followed as a law. It's just a guideline.
-
-#### Call Chains
-
-Long call chains can be broken up into multiple lines. Each call should then
-be placed on its own line, and indented one step. It's as if they are placed
-in a sub-scope. It's still ok, and often good, to have one method call on the
-first line:
-
-```rust
-let b: Vec<_> = a.iter().map(From::from).filter(|x| x.prop() > 0.0).skip(10).take(100).collect();
-
-//becomes
-
-let b: Vec<_> = a.iter()
-    .map(From::from)
-    .filter(|x| x.prop() > 0.0)
-    .skip(10)
-    .take(100)
-    .collect();
-
-//or
-
-let b: Vec<_> = a.iter()
-    .map(From::from)
-    .filter(|x| { //notice the braces and indentation
-        x.prop() > 0.0
-    })
-    .skip(10)
-    .take(100)
-    .collect();
-```
-
-#### Function Signatures
-
-Function signatures poses a risk of becoming very long. Especially when type
-parameters are involved. Spreading them out over multiple lines follows the
-same rules as above: everything is a scope, so everything is indented only
-_one_ step. Here is a demo function and multiple stages of making it vertical:
-
-```rust
-fn long_function_with_a_long_name<'a, A: ?Sized + 'a, B: 'a, C, D>(abacus: &'a A, botanical: &'a B, culinary: Arc<Mutex<C>>, dependency: (D, D)) -> (D, D) where B: AsRef<A>, C: DerefMut, <C as Deref>::Target: RefMut<A> {
-    //...
-}
-
-//`where` can be seen as an opening brace:
-fn long_function_with_a_long_name<'a, A: ?Sized + 'a, B: 'a, C, D>(abacus: &'a A, botanical: &'a B, culinary: Arc<Mutex<C>>, dependency: (D, D)) -> (D, D) where
-    B: AsRef<A>,
-    C: DerefMut,
-    <C as Deref>::Target: RefMut<A>,
-{
-    //...
-}
-
-//The argument list can be reformatted like the where clauses:
-fn long_function_with_a_long_name<'a, A: ?Sized + 'a, B: 'a, C, D>(
-    abacus: &'a A,
-    botanical: &'a B,
-    culinary: Arc<Mutex<C>>,
-    dependency: (D, D),
-) -> (D, D) where
-    B: AsRef<A>,
-    C: DerefMut,
-    <C as Deref>::Target: RefMut<A>,
-{
-    //...
-}
-
-//This is quite extreme, but it follows the same rules:
-fn long_function_with_a_long_name<
-    'a,
-    A: ?Sized + 'a,
-    B: 'a,
-    C,
-    D
->(
-    abacus: &'a A,
-    botanical: &'a B,
-    culinary: Arc<Mutex<C>>,
-    dependency: (D, D),
-) -> (D, D) where
-    B: AsRef<A>,
-    C: DerefMut,
-    <C as Deref>::Target: RefMut<A>,
-{
-    //...
-}
-```
-
-Notice how each section (or list) is clearly separated from each other. This
-way of rewriting a function signature is, of course, just a guideline and
-modifications are allowed for the sake of readability, but that should
-generally not be necessary.
-
-#### Input Arguments
-
-The rules for input arguments are the same as above. Opening and closing
-parenthesis, brackets and braces are separate from the content, and the content is
-indented _one step_ only:
-
-```rust
-do_something_cool([abacus, botanical, culinary, dependency], |x| x.prop() > 0.0);
-
-//becomes
-
-do_something_cool(
-    [
-        abacus,
-        botanical,
-        culinary,
-        dependency,
-    ],
-    |x| x.prop() > 0.0,
-);
-```

README.md

@@ -118,6 +118,27 @@ The RGB gradient goes through gray, while the HSV gradients only change hue:
 
 ![Gradient Comparison](gfx/readme_gradients.png)
 
+### Working with Raw Color Types
+
+Palette supports converting from a raw buffer of data into a color type using the `Pixel` trait. This is useful for interoperation with other crates or programs.
+
+Oftentimes, pixel data is stored in a raw buffer such as a `[u8; 3]`. `from_raw` can be used to convert into a Palette color, `into_format` converts from  `Srgb<u8>` to `Srgb<f32>`, and finally `into_raw` to convert from a Palette color back to a `[u8;3]`.
+
+Here's an example of turning a buffer of `[u8; 3]` into a Palette `Srgb` color and back to a raw buffer.
+```rust
+use approx::assert_relative_eq;
+use palette::{Srgb, Pixel};
+
+let buffer = [255, 0, 255];
+let raw = Srgb::from_raw(&buffer);
+assert_eq!(raw, &Srgb::<u8>::new(255u8, 0, 255));
+
+let raw_float: Srgb<f32> = raw.into_format();
+assert_relative_eq!(raw_float, Srgb::new(1.0, 0.0, 1.0));
+
+let raw: [u8; 3] = Srgb::into_raw(raw_float.into_format());
+assert_eq!(raw, buffer);
+
 ## What It Isn't
 
 This library is only meant for color manipulation and conversion. It's not a fully featured image manipulation library. It will only handle colors, and not whole images. There are features that are meant to work as bridges between Palette and other graphical libraries, but the main features are limited to only focus on single pixel operations, to keep the scope at a manageable size.

palette/Cargo.toml

@@ -23,7 +23,6 @@ serializing = ["serde", "std"]
 #ignore in feature test
 std = ["approx/std", "num-traits/std"]
 libm = ["num-traits/libm"]
-strict = [] # Only for CI internal testing
 
 [dependencies]
 palette_derive = {version = "0.5.0", path = "../palette_derive"}

palette/src/encoding/pixel/mod.rs

@@ -116,7 +116,7 @@ pub unsafe trait Pixel<T>: Sized {
         unsafe { P::from_raw_parts_mut(self as *mut Self as *mut T, Self::CHANNELS) }
     }
 
-    /// Convert from raw color components.
+    /// Convert into raw color components.
     #[inline]
     fn into_raw<P: RawPixelSized<T>>(self) -> P {
         assert_eq!(P::CHANNELS, Self::CHANNELS);

palette/src/lib.rs

@@ -133,12 +133,35 @@
 //! When the desired processing is done, it's time to encode the colors back
 //! into some image format. The same rules applies as for the decoding, but the
 //! process reversed.
+//!
+//! # Working with Raw Data
+//!
+//! Oftentimes, pixel data is stored in a raw buffer such as a `[u8; 3]`. The
+//! [`Pixel`](encoding/pixel/trait.Pixel.html) trait allows for easy interoperation between
+//! Palette colors and other crates or systems. `from_raw` can be used to
+//! convert into a Palette color, `into_format` converts from  `Srgb<u8>` to
+//! `Srgb<f32>`, and finally `into_raw` to convert from a Palette color back to
+//! a `[u8;3]`.
+//!
+//! ```rust
+//! use approx::assert_relative_eq;
+//! use palette::{Srgb, Pixel};
+//!
+//! let buffer = [255, 0, 255];
+//! let raw = Srgb::from_raw(&buffer);
+//! assert_eq!(raw, &Srgb::<u8>::new(255u8, 0, 255));
+//!
+//! let raw_float: Srgb<f32> = raw.into_format();
+//! assert_relative_eq!(raw_float, Srgb::new(1.0, 0.0, 1.0));
+//!
+//! let raw: [u8; 3] = Srgb::into_raw(raw_float.into_format());
+//! assert_eq!(raw, buffer);
+//! ```
 
 // Keep the standard library when running tests, too
 #![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
 #![doc(html_root_url = "https://docs.rs/palette/0.5.0/palette/")]
-#![cfg_attr(feature = "strict", deny(missing_docs))]
-#![cfg_attr(feature = "strict", deny(warnings))]
+#![warn(missing_docs)]
 
 #[cfg(any(feature = "std", test))]
 extern crate core;

palette/src/rgb/packed.rs

@@ -167,7 +167,6 @@ where
 
 #[cfg(test)]
 mod test {
-    // use crate::rgb::{Abgr, Argb, Bgra, Rgba};
     use crate::rgb::packed::channels::{Abgr, Argb, Bgra, Rgba};
     use crate::{Packed, Srgb, Srgba};
 

palette_derive/Cargo.toml

@@ -20,5 +20,3 @@ quote = "^1.0"
 proc-macro2 = "^1.0"
 
 [features]
-#internal
-strict = []

palette_derive/src/lib.rs

@@ -1,10 +1,7 @@
 //! Derives traits from the [palette](https://crates.io/crates/palette) crate.
 
-#![cfg_attr(feature = "strict", deny(warnings))]
 #![recursion_limit = "128"]
 
-extern crate proc_macro;
-
 use proc_macro::TokenStream;
 
 mod convert;

scripts/test_features.sh

@@ -4,7 +4,7 @@ set -e
 features=""
 
 #Features that will always be activated
-required_features="std strict"
+required_features="std"
 
 
 #Find features