[librsvg: 1/4] Documentation on how to add a new property

[Federico Mena Quintero <federico src gnome org>]

commit 7fc23e6323d731ce4b3ce917a3c79bb3e0d43bd4
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon Aug 24 19:42:35 2020 -0500

    Documentation on how to add a new property

 rsvg_internals/src/parsers.rs         |  3 ++-
 rsvg_internals/src/properties.rs      | 35 ++++++++++++++++++++++++++++-
 rsvg_internals/src/property_defs.rs   | 42 +++++++++++++++++++++++++++++++++++
 rsvg_internals/src/property_macros.rs | 34 +++++++++++++++++++++++-----
 4 files changed, 106 insertions(+), 8 deletions(-)
---
diff --git a/rsvg_internals/src/parsers.rs b/rsvg_internals/src/parsers.rs
index 9e9b794f..c9fb2e90 100644
--- a/rsvg_internals/src/parsers.rs
+++ b/rsvg_internals/src/parsers.rs
@@ -16,7 +16,7 @@ pub trait Parse: Sized {
     /// Convenience function to parse a value out of a `&str`.
     ///
     /// This is useful mostly for tests which want to avoid creating a
-    /// `cssparser::Parser` by hand.
+    /// `cssparser::Parser` by hand.  Property types do not need to reimplement this.
     fn parse_str(s: &str) -> Result<Self, ParseError> {
         let mut input = ParserInput::new(s);
         let mut parser = Parser::new(&mut input);
@@ -30,6 +30,7 @@ pub fn optional_comma<'i, 't>(parser: &mut Parser<'i, 't>) {
     let _ = parser.try_parse(|p| p.expect_comma());
 }
 
+/// Parses an `f32` and ensures that it is not an infinity or NaN.
 pub fn finite_f32(n: f32) -> Result<f32, ValueErrorKind> {
     if n.is_finite() {
         Ok(n)
diff --git a/rsvg_internals/src/properties.rs b/rsvg_internals/src/properties.rs
index 0a6e9781..3ed61438 100644
--- a/rsvg_internals/src/properties.rs
+++ b/rsvg_internals/src/properties.rs
@@ -1,4 +1,22 @@
 //! CSS properties, specified values, computed values.
+//!
+//! To implement support for a CSS property, do the following:
+//!
+//! * Create a type that will hold the property's values.  Please do this in the file
+//! `property_defs.rs`; you should cut-and-paste from the existing property definitions or
+//! read the documentation of the [`make_property`] macro.  You should read the
+//! documentation for the [`property_defs`] module to see all that is involved in creating
+//! a type for a property.
+//!
+//! * Modify the call to the `make_properties` macro in this module to include the new
+//! property's name.
+//!
+//! * Modify the rest of librsvg wherever the computed value of the property needs to be used.
+//! This is available in methods that take an argument of type [`ComputedValues`].
+//!
+//! [`make_property`]: ../macro.make_property.html
+//! [`property_defs`]: ../property_defs/index.html
+//! [`ComputedValues`]: ../struct.ComputedValues.html
 
 use cssparser::{
     self, BasicParseErrorKind, DeclarationListParser, ParseErrorKind, Parser, ParserInput, ToCss,
@@ -69,7 +87,7 @@ impl PropertyId {
     }
 }
 
-/// Holds the specified CSS properties for an element
+/// Holds the specified values for the CSS properties of an element.
 #[derive(Clone)]
 pub struct SpecifiedValues {
     indices: [u8; PropertyId::UnsetProperty as usize],
@@ -118,6 +136,20 @@ impl ComputedValues {
 ///
 /// See the only invocation of this macro to see how it is used; it is just
 /// a declarative list of property names.
+///
+/// **NOTE:** If you get a compiler error similar to this:
+///
+/// ```
+/// 362 |         "mix-blend-mode"              => mix_blend_mode              : MixBlendMode,
+///     |         ^^^^^^^^^^^^^^^^ no rules expected this token in macro call
+/// ```
+///
+/// Then it may be that you put the name inside the `longhands` block, when it should be
+/// inside the `longhands_not_supported_by_markup5ever` block.  This is because the
+/// [`markup5ever`] crate does not have predefined names for every single property out
+/// there; just the common ones.
+///
+/// [`markup5ever`]: https://docs.rs/markup5ever
 macro_rules! make_properties {
     {
         shorthands: {
@@ -187,6 +219,7 @@ macro_rules! make_properties {
             )+
         }
 
+        /// Holds the computed values for the CSS properties of an element.
         #[derive(Debug, Default, Clone)]
         pub struct ComputedValues {
             $(
diff --git a/rsvg_internals/src/property_defs.rs b/rsvg_internals/src/property_defs.rs
index b97993c8..67a96657 100644
--- a/rsvg_internals/src/property_defs.rs
+++ b/rsvg_internals/src/property_defs.rs
@@ -1,4 +1,46 @@
 //! Definitions for CSS property types.
+//!
+//! This module defines most of the CSS property types that librsvg supports.  Each
+//! property requires a Rust type that will hold its values, and that type should
+//! implement a few traits, as follows.
+//!
+//! # Requirements for a property type
+//!
+//! You should call the [`make_property`] macro to take care of most of these requirements
+//! automatically:
+//!
+//! * A name for the type.  For example, the `fill` property has a [`Fill`] type defined
+//! in this module.
+//!
+//! * An initial value per the CSS or SVG specs, given through an implementation of the
+//! `Default` trait.
+//!
+//! * Whether the property's computed value inherits to child elements, given
+//! through an implementation of the [`inherits_automatically`] method of the [`Property`]
+//! trait.
+//!
+//! * A way to derive the CSS *computed value* for the property, given through an
+//! implementation of the [`compute`] method of the [`Property`] trait.
+//!
+//! * The actual underlying type.  For example, the [`make_property`] macro can generate a
+//! field-less enum for properties like the `clip-rule` property, which just has
+//! identifier-based values like `nonzero` and `evenodd`.  For general-purpose types like
+//! [`Length`], the macro can wrap them in a newtype like `struct`
+//! [`StrokeWidth`]`(Length)`.  For custom types, the macro call can be used just to
+//! define the initial/default value and whether the property inherits automatically; you
+//! should provide the other required trait implementations separately.
+//!
+//! * An implementation of the [`Parse`] trait for the underlying type.
+//!
+//! [`compute`]: ../property_macros/trait.Property.html#tymethod.compute
+//! [`inherits_automatically`]: ../property_macros/trait.Property.html#tymethod.inherits_automatically
+//! [`Fill`]: struct.Fill.html
+//! [`Length`]: ../length/struct.Length.html
+//! [`make_property`]: ../macro.make_property.html
+//! [`Opacity`]: struct.Opacity.html
+//! [`Parse`]: ../trait.Parse.html
+//! [`Property`]: ../property_macros/trait.Property.html
+//! [`UnitInterval`]: ../unit_interval/struct.UnitInterval.html
 
 use cssparser::{Parser, Token};
 
diff --git a/rsvg_internals/src/property_macros.rs b/rsvg_internals/src/property_macros.rs
index 97e652cd..6e212d3f 100644
--- a/rsvg_internals/src/property_macros.rs
+++ b/rsvg_internals/src/property_macros.rs
@@ -1,7 +1,23 @@
 //! Macros to define CSS properties.
 
+/// Trait which all CSS property types should implement.
+///
+/// This is generic on `T` for testing purposes; in the actual code `T` needs to
+/// be [`ComputedValues`].
+///
+/// [`ComputedValues`]: ../properties/struct.ComputedValues.html
 pub trait Property<T> {
+    /// Whether the property's computed value inherits from parent to child elements.
+    ///
+    /// For each property, the CSS or SVG specs say whether the property inherits
+    /// automatically.  When a property is not specified in an element, the return value
+    /// of this method determines whether the property's value is copied from the parent
+    /// element (`true`), or whether it resets to the initial/default value (`false`).
     fn inherits_automatically() -> bool;
+
+    /// Derive the CSS computed value from the parent element's `ComputedValues` and the `self` value.
+    ///
+    /// The CSS or SVG specs say how to derive this for each property.
     fn compute(&self, _: &T) -> Self;
 }
 
@@ -11,15 +27,15 @@ pub trait Property<T> {
 ///
 /// * Define a type to represent the property's values.
 ///
-/// * A `Parse` implementation to parse the property.
+/// * A [`Parse`] implementation to parse the property.
 ///
 /// * A `Default` implementation to define the property's *initial* value.
 ///
-/// * A `Property` implementation to define whether the property
-/// inherits from the parent element, and how the property calculates
+/// * A [`Property`] implementation to define whether the property
+/// inherits from the parent element, and how the property derives
 /// its computed value.
 ///
-/// When going from `SpecifiedValues` to `ComputedValues, properties
+/// When going from [`SpecifiedValues`] to [`ComputedValues`], properties
 /// which inherit automatically from the parent element will just have
 /// their values cloned.  Properties which do not inherit will be reset back
 /// to their initial value (i.e. their `Default`).
@@ -50,8 +66,8 @@ pub trait Property<T> {
 /// );
 /// ```
 ///
-/// This generates a simple enum like the following, with implementations of `Parse`,
-/// `Default`, and `Property`.
+/// This generates a simple enum like the following, with implementations of [`Parse`],
+/// `Default`, and [`Property`].
 ///
 /// ```ignore
 /// pub enum StrokeLinejoin { Miter, Round, Bevel }
@@ -79,6 +95,12 @@ pub trait Property<T> {
 /// implentation of `Property::compute` that is more than a simple `clone`.  In this case,
 /// define the custom type separately, and use the macro to specify the default value and
 /// the `Property` implementation.
+///
+/// [`Parse`]: ../trait.Parse.html
+/// [`Property`]: ../property_macros/trait.Property.html
+/// [`ComputedValues`]: ../properties/struct.ComputedValues.html
+/// [`SpecifiedValues`]: ../properties/struct.SpecifiedValues.html
+///
 #[macro_export]
 macro_rules! make_property {
     ($computed_values_type: ty,