From b0e5212973ce2efcb1433323d67c06eea1a81785 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Wed, 21 Jul 2021 11:25:49 +0200 Subject: [PATCH] Fs builder methods + tidy up --- bench/src/clock.rs | 27 +++++----- src/eval/mod.rs | 8 +-- src/lib.rs | 9 ++-- src/loading/fs.rs | 126 ++++++++++++++++++++++++++------------------- src/main.rs | 12 ++--- tests/typeset.rs | 103 ++++++++++++++++++------------------ 6 files changed, 150 insertions(+), 135 deletions(-) diff --git a/bench/src/clock.rs b/bench/src/clock.rs index e33c6c13a..c3c5b3785 100644 --- a/bench/src/clock.rs +++ b/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 mut loader = FsLoader::new(); - loader.search_path(FONT_DIR); - Rc::new(loader) - }; - + let loader = FsLoader::new().with_path(FONT_DIR).wrap(); 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 { - let mut borrowed = self.ctx.borrow_mut(); - eval(&mut borrowed, self.src_id, Rc::clone(&self.ast)).output + fn eval(&self) -> Pass { + eval( + &mut self.ctx.borrow_mut(), + self.src_id, + Rc::clone(&self.ast), + ) } - fn exec(&self) -> LayoutTree { - exec(&mut self.ctx.borrow_mut(), &self.module.template).output + fn exec(&self) -> Pass { + exec(&mut self.ctx.borrow_mut(), &self.module.template) } fn layout(&self) -> Vec> { layout(&mut self.ctx.borrow_mut(), &self.tree) } - fn typeset(&self) -> Vec> { - self.ctx.borrow_mut().typeset(self.src_id, &self.src).output + fn typeset(&self) -> Pass>> { + self.ctx.borrow_mut().typeset(self.src_id, &self.src) } fn pdf(&self) -> Vec { diff --git a/src/eval/mod.rs b/src/eval/mod.rs index 0efaaa90d..9ec33b131 100644 --- a/src/eval/mod.rs +++ b/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) -> Pass { - let mut ctx = EvalContext::new(ctx, location); +pub fn eval(ctx: &mut Context, file: FileId, ast: Rc) -> Pass { + 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(), } } diff --git a/src/lib.rs b/src/lib.rs index 90fd87748..594b4a593 100644 --- a/src/lib.rs +++ b/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 - /// relative paths (for importing and image loading). + /// The `file` identifies the source file and is used to resolve relative + /// 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 - /// and so on. + /// The initial properties for page size, font selection and so on. pub fn state(mut self, state: State) -> Self { self.state = Some(state); self diff --git a/src/loading/fs.rs b/src/loading/fs.rs index 8785499e1..407e2d94f 100644 --- a/src/loading/fs.rs +++ b/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. - pub fn resolve_path(&self, path: &Path) -> io::Result { - 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")) - } + /// Builder-style variant of `search_system`. + pub fn with_system(mut self) -> Self { + self.search_system(); + self + } + + /// Builder-style variant of `search_path`. + pub fn with_path(mut self, dir: impl AsRef) -> Self { + 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 { + Rc::new(self) } /// Search for fonts in the operating system's font directories. - #[cfg(all(unix, not(target_os = "macos")))] 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); - } - } - - /// 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_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")); - } + self.search_system_impl(); } /// 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 { + 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(); - loader.search_path("fonts"); - - let map = loader.paths.borrow(); - let mut paths: Vec<_> = map.values().collect(); + let map = FsLoader::new().with_path("fonts").paths.into_inner(); + let mut paths: Vec<_> = map.into_iter().map(|p| p.1).collect(); paths.sort(); assert_eq!(paths, [ diff --git a/src/main.rs b/src/main.rs index f9da37fad..91edff178 100644 --- a/src/main.rs +++ b/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(); - loader.search_path("fonts"); - loader.search_system(); + let loader = typst::loading::FsLoader::new() + .with_path("fonts") + .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. diff --git a/tests/typeset.rs b/tests/typeset.rs index c73c51f61..5a071c759 100644 --- a/tests/typeset.rs +++ b/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. - let loader = { - let mut loader = typst::loading::FsLoader::new(); - loader.search_path(FONT_DIR); - Rc::new(loader) - }; + // We hook up some extra test helpers into the global scope. + let mut std = typst::library::new(); + let panics = Rc::new(RefCell::new(vec![])); + register_helpers(&mut std, Rc::clone(&panics)); - 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>>; + struct Panic { pos: Pos, lhs: Option, rhs: Option, } +fn register_helpers(scope: &mut Scope, panics: Rc>>) { + 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::(ctx, "left-hand side"); + let rhs = args.expect::(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 mut scope = typst::library::new(); - let panics = Rc::new(RefCell::new(vec![])); - register_helpers(&mut scope, Rc::clone(&panics)); + let ast = parse(src); + let module = eval(ctx, src_id, Rc::new(ast.output)); + let tree = exec(ctx, &module.output.template); + let mut frames = layout(ctx, &tree.output); - let parsed = parse(src); - let evaluated = eval(ctx, src_id, Rc::new(parsed.output)); - let executed = exec(ctx, &evaluated.output.template); - let mut layouted = layout(ctx, &executed.output); - - let mut diags = parsed.diags; - diags.extend(evaluated.diags); - diags.extend(executed.diags); + let mut diags = ast.diags; + diags.extend(module.diags); + diags.extend(tree.diags); let mut ok = true; - - for panic in &*panics.borrow() { + for panic in panics.borrow().iter() { 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, DiagSet) { @@ -364,28 +385,6 @@ fn parse_metadata(src: &str, map: &LineMap) -> (Option, DiagSet) { (compare_ref, diags) } -fn register_helpers(scope: &mut Scope, panics: Rc>>) { - 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::(ctx, "left-hand side"); - let rhs = args.expect::(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();