From c384e524800bb55da0c5614f412e7d835ed67945 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Mon, 29 Apr 2019 13:41:00 +0200 Subject: [PATCH] =?UTF-8?q?Improve=20code=20quality=20=F0=9F=8E=AB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/engine/mod.rs | 17 +++-- src/export/pdf.rs | 4 +- src/font.rs | 8 +- src/func.rs | 23 ++++-- src/lib.rs | 59 +++++++++------ src/parsing.rs | 187 ++++++++++++++++++++++++++-------------------- src/syntax.rs | 48 +----------- 7 files changed, 174 insertions(+), 172 deletions(-) diff --git a/src/engine/mod.rs b/src/engine/mod.rs index 37fce8dc4..622dfc5e9 100644 --- a/src/engine/mod.rs +++ b/src/engine/mod.rs @@ -3,7 +3,9 @@ use std::cell::{RefCell, Ref}; use std::collections::HashMap; use std::mem::swap; + use smallvec::SmallVec; + use crate::syntax::{SyntaxTree, Node}; use crate::doc::{Document, Page, Text, TextCommand}; use crate::font::{Font, FontFamily, FontInfo, FontError}; @@ -36,7 +38,8 @@ pub struct Engine<'a> { impl<'a> Engine<'a> { /// Create a new generator from a syntax tree. - pub(crate) fn new(tree: &'a SyntaxTree, context: &'a Context<'a>) -> Engine<'a> { + #[inline] + pub fn new(tree: &'a SyntaxTree, context: &'a Context<'a>) -> Engine<'a> { Engine { tree, ctx: context, @@ -52,7 +55,7 @@ impl<'a> Engine<'a> { } /// Generate the abstract document. - pub(crate) fn typeset(mut self) -> TypeResult { + pub fn typeset(mut self) -> TypesetResult { // Start by moving to a suitable position. self.move_start(); @@ -91,7 +94,7 @@ impl<'a> Engine<'a> { } /// Write a word. - fn write_word(&mut self, word: &str) -> TypeResult<()> { + fn write_word(&mut self, word: &str) -> TypesetResult<()> { // Contains pairs of (characters, font_index, char_width). let mut chars_with_widths = SmallVec::<[(char, usize, Size); 12]>::new(); @@ -127,7 +130,7 @@ impl<'a> Engine<'a> { } /// Write the space character: `' '`. - fn write_space(&mut self) -> TypeResult<()> { + fn write_space(&mut self) -> TypesetResult<()> { let space_width = self.char_width(' ', &self.get_font_for(' ')?.1); if !self.would_overflow(space_width) && self.current_line_width > Size::zero() { self.write_word(" ")?; @@ -190,7 +193,7 @@ impl<'a> Engine<'a> { } /// Load a font that has the character we need. - fn get_font_for(&self, character: char) -> TypeResult<(usize, Ref)> { + fn get_font_for(&self, character: char) -> TypesetResult<(usize, Ref)> { self.font_loader.get(FontQuery { families: &self.ctx.style.font_families, italic: self.italic, @@ -426,9 +429,11 @@ pub enum TypesetError { Font(FontError), } +/// The result type for typesetting. +pub type TypesetResult = Result; + error_type! { err: TypesetError, - res: TypeResult, show: f => match err { TypesetError::MissingFont => write!(f, "missing font"), TypesetError::Font(err) => write!(f, "font error: {}", err), diff --git a/src/export/pdf.rs b/src/export/pdf.rs index 4bb75a0df..ee7ce1ef6 100644 --- a/src/export/pdf.rs +++ b/src/export/pdf.rs @@ -2,17 +2,18 @@ use std::collections::HashSet; use std::io::{self, Write}; + use pdf::{PdfWriter, Ref, Rect, Version, Trailer, Content}; use pdf::doc::{Catalog, PageTree, Page, Resource, Text}; use pdf::font::{Type0Font, CIDFont, CIDFontType, CIDSystemInfo, FontDescriptor, FontFlags}; use pdf::font::{GlyphUnit, CMap, CMapEncoding, WidthRecord, FontStream}; + use crate::doc::{Document, Text as DocText, TextCommand}; use crate::font::{Font, FontError}; use crate::engine::Size; /// Exports documents into _PDFs_. -#[derive(Debug)] pub struct PdfExporter {} impl PdfExporter { @@ -31,7 +32,6 @@ impl PdfExporter { } /// Writes documents in the _PDF_ format. -#[derive(Debug)] struct PdfEngine<'d, W: Write> { writer: PdfWriter, doc: &'d Document, diff --git a/src/font.rs b/src/font.rs index 49670df80..3f3405e85 100644 --- a/src/font.rs +++ b/src/font.rs @@ -12,12 +12,14 @@ use std::collections::HashMap; use std::fs::File; -use std::path::PathBuf; use std::io::{self, Cursor, Read, Seek, SeekFrom, BufReader}; +use std::path::PathBuf; + use byteorder::{BE, ReadBytesExt, WriteBytesExt}; use opentype::{Error as OpentypeError, OpenTypeReader, Outlines, TableRecord, Tag}; use opentype::tables::{Header, Name, CharMap, MaximumProfile, HorizontalMetrics, Post, OS2}; use opentype::global::{MacStyleFlags, NameEntry}; + use crate::engine::Size; @@ -152,7 +154,7 @@ impl Font { } /// Font metrics relevant to the typesetting engine. -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Debug, Copy, Clone)] pub struct FontMetrics { /// Whether the font is italic. pub is_italic: bool, @@ -284,7 +286,6 @@ pub enum FontFamily { } /// A font provider serving fonts from a folder on the local file system. -#[derive(Debug)] pub struct FileSystemFontProvider { base: PathBuf, paths: Vec, @@ -348,7 +349,6 @@ impl FontProvider for FileSystemFontProvider { } } -#[derive(Debug)] struct Subsetter<'d> { // Original font font: &'d Font, diff --git a/src/func.rs b/src/func.rs index d90cd85ea..e92122784 100644 --- a/src/func.rs +++ b/src/func.rs @@ -2,14 +2,14 @@ use std::any::Any; use std::collections::HashMap; -use std::fmt::Debug; +use std::fmt::{self, Debug, Formatter}; use crate::syntax::{FuncHeader, Expression}; use crate::parsing::{ParseTokens, ParseResult}; -/// Parser functions. -pub type ParseFunc = dyn Fn(ParseContext) -> ParseResult>; +/// A function which transforms a parsing context into a boxed function. +type ParseFunc = dyn Fn(ParseContext) -> ParseResult>; /// Types that act as functions. /// @@ -19,7 +19,7 @@ pub type ParseFunc = dyn Fn(ParseContext) -> ParseResult>; /// The trait `FunctionBounds` is automatically implemented for types which can be /// used as functions, that is they fulfill the bounds `Debug + PartialEq + 'static`. pub trait Function: FunctionBounds { - /// Parse the function. + /// Parse the tokens of the context with the given header and scope into self. fn parse(context: ParseContext) -> ParseResult where Self: Sized; /// Execute the function and optionally yield a return value. @@ -41,20 +41,27 @@ impl Scope { pub fn add(&mut self, name: &str) { self.parsers.insert( name.to_owned(), - Box::new(|context| match F::parse(context) { - Ok(func) => Ok(Box::new(func)), - Err(err) => Err(err), + Box::new(|context| { + F::parse(context).map(|func| Box::new(func) as Box) }) ); } /// Return the parser with the given name if there is one. - pub fn get_parser(&self, name: &str) -> Option<&ParseFunc> { + pub(crate) fn get_parser(&self, name: &str) -> Option<&ParseFunc> { self.parsers.get(name).map(|x| &**x) } } +impl Debug for Scope { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + write!(f, "Scope ")?; + write!(f, "{:?}", self.parsers.keys()) + } +} + /// The context for parsing a function. +#[derive(Debug)] pub struct ParseContext<'s, 't> { /// The header of the function to be parsed. pub header: &'s FuncHeader, diff --git a/src/lib.rs b/src/lib.rs index 22440b26b..5a11345ac 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -43,11 +43,13 @@ //! exporter.export(&document, file).unwrap(); //! ``` -use crate::syntax::SyntaxTree; -use crate::parsing::{Parser, ParseTokens, ParseError}; +use std::fmt::{self, Debug, Formatter}; + use crate::doc::Document; -use crate::font::FontProvider; use crate::engine::{Engine, Style, TypesetError}; +use crate::font::FontProvider; +use crate::parsing::{Parser, ParseTokens, ParseResult, ParseError}; +use crate::syntax::SyntaxTree; #[macro_use] mod error; @@ -69,14 +71,6 @@ pub struct Compiler<'p> { context: Context<'p>, } -struct Context<'p> { - /// Style for typesetting. - style: Style, - /// Font providers. - font_providers: Vec>, -} - -/// Functions to set up the compilation context. impl<'p> Compiler<'p> { /// Create a new compiler. #[inline] @@ -100,28 +94,42 @@ impl<'p> Compiler<'p> { pub fn add_font_provider(&mut self, provider: P) where P: FontProvider { self.context.font_providers.push(Box::new(provider)); } -} -/// Compilation functions. -impl<'p> Compiler<'p> { /// Parse source code into a syntax tree. #[inline] - pub fn parse(&self, src: &str) -> Result { + pub fn parse(&self, src: &str) -> ParseResult { let mut tokens = ParseTokens::new(src); Parser::new(&mut tokens).parse() } /// Compile a portable typesetted document from source code. #[inline] - pub fn typeset(&self, src: &str) -> Result { + pub fn typeset(&self, src: &str) -> CompileResult { let tree = self.parse(src)?; let engine = Engine::new(&tree, &self.context); engine.typeset().map_err(Into::into) } } +/// Holds the compilation context. +pub struct Context<'p> { + /// Style for typesetting. + style: Style, + /// Font providers. + font_providers: Vec>, +} + +impl Debug for Context<'_> { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + f.debug_struct("Context") + .field("style", &self.style) + .field("font_providers", &self.font_providers.len()) + .finish() + } +} + /// The general error type for compilation. -pub enum Error { +pub enum CompileError { /// An error that occured while transforming source code into /// an abstract syntax tree. Parse(ParseError), @@ -129,18 +137,21 @@ pub enum Error { Typeset(TypesetError), } +/// The result type for compilation. +pub type CompileResult = Result; + error_type! { - err: Error, + err: CompileError, show: f => match err { - Error::Parse(e) => write!(f, "parse error: {}", e), - Error::Typeset(e) => write!(f, "typeset error: {}", e), + CompileError::Parse(e) => write!(f, "parse error: {}", e), + CompileError::Typeset(e) => write!(f, "typeset error: {}", e), }, source: match err { - Error::Parse(e) => Some(e), - Error::Typeset(e) => Some(e), + CompileError::Parse(e) => Some(e), + CompileError::Typeset(e) => Some(e), }, - from: (ParseError, Error::Parse(err)), - from: (TypesetError, Error::Typeset(err)), + from: (ParseError, CompileError::Parse(err)), + from: (TypesetError, CompileError::Typeset(err)), } diff --git a/src/parsing.rs b/src/parsing.rs index 718d895bf..fed3a3ade 100644 --- a/src/parsing.rs +++ b/src/parsing.rs @@ -6,12 +6,12 @@ use std::iter::Peekable; use std::mem::swap; use std::ops::Deref; +use unicode_segmentation::{UnicodeSegmentation, UWordBounds}; + use crate::syntax::*; use crate::func::{ParseContext, Scope}; use crate::utility::{Splinor, Spline, Splined, StrExt}; -use unicode_segmentation::{UnicodeSegmentation, UWordBounds}; - /// An iterator over the tokens of source code. #[derive(Clone)] @@ -22,17 +22,6 @@ pub struct Tokens<'s> { stack: Vec>, } -impl fmt::Debug for Tokens<'_> { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_struct("Tokens") - .field("source", &self.source) - .field("words", &"Peekable") - .field("state", &self.state) - .field("stack", &self.stack) - .finish() - } -} - /// The state the tokenizer is in. #[derive(Debug, Clone)] enum TokensState<'s> { @@ -48,18 +37,39 @@ enum TokensState<'s> { DoubleUnderscore(Spline<'s, Token<'s>>), } -impl PartialEq for TokensState<'_> { - fn eq(&self, other: &TokensState) -> bool { - use TokensState as TS; - - match (self, other) { - (TS::Body, TS::Body) => true, - (TS::Function, TS::Function) => true, - (TS::MaybeBody, TS::MaybeBody) => true, - // They are not necessarily different, but we don't care - _ => false, +impl<'s> Tokens<'s> { + /// Create a new token stream from text. + #[inline] + pub fn new(source: &'s str) -> Tokens<'s> { + Tokens { + source, + words: source.split_word_bounds().peekable(), + state: TokensState::Body, + stack: vec![], } } + + /// Advance the iterator by one step. + fn advance(&mut self) { + self.words.next(); + } + + /// Switch to the given state. + fn switch(&mut self, mut state: TokensState<'s>) { + swap(&mut state, &mut self.state); + self.stack.push(state); + } + + /// Go back to the top-of-stack state. + fn unswitch(&mut self) { + self.state = self.stack.pop().unwrap_or(TokensState::Body); + } + + /// Advance and return the given token. + fn consumed(&mut self, token: Token<'s>) -> Token<'s> { + self.advance(); + token + } } impl<'s> Iterator for Tokens<'s> { @@ -173,39 +183,29 @@ impl<'s> Iterator for Tokens<'s> { } } -impl<'s> Tokens<'s> { - /// Create a new token stream from text. - #[inline] - pub fn new(source: &'s str) -> Tokens<'s> { - Tokens { - source, - words: source.split_word_bounds().peekable(), - state: TokensState::Body, - stack: vec![], +impl fmt::Debug for Tokens<'_> { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.debug_struct("Tokens") + .field("source", &self.source) + .field("words", &"Peekable") + .field("state", &self.state) + .field("stack", &self.stack) + .finish() + } +} + +impl PartialEq for TokensState<'_> { + fn eq(&self, other: &TokensState) -> bool { + use TokensState as TS; + + match (self, other) { + (TS::Body, TS::Body) => true, + (TS::Function, TS::Function) => true, + (TS::MaybeBody, TS::MaybeBody) => true, + // They are not necessarily different, but we don't care + _ => false, } } - - /// Advance the iterator by one step. - fn advance(&mut self) { - self.words.next(); - } - - /// Switch to the given state. - fn switch(&mut self, mut state: TokensState<'s>) { - swap(&mut state, &mut self.state); - self.stack.push(state); - } - - /// Go back to the top-of-stack state. - fn unswitch(&mut self) { - self.state = self.stack.pop().unwrap_or(TokensState::Body); - } - - /// Advance and return the given token. - fn consumed(&mut self, token: Token<'s>) -> Token<'s> { - self.advance(); - token - } } /// Transforms token streams to syntax trees. @@ -228,18 +228,20 @@ enum ParserState { } impl<'s, 't> Parser<'s, 't> { - /// Create a new parser from a type that emits results of tokens. + /// Create a new parser from a stream of tokens. + #[inline] pub fn new(tokens: &'s mut ParseTokens<'t>) -> Parser<'s, 't> { - Parser::new_internal(ParserScope::Owned(Scope::new()), tokens) + Parser::new_internal(tokens, ParserScope::Owned(Scope::new())) } /// Create a new parser with a scope containing function definitions. - pub fn with_scope(scope: &'s Scope, tokens: &'s mut ParseTokens<'t>) -> Parser<'s, 't> { - Parser::new_internal(ParserScope::Shared(scope), tokens) + #[inline] + pub fn with_scope(tokens: &'s mut ParseTokens<'t>, scope: &'s Scope) -> Parser<'s, 't> { + Parser::new_internal(tokens, ParserScope::Shared(scope)) } /// Internal helper for construction. - fn new_internal(scope: ParserScope<'s>, tokens: &'s mut ParseTokens<'t>) -> Parser<'s, 't> { + fn new_internal(tokens: &'s mut ParseTokens<'t>, scope: ParserScope<'s>) -> Parser<'s, 't> { Parser { tokens, scope, @@ -248,7 +250,7 @@ impl<'s, 't> Parser<'s, 't> { } } - /// Parse into an abstract syntax tree. + /// Parse the source into an abstract syntax tree. pub fn parse(mut self) -> ParseResult { use ParserState as PS; @@ -314,7 +316,11 @@ impl<'s, 't> Parser<'s, 't> { // The next token should be the name of the function. let name = match self.tokens.next() { Some(Token::Word(word)) => { - Ident::new(word).ok_or_else(|| ParseError::new("invalid identifier")) + if word.is_identifier() { + Ok(word.to_owned()) + } else { + Err(ParseError::new("invalid identifier")) + } }, _ => Err(ParseError::new("expected identifier")), }?; @@ -345,8 +351,6 @@ impl<'s, 't> Parser<'s, 't> { let body = if has_body { self.tokens.start(); - println!("starting with: {:?}", self.tokens); - let body = parser(ParseContext { header: &header, tokens: Some(&mut self.tokens), @@ -354,7 +358,6 @@ impl<'s, 't> Parser<'s, 't> { })?; self.tokens.finish(); - println!("finished with: {:?}", self.tokens); // Now the body should be closed. if self.tokens.next() != Some(Token::RightBracket) { @@ -431,6 +434,7 @@ impl<'s, 't> Parser<'s, 't> { } /// An owned or shared scope. +#[derive(Debug)] enum ParserScope<'s> { Owned(Scope), Shared(&'s Scope) @@ -448,10 +452,14 @@ impl Deref for ParserScope<'_> { } /// A token iterator that iterates over exactly one body. -#[derive(Debug)] +/// +/// This iterator wraps [`Tokens`] and yields exactly the tokens of one +/// function body or the complete top-level body and stops there. +#[derive(Debug, Clone)] pub struct ParseTokens<'s> { tokens: Peekable>, parens: Vec, + blocked: bool, } impl<'s> ParseTokens<'s> { @@ -467,16 +475,22 @@ impl<'s> ParseTokens<'s> { ParseTokens { tokens: tokens.peekable(), parens: vec![], + blocked: false, } } /// Peek at the next token. #[inline] pub fn peek(&mut self) -> Option<&Token<'s>> { + if self.blocked { + return None; + } + let token = self.tokens.peek(); if token == Some(&Token::RightBracket) && self.parens.last() == Some(&0) { return None; } + token } @@ -487,6 +501,7 @@ impl<'s> ParseTokens<'s> { /// Finish a substream of tokens. fn finish(&mut self) { + self.blocked = false; self.parens.pop().unwrap(); } } @@ -495,11 +510,18 @@ impl<'s> Iterator for ParseTokens<'s> { type Item = Token<'s>; fn next(&mut self) -> Option> { - let token = self.tokens.next(); + if self.blocked { + return None; + } + + let token = self.tokens.peek(); match token { Some(Token::RightBracket) => { match self.parens.last_mut() { - Some(&mut 0) => return None, + Some(&mut 0) => { + self.blocked = true; + return None + }, Some(top) => *top -= 1, None => {} } @@ -511,18 +533,16 @@ impl<'s> Iterator for ParseTokens<'s> { } _ => {} }; - token + self.tokens.next() } } /// The error type for parsing. -pub struct ParseError { - message: String, -} +pub struct ParseError(String); impl ParseError { fn new>(message: S) -> ParseError { - ParseError { message: message.into() } + ParseError(message.into()) } } @@ -531,7 +551,7 @@ pub type ParseResult = Result; error_type! { err: ParseError, - show: f => f.write_str(&err.message), + show: f => f.write_str(&err.0), } @@ -663,7 +683,7 @@ mod parse_tests { impl Function for TreeFn { fn parse(context: ParseContext) -> ParseResult where Self: Sized { if let Some(tokens) = context.tokens { - Parser::with_scope(context.scope, tokens).parse().map(|tree| TreeFn(tree)) + Parser::with_scope(tokens, context.scope).parse().map(|tree| TreeFn(tree)) } else { Err(ParseError::new("expected body for tree fn")) } @@ -697,7 +717,7 @@ mod parse_tests { (@$name:expr, $body:expr) => { FuncCall { header: FuncHeader { - name: Ident::new($name).unwrap(), + name: $name.to_string(), args: vec![], kwargs: HashMap::new(), }, @@ -715,28 +735,29 @@ mod parse_tests { ($($x:expr,)*) => (tree![$($x),*]) } + fn parse(src: &str, scope: &Scope) -> ParseResult { + let mut tokens = ParseTokens::new(src); + Parser::with_scope(&mut tokens, scope).parse() + } + /// Test if the source code parses into the syntax tree. fn test(src: &str, tree: SyntaxTree) { - let mut tokens = ParseTokens::new(src); - assert_eq!(Parser::new(&mut tokens).parse().unwrap(), tree); + assert_eq!(parse(src, &Scope::new()).unwrap(), tree); } /// Test with a scope containing function definitions. fn test_scoped(scope: &Scope, src: &str, tree: SyntaxTree) { - let mut tokens = ParseTokens::new(src); - assert_eq!(Parser::with_scope(scope, &mut tokens).parse().unwrap(), tree); + assert_eq!(parse(src, &scope).unwrap(), tree); } /// Test if the source parses into the error. fn test_err(src: &str, err: &str) { - let mut tokens = ParseTokens::new(src); - assert_eq!(Parser::new(&mut tokens).parse().unwrap_err().message, err); + assert_eq!(parse(src, &Scope::new()).unwrap_err().to_string(), err); } /// Test with a scope if the source parses into the error. fn test_err_scoped(scope: &Scope, src: &str, err: &str) { - let mut tokens = ParseTokens::new(src); - assert_eq!(Parser::with_scope(scope, &mut tokens).parse().unwrap_err().message, err); + assert_eq!(parse(src, &scope).unwrap_err().to_string(), err); } /// Parse the basic cases. diff --git a/src/syntax.rs b/src/syntax.rs index 8d248ab7c..bad7ab261 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -1,11 +1,7 @@ //! Tokenized and syntax tree representations of source code. use std::collections::HashMap; -use std::fmt::{self, Display, Formatter}; -use std::ops::Deref; - use crate::func::Function; -use crate::utility::StrExt; /// A logical unit of the incoming text stream. @@ -72,7 +68,7 @@ pub enum Node { Func(FuncCall), } -/// A complete function invocation consisting of header and body. +/// A function invocation consisting of header and body. #[derive(Debug)] pub struct FuncCall { pub header: FuncHeader, @@ -88,49 +84,11 @@ impl PartialEq for FuncCall { /// Contains header information of a function invocation. #[derive(Debug, Clone, PartialEq)] pub struct FuncHeader { - pub name: Ident, + pub name: String, pub args: Vec, - pub kwargs: HashMap + pub kwargs: HashMap } /// A potentially unevaluated expression. #[derive(Debug, Clone, PartialEq)] pub enum Expression {} - -/// An owned valid identifier. -#[derive(Debug, Clone, Eq, PartialEq, Hash)] -pub struct Ident(String); - -impl Ident { - /// Create a new identifier if the string is a valid one. - #[inline] - pub fn new>(ident: S) -> Option { - let ident = ident.into(); - if ident.is_identifier() { - Some(Ident(ident)) - } else { - None - } - } - - /// Consume self and return the underlying string. - #[inline] - pub fn into_inner(self) -> String { - self.0 - } -} - -impl Display for Ident { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - Display::fmt(&self.0, f) - } -} - -impl Deref for Ident { - type Target = str; - - #[inline] - fn deref(&self) -> &str { - &*self.0 - } -}