Fs builder methods + tidy up

bench/src/clock.rs

@@ -4,6 +4,7 @@ use std::rc::Rc;
 
 use criterion::{criterion_group, criterion_main, Criterion};
 
+use typst::diag::Pass;
 use typst::eval::{eval, Module};
 use typst::exec::exec;
 use typst::export::pdf;
@@ -18,18 +19,13 @@ const TYP_DIR: &str = "../tests/typ";
 const CASES: &[&str] = &["coma.typ", "text/basic.typ"];
 
 fn benchmarks(c: &mut Criterion) {
-    let loader = {
+    let loader = FsLoader::new().with_path(FONT_DIR).wrap();
-        let mut loader = FsLoader::new();
-        loader.search_path(FONT_DIR);
-        Rc::new(loader)
-    };
-
     let ctx = Rc::new(RefCell::new(Context::new(loader.clone())));
 
     for case in CASES {
         let path = Path::new(TYP_DIR).join(case);
         let name = path.file_stem().unwrap().to_string_lossy();
-        let src_id = loader.resolve_path(&path).unwrap();
+        let src_id = loader.resolve(&path).unwrap();
         let src = std::fs::read_to_string(&path).unwrap();
         let case = Case::new(src_id, src, ctx.clone());
 
@@ -115,21 +111,24 @@ impl Case {
         parse(&self.src).output
     }
 
-    fn eval(&self) -> Module {
+    fn eval(&self) -> Pass<Module> {
-        let mut borrowed = self.ctx.borrow_mut();
+        eval(
-        eval(&mut borrowed, self.src_id, Rc::clone(&self.ast)).output
+            &mut self.ctx.borrow_mut(),
+            self.src_id,
+            Rc::clone(&self.ast),
+        )
     }
 
-    fn exec(&self) -> LayoutTree {
+    fn exec(&self) -> Pass<LayoutTree> {
-        exec(&mut self.ctx.borrow_mut(), &self.module.template).output
+        exec(&mut self.ctx.borrow_mut(), &self.module.template)
     }
 
     fn layout(&self) -> Vec<Rc<Frame>> {
         layout(&mut self.ctx.borrow_mut(), &self.tree)
     }
 
-    fn typeset(&self) -> Vec<Rc<Frame>> {
+    fn typeset(&self) -> Pass<Vec<Rc<Frame>>> {
-        self.ctx.borrow_mut().typeset(self.src_id, &self.src).output
+        self.ctx.borrow_mut().typeset(self.src_id, &self.src)
     }
 
     fn pdf(&self) -> Vec<u8> {

src/eval/mod.rs

@@ -36,8 +36,8 @@ use crate::syntax::*;
 use crate::Context;
 
 /// Evaluate a parsed source file into a module.
-pub fn eval(ctx: &mut Context, location: FileId, ast: Rc<SyntaxTree>) -> Pass<Module> {
+pub fn eval(ctx: &mut Context, file: FileId, ast: Rc<SyntaxTree>) -> Pass<Module> {
-    let mut ctx = EvalContext::new(ctx, location);
+    let mut ctx = EvalContext::new(ctx, file);
     let template = ast.eval(&mut ctx);
     let module = Module { scope: ctx.scopes.top, template };
     Pass::new(module, ctx.diags)
@@ -70,13 +70,13 @@ pub struct EvalContext<'a> {
 
 impl<'a> EvalContext<'a> {
     /// Create a new evaluation context.
-    pub fn new(ctx: &'a mut Context, location: FileId) -> Self {
+    pub fn new(ctx: &'a mut Context, file: FileId) -> Self {
         Self {
             loader: ctx.loader.as_ref(),
             images: &mut ctx.images,
             scopes: Scopes::new(Some(&ctx.std)),
             diags: DiagSet::new(),
-            route: vec![location],
+            route: vec![file],
             modules: HashMap::new(),
         }
     }

src/lib.rs

@@ -96,8 +96,8 @@ impl Context {
 
     /// Typeset a source file into a collection of layouted frames.
     ///
-    /// The `file` is the file id of the source file and is used to resolve
+    /// The `file` identifies the source file and is used to resolve relative
-    /// relative paths (for importing and image loading).
+    /// paths (for importing and image loading).
     ///
     /// Returns a vector of frames representing individual pages alongside
     /// diagnostic information (errors and warnings).
@@ -125,15 +125,14 @@ pub struct ContextBuilder {
 }
 
 impl ContextBuilder {
-    /// The scope containing definitions that are available everywhere,
+    /// The scope containing definitions that are available everywhere
     /// (the standard library).
     pub fn std(mut self, std: Scope) -> Self {
         self.std = Some(std);
         self
     }
 
-    /// The `state` defining initial properties for page size, font selection
+    /// The initial properties for page size, font selection and so on.
-    /// and so on.
     pub fn state(mut self, state: State) -> Self {
         self.state = Some(state);
         self

src/loading/fs.rs

@@ -3,6 +3,7 @@ use std::collections::HashMap;
 use std::fs::{self, File};
 use std::io;
 use std::path::{Path, PathBuf};
+use std::rc::Rc;
 
 use memmap2::Mmap;
 use same_file::Handle;
@@ -28,58 +29,27 @@ impl FsLoader {
         Self { faces: vec![], paths: RefCell::default() }
     }
 
-    /// Resolve a file id for a path.
+    /// Builder-style variant of `search_system`.
-    pub fn resolve_path(&self, path: &Path) -> io::Result<FileId> {
+    pub fn with_system(mut self) -> Self {
-        let file = File::open(path)?;
+        self.search_system();
-        let meta = file.metadata()?;
+        self
-        if meta.is_file() {
-            let handle = Handle::from_file(file)?;
-            let id = FileId(fxhash::hash64(&handle));
-            self.paths.borrow_mut().insert(id, path.normalize());
-            Ok(id)
-        } else {
-            Err(io::Error::new(io::ErrorKind::Other, "not a file"))
-        }
     }
 
-    /// Search for fonts in the operating system's font directories.
+    /// Builder-style variant of `search_path`.
-    #[cfg(all(unix, not(target_os = "macos")))]
+    pub fn with_path(mut self, dir: impl AsRef<Path>) -> Self {
-    pub fn search_system(&mut self) {
-        self.search_path("/usr/share/fonts");
-        self.search_path("/usr/local/share/fonts");
-
-        if let Some(dir) = dirs::font_dir() {
         self.search_path(dir);
+        self
     }
+
+    /// Builder-style method to wrap the loader in an [`Rc`] to make it usable
+    /// with the [`Context`](crate::Context).
+    pub fn wrap(self) -> Rc<Self> {
+        Rc::new(self)
     }
 
     /// Search for fonts in the operating system's font directories.
-    #[cfg(target_os = "macos")]
     pub fn search_system(&mut self) {
-        self.search_path("/Library/Fonts");
+        self.search_system_impl();
-        self.search_path("/Network/Library/Fonts");
-        self.search_path("/System/Library/Fonts");
-
-        if let Some(dir) = dirs::font_dir() {
-            self.search_path(dir);
-        }
-    }
-
-    /// Search for fonts in the operating system's font directories.
-    #[cfg(windows)]
-    pub fn search_system(&mut self) {
-        let windir =
-            std::env::var("WINDIR").unwrap_or_else(|_| "C:\\Windows".to_string());
-
-        self.search_path(Path::new(&windir).join("Fonts"));
-
-        if let Some(roaming) = dirs::config_dir() {
-            self.search_path(roaming.join("Microsoft\\Windows\\Fonts"));
-        }
-
-        if let Some(local) = dirs::cache_dir() {
-            self.search_path(local.join("Microsoft\\Windows\\Fonts"));
-        }
     }
 
     /// Search for all fonts at a path.
@@ -108,6 +78,57 @@ impl FsLoader {
         }
     }
 
+    /// Resolve a file id for a path.
+    pub fn resolve(&self, path: &Path) -> io::Result<FileId> {
+        let file = File::open(path)?;
+        let meta = file.metadata()?;
+        if meta.is_file() {
+            let handle = Handle::from_file(file)?;
+            let id = FileId(fxhash::hash64(&handle));
+            self.paths.borrow_mut().insert(id, path.normalize());
+            Ok(id)
+        } else {
+            Err(io::Error::new(io::ErrorKind::Other, "not a file"))
+        }
+    }
+
+    #[cfg(all(unix, not(target_os = "macos")))]
+    fn search_system_impl(&mut self) {
+        self.search_path("/usr/share/fonts");
+        self.search_path("/usr/local/share/fonts");
+
+        if let Some(dir) = dirs::font_dir() {
+            self.search_path(dir);
+        }
+    }
+
+    #[cfg(target_os = "macos")]
+    fn search_system_impl(&mut self) {
+        self.search_path("/Library/Fonts");
+        self.search_path("/Network/Library/Fonts");
+        self.search_path("/System/Library/Fonts");
+
+        if let Some(dir) = dirs::font_dir() {
+            self.search_path(dir);
+        }
+    }
+
+    #[cfg(windows)]
+    fn search_system_impl(&mut self) {
+        let windir =
+            std::env::var("WINDIR").unwrap_or_else(|_| "C:\\Windows".to_string());
+
+        self.search_path(Path::new(&windir).join("Fonts"));
+
+        if let Some(roaming) = dirs::config_dir() {
+            self.search_path(roaming.join("Microsoft\\Windows\\Fonts"));
+        }
+
+        if let Some(local) = dirs::cache_dir() {
+            self.search_path(local.join("Microsoft\\Windows\\Fonts"));
+        }
+    }
+
     /// Index the font faces in the file at the given path.
     ///
     /// The file may form a font collection and contain multiple font faces,
@@ -154,7 +175,7 @@ impl FsLoader {
             stretch: FontStretch::from_number(face.width().to_number()),
         };
 
-        let file = self.resolve_path(path)?;
+        let file = self.resolve(path)?;
         self.faces.push(FaceInfo { file, index, family, variant });
 
         Ok(())
@@ -182,11 +203,8 @@ mod tests {
 
     #[test]
     fn test_index_font_dir() {
-        let mut loader = FsLoader::new();
+        let map = FsLoader::new().with_path("fonts").paths.into_inner();
-        loader.search_path("fonts");
+        let mut paths: Vec<_> = map.into_iter().map(|p| p.1).collect();
-
-        let map = loader.paths.borrow();
-        let mut paths: Vec<_> = map.values().collect();
         paths.sort();
 
         assert_eq!(paths, [

src/main.rs

@@ -1,6 +1,5 @@
 use std::fs;
 use std::path::{Path, PathBuf};
-use std::rc::Rc;
 
 use anyhow::{anyhow, bail, Context};
 use same_file::is_same_file;
@@ -30,17 +29,18 @@ fn main() -> anyhow::Result<()> {
     }
 
     // Create a loader for fonts and files.
-    let mut loader = typst::loading::FsLoader::new();
+    let loader = typst::loading::FsLoader::new()
-    loader.search_path("fonts");
+        .with_path("fonts")
-    loader.search_system();
+        .with_system()
+        .wrap();
 
     // Resolve the file id of the source file and read the file.
-    let src_id = loader.resolve_path(src_path).context("source file not found")?;
+    let src_id = loader.resolve(src_path).context("source file not found")?;
     let src = fs::read_to_string(&src_path)
         .map_err(|_| anyhow!("failed to read source file"))?;
 
     // Typeset.
-    let mut ctx = typst::Context::new(Rc::new(loader));
+    let mut ctx = typst::Context::new(loader);
     let pass = ctx.typeset(src_id, &src);
 
     // Print diagnostics.

tests/typeset.rs

@@ -63,15 +63,16 @@ fn main() {
     state.page.size = Size::new(Length::pt(120.0), Length::inf());
     state.page.margins = Sides::splat(Some(Length::pt(10.0).into()));
 
-    // Create a file system loader.
+    // We hook up some extra test helpers into the global scope.
-    let loader = {
+    let mut std = typst::library::new();
-        let mut loader = typst::loading::FsLoader::new();
+    let panics = Rc::new(RefCell::new(vec![]));
-        loader.search_path(FONT_DIR);
+    register_helpers(&mut std, Rc::clone(&panics));
-        Rc::new(loader)
-    };
 
-    let mut ctx = typst::Context::builder().state(state).build(loader.clone());
+    // Create loader and context.
+    let loader = FsLoader::new().with_path(FONT_DIR).wrap();
+    let mut ctx = Context::builder().std(std).state(state).build(loader.clone());
 
+    // Run all the tests.
     let mut ok = true;
     for src_path in filtered {
         let path = src_path.strip_prefix(TYP_DIR).unwrap();
@@ -81,8 +82,9 @@ fn main() {
             args.pdf.then(|| Path::new(PDF_DIR).join(path).with_extension("pdf"));
 
         ok &= test(
-            loader.as_ref(),
             &mut ctx,
+            loader.as_ref(),
+            &panics,
             &src_path,
             &png_path,
             &ref_path,
@@ -128,15 +130,38 @@ impl Args {
     }
 }
 
+type Panics = Rc<RefCell<Vec<Panic>>>;
+
 struct Panic {
     pos: Pos,
     lhs: Option<Value>,
     rhs: Option<Value>,
 }
 
+fn register_helpers(scope: &mut Scope, panics: Rc<RefCell<Vec<Panic>>>) {
+    scope.def_const("error", Value::Error);
+    scope.def_func("args", |_, args| {
+        let repr = typst::pretty::pretty(args);
+        args.items.clear();
+        Value::template(move |ctx| {
+            ctx.set_monospace();
+            ctx.push_text(&repr);
+        })
+    });
+    scope.def_func("test", move |ctx, args| {
+        let lhs = args.expect::<Value>(ctx, "left-hand side");
+        let rhs = args.expect::<Value>(ctx, "right-hand side");
+        if lhs != rhs {
+            panics.borrow_mut().push(Panic { pos: args.span.start, lhs, rhs });
+        }
+        Value::None
+    });
+}
+
 fn test(
-    loader: &FsLoader,
     ctx: &mut Context,
+    loader: &FsLoader,
+    panics: &Panics,
     src_path: &Path,
     png_path: &Path,
     ref_path: &Path,
@@ -146,7 +171,7 @@ fn test(
     println!("Testing {}", name.display());
 
     let src = fs::read_to_string(src_path).unwrap();
-    let src_id = loader.resolve_path(src_path).unwrap();
+    let src_id = loader.resolve(src_path).unwrap();
 
     let mut ok = true;
     let mut frames = vec![];
@@ -170,7 +195,7 @@ fn test(
             }
         } else {
             let (part_ok, compare_here, part_frames) =
-                test_part(ctx, src_id, part, i, compare_ref, lines);
+                test_part(ctx, panics, src_id, part, i, compare_ref, lines);
             ok &= part_ok;
             compare_ever |= compare_here;
             frames.extend(part_frames);
@@ -210,6 +235,7 @@ fn test(
 
 fn test_part(
     ctx: &mut Context,
+    panics: &Panics,
     src_id: FileId,
     src: &str,
     i: usize,
@@ -220,23 +246,17 @@ fn test_part(
     let (local_compare_ref, ref_diags) = parse_metadata(src, &map);
     let compare_ref = local_compare_ref.unwrap_or(compare_ref);
 
-    // We hook up some extra test helpers into the global scope.
+    let ast = parse(src);
-    let mut scope = typst::library::new();
+    let module = eval(ctx, src_id, Rc::new(ast.output));
-    let panics = Rc::new(RefCell::new(vec![]));
+    let tree = exec(ctx, &module.output.template);
-    register_helpers(&mut scope, Rc::clone(&panics));
+    let mut frames = layout(ctx, &tree.output);
 
-    let parsed = parse(src);
+    let mut diags = ast.diags;
-    let evaluated = eval(ctx, src_id, Rc::new(parsed.output));
+    diags.extend(module.diags);
-    let executed = exec(ctx, &evaluated.output.template);
+    diags.extend(tree.diags);
-    let mut layouted = layout(ctx, &executed.output);
-
-    let mut diags = parsed.diags;
-    diags.extend(evaluated.diags);
-    diags.extend(executed.diags);
 
     let mut ok = true;
-
+    for panic in panics.borrow().iter() {
-    for panic in &*panics.borrow() {
         let line = map.location(panic.pos).unwrap().line;
         println!("  Assertion failed in line {} ❌", lines + line);
         if let (Some(lhs), Some(rhs)) = (&panic.lhs, &panic.rhs) {
@@ -248,6 +268,8 @@ fn test_part(
         ok = false;
     }
 
+    panics.borrow_mut().clear();
+
     if diags != ref_diags {
         println!("  Subtest {} does not match expected diagnostics. ❌", i);
         ok = false;
@@ -279,8 +301,7 @@ fn test_part(
 
             ctx.layouts.turnaround();
 
-            let cached_result = layout(ctx, &executed.output);
+            let cached = layout(ctx, &tree.output);
-
             let misses = ctx
                 .layouts
                 .frames
@@ -297,7 +318,7 @@ fn test_part(
                 );
             }
 
-            if cached_result != layouted {
+            if cached != frames {
                 ok = false;
                 println!(
                     "    Recompilation of subtest {} differs from clean pass ❌",
@@ -311,10 +332,10 @@ fn test_part(
     }
 
     if !compare_ref {
-        layouted.clear();
+        frames.clear();
     }
 
-    (ok, compare_ref, layouted)
+    (ok, compare_ref, frames)
 }
 
 fn parse_metadata(src: &str, map: &LineMap) -> (Option<bool>, DiagSet) {
@@ -364,28 +385,6 @@ fn parse_metadata(src: &str, map: &LineMap) -> (Option<bool>, DiagSet) {
     (compare_ref, diags)
 }
 
-fn register_helpers(scope: &mut Scope, panics: Rc<RefCell<Vec<Panic>>>) {
-    scope.def_const("error", Value::Error);
-
-    scope.def_func("args", |_, args| {
-        let repr = typst::pretty::pretty(args);
-        args.items.clear();
-        Value::template(move |ctx| {
-            ctx.set_monospace();
-            ctx.push_text(&repr);
-        })
-    });
-
-    scope.def_func("test", move |ctx, args| {
-        let lhs = args.expect::<Value>(ctx, "left-hand side");
-        let rhs = args.expect::<Value>(ctx, "right-hand side");
-        if lhs != rhs {
-            panics.borrow_mut().push(Panic { pos: args.span.start, lhs, rhs });
-        }
-        Value::None
-    });
-}
-
 fn print_diag(diag: &Diag, map: &LineMap, lines: u32) {
     let mut start = map.location(diag.span.start).unwrap();
     let mut end = map.location(diag.span.end).unwrap();