Review One: A Set Rules Story

1 files changed, 57 insertions, 19 deletions

diff --git a/src/eval/class.rs b/src/eval/class.rs
index 45674933..c4393b8a 100644
--- a/src/eval/class.rs
+++ b/src/eval/class.rs
@@ -6,14 +6,42 @@ use super::{Args, EvalContext, Node, Styles};
 use crate::diag::TypResult;
 use crate::util::EcoString;
 
-/// A class of nodes.
+/// A class of [nodes](Node).
+///
+/// You can [construct] an instance of a class in Typst code by invoking the
+/// class as a callable. This always produces some node, but not necessarily one
+/// of fixed type. For example, the `text` constructor does not actually create
+/// a [`TextNode`]. Instead it applies styling to whatever node you pass in and
+/// returns it structurally unchanged.
+///
+/// The arguments you can pass to a class constructor fall into two categories:
+/// Data that is inherent to the instance (e.g. the text of a heading) and style
+/// properties (e.g. the fill color of a heading). As the latter are often
+/// shared by many instances throughout a document, they can also be
+/// conveniently configured through class's [`set`] rule. Then, they apply to
+/// all nodes that are instantiated into the template where the `set` was
+/// executed.
+///
+/// ```typst
+/// This is normal.
+/// [
+///   #set text(weight: "bold")
+///   #set heading(fill: blue)
+///   = A blue & bold heading
+/// ]
+/// Normal again.
+/// ```
+///
+/// [construct]: Self::construct
+/// [`TextNode`]: crate::library::TextNode
+/// [`set`]: Self::set
 #[derive(Clone)]
 pub struct Class(Rc<Inner<dyn Bounds>>);
 
 /// The unsized structure behind the [`Rc`].
 struct Inner<T: ?Sized> {
     name: EcoString,
-    dispatch: T,
+    shim: T,
 }
 
 impl Class {
@@ -22,10 +50,10 @@ impl Class {
     where
         T: Construct + Set + 'static,
     {
-        Self(Rc::new(Inner {
-            name,
-            dispatch: Dispatch::<T>(PhantomData),
-        }))
+        // By specializing the shim to `T`, its vtable will contain T's
+        // `Construct` and `Set` impls (through the `Bounds` trait), enabling us
+        // to use them in the class's methods.
+        Self(Rc::new(Inner { name, shim: Shim::<T>(PhantomData) }))
     }
 
     /// The name of the class.
@@ -34,13 +62,22 @@ impl Class {
     }
 
     /// Construct an instance of the class.
+    ///
+    /// This parses both property and data arguments (in this order) and styles
+    /// the node constructed from the data with the style properties.
     pub fn construct(&self, ctx: &mut EvalContext, args: &mut Args) -> TypResult<Node> {
-        self.0.dispatch.construct(ctx, args)
+        let mut styles = Styles::new();
+        self.set(args, &mut styles)?;
+        let node = self.0.shim.construct(ctx, args)?;
+        Ok(node.styled(styles))
     }
 
     /// Execute the class's set rule.
-    pub fn set(&self, styles: &mut Styles, args: &mut Args) -> TypResult<()> {
-        self.0.dispatch.set(styles, args)
+    ///
+    /// This parses property arguments and writes the resulting styles into the
+    /// given style map. There are no further side effects.
+    pub fn set(&self, args: &mut Args, styles: &mut Styles) -> TypResult<()> {
+        self.0.shim.set(args, styles)
     }
 }
 
@@ -54,7 +91,8 @@ impl Debug for Class {
 
 impl PartialEq for Class {
     fn eq(&self, other: &Self) -> bool {
-        // We cast to thin pointers for comparison.
+        // We cast to thin pointers for comparison because we don't want to
+        // compare vtables (there can be duplicate vtables across codegen units).
         std::ptr::eq(
             Rc::as_ptr(&self.0) as *const (),
             Rc::as_ptr(&other.0) as *const (),
@@ -75,19 +113,19 @@ pub trait Construct {
 pub trait Set {
     /// Parse the arguments and insert style properties of this class into the
     /// given style map.
-    fn set(styles: &mut Styles, args: &mut Args) -> TypResult<()>;
+    fn set(args: &mut Args, styles: &mut Styles) -> TypResult<()>;
 }
 
-/// Zero-sized struct whose vtable contains the constructor and set rule of a
-/// class.
-struct Dispatch<T>(PhantomData<T>);
-
+/// Rewires the operations available on a class in an object-safe way. This is
+/// only implemented by the zero-sized `Shim` struct.
 trait Bounds {
     fn construct(&self, ctx: &mut EvalContext, args: &mut Args) -> TypResult<Node>;
-    fn set(&self, styles: &mut Styles, args: &mut Args) -> TypResult<()>;
+    fn set(&self, args: &mut Args, styles: &mut Styles) -> TypResult<()>;
 }
 
-impl<T> Bounds for Dispatch<T>
+struct Shim<T>(PhantomData<T>);
+
+impl<T> Bounds for Shim<T>
 where
     T: Construct + Set,
 {
@@ -95,7 +133,7 @@ where
         T::construct(ctx, args)
     }
 
-    fn set(&self, styles: &mut Styles, args: &mut Args) -> TypResult<()> {
-        T::set(styles, args)
+    fn set(&self, args: &mut Args, styles: &mut Styles) -> TypResult<()> {
+        T::set(args, styles)
     }
 }