From 7218892c722ca583297c0ebbda350bdf6f16d3ce Mon Sep 17 00:00:00 2001
From: Laurenz <laurmaedje@gmail.com>
Date: Tue, 1 Jun 2021 12:46:01 +0200
Subject: [PATCH] Refactor path handling
---
Cargo.toml | 2 +-
bench/src/bench.rs | 6 +++---
src/eval/capture.rs | 2 +-
src/eval/mod.rs | 46 ++++++++++++++++++++++---------------------
src/eval/scope.rs | 11 +----------
src/eval/value.rs | 1 -
src/exec/mod.rs | 9 +++------
src/export/pdf.rs | 6 +++---
src/font.rs | 14 +++++--------
src/image.rs | 42 ++++++++++++++++-----------------------
src/lib.rs | 23 +++++++++++++++++++---
src/library/markup.rs | 2 +-
src/loading/fs.rs | 44 ++++++++++++++++-------------------------
src/loading/mod.rs | 14 ++++++++++++-
src/main.rs | 16 +++++++++------
src/util.rs | 26 ++++++++++++++++++++++++
tests/typeset.rs | 4 ++--
17 files changed, 147 insertions(+), 121 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index 58f5c9379..066ca4945 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -6,7 +6,7 @@ edition = "2018"
[features]
default = ["cli", "fs"]
-cli = ["anyhow", "fs"]
+cli = ["anyhow", "fs", "same-file"]
fs = ["dirs", "memmap2", "same-file", "walkdir"]
[workspace]
diff --git a/bench/src/bench.rs b/bench/src/bench.rs
index 2a9014903..d4e297bf3 100644
--- a/bench/src/bench.rs
+++ b/bench/src/bench.rs
@@ -41,16 +41,16 @@ fn benchmarks(c: &mut Criterion) {
// Prepare intermediate results, run warm and fill caches.
let src = std::fs::read_to_string(&path).unwrap();
let tree = Rc::new(parse(&src).output);
- let evaluated = eval(&mut loader, &mut cache, &path, tree.clone(), &scope);
+ let evaluated = eval(&mut loader, &mut cache, Some(&path), tree.clone(), &scope);
let executed = exec(&evaluated.output.template, state.clone());
let layouted = layout(&mut loader, &mut cache, &executed.output);
// Bench!
bench!("parse": parse(&src));
- bench!("eval": eval(&mut loader, &mut cache, &path, tree.clone(), &scope));
+ bench!("eval": eval(&mut loader, &mut cache, Some(&path), tree.clone(), &scope));
bench!("exec": exec(&evaluated.output.template, state.clone()));
bench!("layout": layout(&mut loader, &mut cache, &executed.output));
- bench!("typeset": typeset(&mut loader, &mut cache, &path, &src, &scope, state.clone()));
+ bench!("typeset": typeset(&mut loader, &mut cache, Some(&path), &src, &scope, state.clone()));
bench!("pdf": pdf(&cache, &layouted));
}
}
diff --git a/src/eval/capture.rs b/src/eval/capture.rs
index bee523ef7..74da4747e 100644
--- a/src/eval/capture.rs
+++ b/src/eval/capture.rs
@@ -17,7 +17,7 @@ impl<'a> CapturesVisitor<'a> {
pub fn new(external: &'a Scopes) -> Self {
Self {
external,
- internal: Scopes::new(),
+ internal: Scopes::new(None),
captures: Scope::new(),
}
}
diff --git a/src/eval/mod.rs b/src/eval/mod.rs
index 38c6263e0..c480ddfec 100644
--- a/src/eval/mod.rs
+++ b/src/eval/mod.rs
@@ -23,22 +23,17 @@ use crate::loading::{FileHash, Loader};
use crate::parse::parse;
use crate::syntax::visit::Visit;
use crate::syntax::*;
+use crate::util::PathExt;
-/// Evaluated a parsed source file into a module.
-///
-/// The `path` should point to the source file for the `tree` and is used to
-/// resolve relative path names.
-///
-/// The `scope` consists of the base definitions that are present from the
-/// beginning (typically, the standard library).
+/// Evaluate a parsed source file into a module.
pub fn eval(
loader: &mut dyn Loader,
cache: &mut Cache,
- path: &Path,
+ path: Option<&Path>,
tree: Rc<Tree>,
- base: &Scope,
+ scope: &Scope,
) -> Pass<Module> {
- let mut ctx = EvalContext::new(loader, cache, path, base);
+ let mut ctx = EvalContext::new(loader, cache, path, scope);
let map = tree.eval(&mut ctx);
let module = Module {
scope: ctx.scopes.top,
@@ -67,7 +62,7 @@ pub struct EvalContext<'a> {
/// Evaluation diagnostics.
pub diags: DiagSet,
/// The location of the currently evaluated file.
- pub path: PathBuf,
+ pub path: Option<PathBuf>,
/// The stack of imported files that led to evaluation of the current file.
pub route: Vec<FileHash>,
/// A map of loaded module.
@@ -79,20 +74,24 @@ impl<'a> EvalContext<'a> {
pub fn new(
loader: &'a mut dyn Loader,
cache: &'a mut Cache,
- path: &Path,
- base: &'a Scope,
+ path: Option<&Path>,
+ scope: &'a Scope,
) -> Self {
+ let path = path.map(PathExt::normalize);
+
let mut route = vec![];
- if let Some(hash) = loader.resolve(path) {
- route.push(hash);
+ if let Some(path) = &path {
+ if let Some(hash) = loader.resolve(path) {
+ route.push(hash);
+ }
}
Self {
loader,
cache,
- scopes: Scopes::with_base(Some(base)),
+ scopes: Scopes::new(Some(scope)),
diags: DiagSet::new(),
- path: path.to_owned(),
+ path,
route,
modules: HashMap::new(),
}
@@ -102,10 +101,13 @@ impl<'a> EvalContext<'a> {
///
/// Generates an error if the file is not found.
pub fn resolve(&mut self, path: &str, span: Span) -> Option<(PathBuf, FileHash)> {
- let dir = self.path.parent().expect("location is a file");
- let path = dir.join(path);
+ let path = match &self.path {
+ Some(current) => current.parent()?.join(path),
+ None => PathBuf::from(path),
+ };
+
match self.loader.resolve(&path) {
- Some(hash) => Some((path, hash)),
+ Some(hash) => Some((path.normalize(), hash)),
None => {
self.diag(error!(span, "file not found"));
None
@@ -142,10 +144,10 @@ impl<'a> EvalContext<'a> {
let parsed = parse(string);
// Prepare the new context.
- let new_scopes = Scopes::with_base(self.scopes.base);
+ let new_scopes = Scopes::new(self.scopes.base);
let old_scopes = mem::replace(&mut self.scopes, new_scopes);
let old_diags = mem::replace(&mut self.diags, parsed.diags);
- let old_path = mem::replace(&mut self.path, resolved);
+ let old_path = mem::replace(&mut self.path, Some(resolved));
self.route.push(hash);
// Evaluate the module.
diff --git a/src/eval/scope.rs b/src/eval/scope.rs
index cfa2bccd6..e5afb6b00 100644
--- a/src/eval/scope.rs
+++ b/src/eval/scope.rs
@@ -22,16 +22,7 @@ pub struct Scopes<'a> {
impl<'a> Scopes<'a> {
/// Create a new, empty hierarchy of scopes.
- pub fn new() -> Self {
- Self {
- top: Scope::new(),
- scopes: vec![],
- base: None,
- }
- }
-
- /// Create a new hierarchy of scopes with a base scope.
- pub fn with_base(base: Option<&'a Scope>) -> Self {
+ pub fn new(base: Option<&'a Scope>) -> Self {
Self { top: Scope::new(), scopes: vec![], base }
}
diff --git a/src/eval/value.rs b/src/eval/value.rs
index d10d734af..e2ff5383b 100644
--- a/src/eval/value.rs
+++ b/src/eval/value.rs
@@ -664,7 +664,6 @@ impl From<AnyValue> for Value {
/// This would allow the type `FontFamily` to be cast from:
/// - a [`Value::Any`] variant already containing a `FontFamily`,
/// - a string, producing a named font family.
-#[macro_export]
macro_rules! value {
($type:ty:
$type_name:literal
diff --git a/src/exec/mod.rs b/src/exec/mod.rs
index 643d5b44a..35e6b55cc 100644
--- a/src/exec/mod.rs
+++ b/src/exec/mod.rs
@@ -15,9 +15,6 @@ use crate::pretty::pretty;
use crate::syntax::*;
/// Execute a template to produce a layout tree.
-///
-/// The `state` is the base state that may be updated over the course of
-/// execution.
pub fn exec(template: &TemplateValue, state: State) -> Pass<layout::Tree> {
let mut ctx = ExecContext::new(state);
template.exec(&mut ctx);
@@ -53,7 +50,7 @@ impl ExecWithMap for Tree {
impl ExecWithMap for Node {
fn exec_with_map(&self, ctx: &mut ExecContext, map: &NodeMap) {
match self {
- Node::Text(text) => ctx.push_text(text.clone()),
+ Node::Text(text) => ctx.push_text(text),
Node::Space => ctx.push_word_space(),
_ => map[&(self as *const _)].exec(ctx),
}
@@ -66,7 +63,7 @@ impl Exec for Value {
Value::None => {}
Value::Int(v) => ctx.push_text(pretty(v)),
Value::Float(v) => ctx.push_text(pretty(v)),
- Value::Str(v) => ctx.push_text(v.clone()),
+ Value::Str(v) => ctx.push_text(v),
Value::Template(v) => v.exec(ctx),
Value::Error => {}
other => {
@@ -93,7 +90,7 @@ impl Exec for TemplateNode {
fn exec(&self, ctx: &mut ExecContext) {
match self {
Self::Tree { tree, map } => tree.exec_with_map(ctx, &map),
- Self::Str(v) => ctx.push_text(v.clone()),
+ Self::Str(v) => ctx.push_text(v),
Self::Func(v) => v.exec(ctx),
}
}
diff --git a/src/export/pdf.rs b/src/export/pdf.rs
index 0ca4df388..1cc62332a 100644
--- a/src/export/pdf.rs
+++ b/src/export/pdf.rs
@@ -139,7 +139,7 @@ impl<'a> PdfExporter<'a> {
// We only write font switching actions when the used face changes. To
// do that, we need to remember the active face.
- let mut face = FaceId::MAX;
+ let mut face = None;
let mut size = Length::zero();
let mut fill: Option<Fill> = None;
@@ -158,8 +158,8 @@ impl<'a> PdfExporter<'a> {
// Then, also check if we need to issue a font switching
// action.
- if shaped.face_id != face || shaped.size != size {
- face = shaped.face_id;
+ if face != Some(shaped.face_id) || shaped.size != size {
+ face = Some(shaped.face_id);
size = shaped.size;
let name = format!("F{}", self.fonts.map(shaped.face_id));
diff --git a/src/font.rs b/src/font.rs
index 69a309006..516d4bbe7 100644
--- a/src/font.rs
+++ b/src/font.rs
@@ -6,8 +6,7 @@ use std::fmt::{self, Debug, Display, Formatter};
use serde::{Deserialize, Serialize};
use crate::geom::Length;
-use crate::loading::Buffer;
-use crate::loading::Loader;
+use crate::loading::{Buffer, Loader};
/// A font face.
pub struct Face {
@@ -171,7 +170,7 @@ impl FontCache {
let mut families = HashMap::<String, Vec<FaceId>>::new();
for (i, info) in loader.faces().iter().enumerate() {
- let id = FaceId(i as u32);
+ let id = FaceId(i as u64);
faces.push(None);
families
.entry(info.family.to_lowercase())
@@ -259,22 +258,19 @@ impl FontCache {
/// A unique identifier for a loaded font face.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
-pub struct FaceId(u32);
+pub struct FaceId(u64);
impl FaceId {
- /// A blank initialization value.
- pub const MAX: Self = Self(u32::MAX);
-
/// Create a face id from the raw underlying value.
///
/// This should only be called with values returned by
/// [`into_raw`](Self::into_raw).
- pub fn from_raw(v: u32) -> Self {
+ pub fn from_raw(v: u64) -> Self {
Self(v)
}
/// Convert into the raw underlying value.
- pub fn into_raw(self) -> u32 {
+ pub fn into_raw(self) -> u64 {
self.0
}
}
diff --git a/src/image.rs b/src/image.rs
index 3c5c85732..90252ff33 100644
--- a/src/image.rs
+++ b/src/image.rs
@@ -9,7 +9,7 @@ use image::io::Reader as ImageReader;
use image::{DynamicImage, GenericImageView, ImageFormat};
use serde::{Deserialize, Serialize};
-use crate::loading::{FileHash, Loader};
+use crate::loading::Loader;
/// A loaded image.
pub struct Image {
@@ -55,10 +55,8 @@ impl Debug for Image {
/// Caches decoded images.
pub struct ImageCache {
- /// Loaded images indexed by [`ImageId`].
- images: Vec<Image>,
/// Maps from file hashes to ids of decoded images.
- map: HashMap<FileHash, ImageId>,
+ images: HashMap<ImageId, Image>,
/// Callback for loaded images.
on_load: Option<Box<dyn Fn(ImageId, &Image)>>,
}
@@ -66,28 +64,22 @@ pub struct ImageCache {
impl ImageCache {
/// Create a new, empty image cache.
pub fn new() -> Self {
- Self {
- images: vec![],
- map: HashMap::new(),
- on_load: None,
- }
+ Self { images: HashMap::new(), on_load: None }
}
/// Load and decode an image file from a path.
pub fn load(&mut self, loader: &mut dyn Loader, path: &Path) -> Option<ImageId> {
- Some(match self.map.entry(loader.resolve(path)?) {
- Entry::Occupied(entry) => *entry.get(),
- Entry::Vacant(entry) => {
- let buffer = loader.load_file(path)?;
- let image = Image::parse(&buffer)?;
- let id = ImageId(self.images.len() as u32);
- if let Some(callback) = &self.on_load {
- callback(id, &image);
- }
- self.images.push(image);
- *entry.insert(id)
+ let hash = loader.resolve(path)?;
+ let id = ImageId(hash.into_raw());
+ if let Entry::Vacant(entry) = self.images.entry(id) {
+ let buffer = loader.load_file(path)?;
+ let image = Image::parse(&buffer)?;
+ if let Some(callback) = &self.on_load {
+ callback(id, &image);
}
- })
+ entry.insert(image);
+ }
+ Some(id)
}
/// Get a reference to a loaded image.
@@ -96,7 +88,7 @@ impl ImageCache {
/// only be called with ids returned by [`load()`](Self::load).
#[track_caller]
pub fn get(&self, id: ImageId) -> &Image {
- &self.images[id.0 as usize]
+ &self.images[&id]
}
/// Register a callback which is invoked each time an image is loaded.
@@ -110,19 +102,19 @@ impl ImageCache {
/// A unique identifier for a loaded image.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
-pub struct ImageId(u32);
+pub struct ImageId(u64);
impl ImageId {
/// Create an image id from the raw underlying value.
///
/// This should only be called with values returned by
/// [`into_raw`](Self::into_raw).
- pub fn from_raw(v: u32) -> Self {
+ pub fn from_raw(v: u64) -> Self {
Self(v)
}
/// Convert into the raw underlying value.
- pub fn into_raw(self) -> u32 {
+ pub fn into_raw(self) -> u64 {
self.0
}
}
diff --git a/src/lib.rs b/src/lib.rs
index 65e23c799..3c50230f6 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -59,16 +59,33 @@ use crate::layout::Frame;
use crate::loading::Loader;
/// Process source code directly into a collection of layouted frames.
+///
+/// # Parameters
+/// - The `loader` is used to load fonts, images and other source files.
+/// - The `cache` stores things that are reusable across several compilations
+/// like loaded fonts, decoded images and layouting artifacts.
+/// - The `path` should point to the source file if `src` comes from the file
+/// system and is used to resolve relative paths (for importing and image
+/// loading).
+/// - The `src` is the _Typst_ source code to typeset.
+/// - The `scope` contains definitions that are available everywhere,
+/// typically the standard library.
+/// - The `state` defines initial properties for page size, font selection and
+/// so on.
+///
+/// # Return value
+/// Returns a vector of frames representing individual pages alongside
+/// diagnostic information (errors and warnings).
pub fn typeset(
loader: &mut dyn Loader,
cache: &mut Cache,
- path: &Path,
+ path: Option<&Path>,
src: &str,
- base: &Scope,
+ scope: &Scope,
state: State,
) -> Pass<Vec<Frame>> {
let parsed = parse::parse(src);
- let evaluated = eval::eval(loader, cache, path, Rc::new(parsed.output), base);
+ let evaluated = eval::eval(loader, cache, path, Rc::new(parsed.output), scope);
let executed = exec::exec(&evaluated.output.template, state);
let layouted = layout::layout(loader, cache, &executed.output);
diff --git a/src/library/markup.rs b/src/library/markup.rs
index c218746ba..0a80fe747 100644
--- a/src/library/markup.rs
+++ b/src/library/markup.rs
@@ -160,7 +160,7 @@ pub fn raw(ctx: &mut EvalContext, args: &mut FuncArgs) -> Value {
let snapshot = ctx.state.clone();
ctx.set_monospace();
- ctx.push_text(text.clone());
+ ctx.push_text(&text);
ctx.state = snapshot;
if block {
diff --git a/src/loading/fs.rs b/src/loading/fs.rs
index ac3f37065..7fa1c1202 100644
--- a/src/loading/fs.rs
+++ b/src/loading/fs.rs
@@ -24,8 +24,7 @@ pub struct FsLoader {
cache: FileCache,
}
-/// Maps from paths to loaded file buffers. When the buffer is `None` the file
-/// does not exist or couldn't be read.
+/// Maps from resolved file hashes to loaded file buffers.
type FileCache = HashMap<FileHash, Buffer>;
impl FsLoader {
@@ -169,38 +168,29 @@ impl Loader for FsLoader {
}
fn load_face(&mut self, idx: usize) -> Option<Buffer> {
- load(&mut self.cache, &self.files[idx])
+ self.load_file(&self.files[idx].clone())
}
fn load_file(&mut self, path: &Path) -> Option<Buffer> {
- load(&mut self.cache, path)
+ let hash = self.resolve(path)?;
+ Some(Rc::clone(match self.cache.entry(hash) {
+ Entry::Occupied(entry) => entry.into_mut(),
+ Entry::Vacant(entry) => {
+ let buffer = std::fs::read(path).ok()?;
+ entry.insert(Rc::new(buffer))
+ }
+ }))
}
fn resolve(&self, path: &Path) -> Option<FileHash> {
- hash(path)
- }
-}
-
-/// Load from the file system using a cache.
-fn load(cache: &mut FileCache, path: &Path) -> Option<Buffer> {
- Some(match cache.entry(hash(path)?) {
- Entry::Occupied(entry) => entry.get().clone(),
- Entry::Vacant(entry) => {
- let buffer = std::fs::read(path).ok()?;
- entry.insert(Rc::new(buffer)).clone()
+ let file = File::open(path).ok()?;
+ let meta = file.metadata().ok()?;
+ if meta.is_file() {
+ let handle = Handle::from_file(file).ok()?;
+ Some(FileHash::from_raw(fxhash::hash64(&handle)))
+ } else {
+ None
}
- })
-}
-
-/// Create a hash that is the same for all paths pointing to the same file.
-fn hash(path: &Path) -> Option<FileHash> {
- let file = File::open(path).ok()?;
- let meta = file.metadata().ok()?;
- if meta.is_file() {
- let handle = Handle::from_file(file).ok()?;
- Some(FileHash(fxhash::hash64(&handle)))
- } else {
- None
}
}
diff --git a/src/loading/mod.rs b/src/loading/mod.rs
index f57b5c730..0e171796b 100644
--- a/src/loading/mod.rs
+++ b/src/loading/mod.rs
@@ -36,7 +36,19 @@ pub trait Loader {
///
/// Should be the same for all paths pointing to the same file.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
-pub struct FileHash(pub u64);
+pub struct FileHash(u64);
+
+impl FileHash {
+ /// Create an file hash from a raw hash value.
+ pub fn from_raw(v: u64) -> Self {
+ Self(v)
+ }
+
+ /// Convert into the raw underlying hash value.
+ pub fn into_raw(self) -> u64 {
+ self.0
+ }
+}
/// A loader which serves nothing.
pub struct BlankLoader;
diff --git a/src/main.rs b/src/main.rs
index 449cad20a..d0762b3e7 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -2,8 +2,7 @@ use std::fs;
use std::path::{Path, PathBuf};
use anyhow::{anyhow, bail, Context};
-
-use typst::loading::Loader;
+use same_file::is_same_file;
fn main() -> anyhow::Result<()> {
let args: Vec<_> = std::env::args().collect();
@@ -33,9 +32,7 @@ fn main() -> anyhow::Result<()> {
};
// Ensure that the source file is not overwritten.
- let src_hash = loader.resolve(&src_path);
- let dest_hash = loader.resolve(&dest_path);
- if src_hash.is_some() && src_hash == dest_hash {
+ if is_same_file(src_path, &dest_path).unwrap_or(false) {
bail!("source and destination files are the same");
}
@@ -47,7 +44,14 @@ fn main() -> anyhow::Result<()> {
let mut cache = typst::cache::Cache::new(&loader);
let scope = typst::library::new();
let state = typst::exec::State::default();
- let pass = typst::typeset(&mut loader, &mut cache, &src_path, &src, &scope, state);
+ let pass = typst::typeset(
+ &mut loader,
+ &mut cache,
+ Some(&src_path),
+ &src,
+ &scope,
+ state,
+ );
// Print diagnostics.
let map = typst::parse::LineMap::new(&src);
diff --git a/src/util.rs b/src/util.rs
index 72db4518b..8a8c04b6d 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -2,6 +2,7 @@
use std::cmp::Ordering;
use std::ops::Range;
+use std::path::{Component, Path, PathBuf};
/// Additional methods for slices.
pub trait SliceExt<T> {
@@ -79,3 +80,28 @@ impl RangeExt for Range<usize> {
}
}
}
+
+/// Additional methods for [`Path`].
+pub trait PathExt {
+ /// Lexically normalize a path.
+ fn normalize(&self) -> PathBuf;
+}
+
+impl PathExt for Path {
+ fn normalize(&self) -> PathBuf {
+ let mut out = PathBuf::new();
+ for component in self.components() {
+ match component {
+ Component::CurDir => {}
+ Component::ParentDir => match out.components().next_back() {
+ Some(Component::Normal(_)) => {
+ out.pop();
+ }
+ _ => out.push(component),
+ },
+ _ => out.push(component),
+ }
+ }
+ out
+ }
+}
diff --git a/tests/typeset.rs b/tests/typeset.rs
index 3f9bbd1d5..604a82758 100644
--- a/tests/typeset.rs
+++ b/tests/typeset.rs
@@ -203,7 +203,7 @@ fn test(
fn test_part(
loader: &mut FsLoader,
cache: &mut Cache,
- path: &Path,
+ src_path: &Path,
src: &str,
i: usize,
compare_ref: bool,
@@ -224,7 +224,7 @@ fn test_part(
state.page.size = Size::new(Length::pt(120.0), Length::raw(f64::INFINITY));
state.page.margins = Sides::splat(Some(Length::pt(10.0).into()));
- let mut pass = typst::typeset(loader, cache, path, &src, &scope, state);
+ let mut pass = typst::typeset(loader, cache, Some(src_path), &src, &scope, state);
if !compare_ref {
pass.output.clear();
}