Apply simple suggestions from code review

Co-Authored-By: Laurenz <laurmaedje@gmail.com>

7 files changed, 32 insertions, 22 deletions

diff --git a/src/eval/node.rs b/src/eval/node.rs
index d5b67adb..43cb906b 100644
--- a/src/eval/node.rs
+++ b/src/eval/node.rs
@@ -354,8 +354,8 @@ impl Packer {
 
             // Take the flow and erase any styles that will be inherited anyway.
             let Builder { mut children, styles, .. } = mem::take(&mut self.flow);
-            for child in &mut children {
-                child.styles_mut().map(|s| s.erase(&styles));
+            for local in children.iter_mut().filter_map(FlowChild::styles_mut) {
+                local.erase(&styles);
             }
 
             let flow = FlowNode(children).pack();
diff --git a/src/library/columns.rs b/src/library/columns.rs
index 88ad8172..3109eda3 100644
--- a/src/library/columns.rs
+++ b/src/library/columns.rs
@@ -3,12 +3,11 @@ use super::ParNode;
 
 /// `columns`: Stack children along an axis.
 pub fn columns(_: &mut EvalContext, args: &mut Args) -> TypResult<Value> {
-    let count = args.expect("column count")?;
+    let columns = args.expect("column count")?;
     let gutter = args.named("gutter")?.unwrap_or(Relative::new(0.04).into());
     let body: Node = args.expect("body")?;
-
     Ok(Value::block(ColumnsNode {
-        columns: count,
+        columns,
         gutter,
         child: body.into_block(),
     }))
@@ -23,7 +22,7 @@ pub fn colbreak(_: &mut EvalContext, _: &mut Args) -> TypResult<Value> {
 #[derive(Debug, Hash)]
 pub struct ColumnsNode {
     /// How many columns there should be.
-    pub columns: usize,
+    pub columns: NonZeroUsize,
     /// The size of the gutter space between each column.
     pub gutter: Linear,
     /// The child to be layouted into the columns. Most likely, this should be a
@@ -50,11 +49,10 @@ impl Layout for ColumnsNode {
         // `region.backlog`.
         let mut sizes = vec![];
 
-        // Assure there is at least one column.
-        let columns = self.columns.max(1);
+        let columns = self.columns.get();
 
         for (current, base) in std::iter::once((regions.current, regions.base))
-            .chain(regions.backlog.clone().into_iter().map(|s| (s, s)))
+            .chain(regions.backlog.as_slice().iter().map(|&s| (s, s)))
         {
             let gutter = self.gutter.resolve(base.x);
             gutters.push(gutter);
@@ -68,8 +66,9 @@ impl Layout for ColumnsNode {
         }
 
         let first = sizes.remove(0);
-        let mut col_regions = Regions::one(first, first, regions.expand);
-        col_regions.backlog = sizes.clone().into_iter();
+        let mut pod =
+            Regions::one(first, Spec::new(first.x, regions.base.y), regions.expand);
+        pod.backlog = sizes.clone().into_iter();
 
         // We have to treat the last region separately.
         let last_column_gutter = regions.last.map(|last| {
@@ -78,11 +77,11 @@ impl Layout for ColumnsNode {
                 (last.x - gutter * (columns - 1) as f64) / columns as f64,
                 last.y,
             );
-            col_regions.last = Some(size);
+            pod.last = Some(size);
             (size, gutter)
         });
 
-        let frames = self.child.layout(ctx, &col_regions);
+        let frames = self.child.layout(ctx, &pod);
         let dir = ctx.styles.get(ParNode::DIR);
 
         // Dealing with infinite height areas here.
diff --git a/src/library/flow.rs b/src/library/flow.rs
index ca730db1..6bfa3ddd 100644
--- a/src/library/flow.rs
+++ b/src/library/flow.rs
@@ -32,12 +32,12 @@ impl Debug for FlowNode {
 pub enum FlowChild {
     /// A paragraph/block break.
     Break(Styles),
-    /// Skip the rest of the region and move to the next.
-    Skip,
     /// Vertical spacing between other children.
     Spacing(SpacingNode),
     /// An arbitrary node.
     Node(PackedNode),
+    /// Skip the rest of the region and move to the next.
+    Skip,
 }
 
 impl FlowChild {
@@ -73,7 +73,7 @@ impl Debug for FlowChild {
             }
             Self::Spacing(node) => node.fmt(f),
             Self::Node(node) => node.fmt(f),
-            Self::Skip => write!(f, "Skip"),
+            Self::Skip => f.pad("Skip"),
         }
     }
 }
diff --git a/src/library/mod.rs b/src/library/mod.rs
index 313d423b..1c97f529 100644
--- a/src/library/mod.rs
+++ b/src/library/mod.rs
@@ -26,6 +26,7 @@ mod utility;
 /// Helpful imports for creating library functionality.
 mod prelude {
     pub use std::fmt::{self, Debug, Formatter};
+    pub use std::num::NonZeroUsize;
     pub use std::rc::Rc;
 
     pub use typst_macros::properties;
@@ -172,6 +173,15 @@ castable! {
 }
 
 castable! {
+    prelude::NonZeroUsize,
+    Expected: "positive integer",
+    Value::Int(int) => int
+        .try_into()
+        .and_then(|n: usize| n.try_into())
+        .map_err(|_| "must be positive")?,
+}
+
+castable! {
     String,
     Expected: "string",
     Value::Str(string) => string.into(),
diff --git a/src/library/page.rs b/src/library/page.rs
index 6585edb9..100b4d0c 100644
--- a/src/library/page.rs
+++ b/src/library/page.rs
@@ -41,7 +41,7 @@ impl PageNode {
     /// The page's background color.
     pub const FILL: Option<Paint> = None;
     /// How many columns the page has.
-    pub const COLUMNS: usize = 1;
+    pub const COLUMNS: NonZeroUsize = NonZeroUsize::new(1).unwrap();
     /// How many columns the page has.
     pub const COLUMN_GUTTER: Linear = Relative::new(0.04).into();
 }
@@ -119,11 +119,11 @@ impl PageNode {
         };
 
         let columns = ctx.styles.get(Self::COLUMNS);
-        let child = if ctx.styles.get(Self::COLUMNS) > 1 {
+        let child = if columns.get() > 1 {
             ColumnsNode {
-                child: self.child.clone(),
                 columns,
                 gutter: ctx.styles.get(Self::COLUMN_GUTTER),
+                child: self.child.clone(),
             }
             .pack()
         } else {
diff --git a/tests/ref/layout/columns.png b/tests/ref/layout/columns.png
index a1aee61c..a9e7f661 100644
--- a/tests/ref/layout/columns.png
+++ b/tests/ref/layout/columns.png
diff --git a/tests/typ/layout/columns.typ b/tests/typ/layout/columns.typ
index b6019a8a..ed0eb5d6 100644
--- a/tests/typ/layout/columns.typ
+++ b/tests/typ/layout/columns.typ
@@ -48,6 +48,7 @@ a page for a test but it does get the job done.
 إلى جانب كل من البروتينات والليبيدات والسكريات المتعددة
 #rect(fill: eastern, height: 8pt, width: 6pt)
 الجزيئات الضخمة الأربعة الضرورية للحياة. 
+
 ---
 // Test the `colbreak` function.
 #set page(height: 1cm, width: 7.05cm, columns: 2)
@@ -69,7 +70,7 @@ C
 // Test columns when one of them is empty.
 #set page(width: auto, columns: 3)
 
-The page can grow as much as it wants horizontally.
+Arbitrary horizontal growth.
 
 ---
 // Test columns in an infinitely wide frame.
@@ -91,7 +92,7 @@ This is a normal page. Very normal.
 
 ---
 // Test a page with zero columns.
+// Error: 49-50 must be positive
 #set page(height: auto, width: 7.05cm, columns: 0)
 
-This makes less sense but will still
-produce a normal page.
+This makes less sense.