Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional support for lax rendering and parsing #492

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions crates/core/src/parser/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,3 +254,19 @@ where
Box::new(filter)
}
}

/// A filter used internally when parsing is done in lax mode.
#[derive(Debug, Default)]
pub struct NoopFilter;

impl Filter for NoopFilter {
fn evaluate(&self, input: &dyn ValueView, _runtime: &dyn Runtime) -> Result<Value> {
Ok(input.to_value())
}
}

impl Display for NoopFilter {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
::std::write!(f, "{}", "noop")
}
}
13 changes: 13 additions & 0 deletions crates/core/src/parser/lang.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,25 @@ use super::ParseFilter;
use super::ParseTag;
use super::PluginRegistry;

#[derive(Clone)]
pub enum ParseMode {
Comment on lines +6 to +7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should also have Copy, Debug, PartialEq and Eq

Strict,
Lax,
}
Comment on lines +6 to +10
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, no docs on this


impl Default for ParseMode {
fn default() -> Self {
Self::Strict
}
}
Comment on lines +6 to +16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very general name that only covers filters. The name could imply tags, blocks, syntax errors as well.

Should we make this more specific?

Also, the line for what is parse vs render might be a bit fuzzy for a user that limiting these to just Render and Parse modes obscures the behavior.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very general name that only covers filters. The name could imply tags, blocks, syntax errors as well.

Should we make this more specific?

Fair. I can update the name to be more specific.

Also, the line for what is parse vs render might be a bit fuzzy for a user that limiting these to just Render and Parse modes obscures the behavior.

Can you elaborate a bit more what you mean here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate a bit more what you mean here?

Is it required that filter lookups happen during parse or could it happen during runtime? At the moment, that is an implementation detail that can change.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that the implementation can change, but I personally think the way it's currently implemented is the correct behavior.

The way it's currently implemented allows you to store a template that are parsed once and reused multiple times. "Lazily" parsing the template means you delay errors that IMO should be handled when the template itself is created and in turn parsed.

Happy to explore alternative options if you think that's the right approach.


#[derive(Clone, Default)]
#[non_exhaustive]
pub struct Language {
pub blocks: PluginRegistry<Box<dyn ParseBlock>>,
pub tags: PluginRegistry<Box<dyn ParseTag>>,
pub filters: PluginRegistry<Box<dyn ParseFilter>>,
pub mode: ParseMode,
}

impl Language {
Expand Down
64 changes: 47 additions & 17 deletions crates/core/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ use crate::runtime::Expression;
use crate::runtime::Renderable;
use crate::runtime::Variable;

use super::Language;
use super::Text;
use super::{Filter, FilterArguments, FilterChain};
use super::{Language, ParseMode};

use pest::Parser;

Expand Down Expand Up @@ -205,22 +205,38 @@ fn parse_filter(filter: Pair, options: &Language) -> Result<Box<dyn Filter>> {
keyword: Box::new(keyword_args.into_iter()),
};

let f = options.filters.get(name).ok_or_else(|| {
let mut available: Vec<_> = options.filters.plugin_names().collect();
available.sort_unstable();
let available = itertools::join(available, ", ");
Error::with_msg("Unknown filter")
.context("requested filter", name.to_owned())
.context("available filters", available)
})?;

let f = f
.parse(args)
.trace("Filter parsing error")
.context_key("filter")
.value_with(|| filter_str.to_string().into())?;

Ok(f)
match options.mode {
ParseMode::Strict => {
let f = options.filters.get(name).ok_or_else(|| {
let mut available: Vec<_> = options.filters.plugin_names().collect();
available.sort_unstable();
let available = itertools::join(available, ", ");
Error::with_msg("Unknown filter")
.context("requested filter", name.to_owned())
.context("available filters", available)
})?;

let f = f
.parse(args)
.trace("Filter parsing error")
.context_key("filter")
.value_with(|| filter_str.to_string().into())?;

Ok(f)
}
ParseMode::Lax => match options.filters.get(name) {
Some(f) => {
let f = f
.parse(args)
.trace("Filter parsing error")
.context_key("filter")
.value_with(|| filter_str.to_string().into())?;

Ok(f)
}
None => Ok(Box::new(super::NoopFilter {})),
},
}
}

/// Parses a `FilterChain` from a `Pair` with a filter chain.
Expand Down Expand Up @@ -1172,6 +1188,20 @@ mod test {
assert_eq!(output, "5");
}

#[test]
fn test_parse_mode_filters() {
let mut options = Language::default();
let text = "{{ exp | undefined }}";

options.mode = ParseMode::Strict;
let result = parse(text, &options);
assert_eq!(result.is_err(), true);

options.mode = ParseMode::Lax;
let result = parse(text, &options);
assert_eq!(result.is_err(), false);
}

/// Macro implementation of custom block test.
macro_rules! test_custom_block_tags_impl {
($start_tag:expr, $end_tag:expr) => {{
Expand Down
37 changes: 36 additions & 1 deletion crates/core/src/runtime/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,13 @@ impl Expression {
Expression::Literal(ref x) => ValueCow::Borrowed(x),
Expression::Variable(ref x) => {
let path = x.evaluate(runtime)?;
runtime.get(&path)?

match runtime.render_mode() {
super::RenderingMode::Lax => {
runtime.try_get(&path).unwrap_or_else(|| Value::Nil.into())
}
_ => runtime.get(&path)?,
}
}
};
Ok(val)
Expand All @@ -72,3 +78,32 @@ impl fmt::Display for Expression {
}
}
}

#[cfg(test)]
mod test {
use super::*;

use crate::model::Object;
use crate::model::Value;
use crate::runtime::RenderingMode;
use crate::runtime::RuntimeBuilder;
use crate::runtime::StackFrame;

#[test]
fn test_rendering_mode() {
let globals = Object::new();
let expression = Expression::Variable(Variable::with_literal("test"));

let runtime = RuntimeBuilder::new()
.set_render_mode(RenderingMode::Strict)
.build();
let runtime = StackFrame::new(&runtime, &globals);
assert_eq!(expression.evaluate(&runtime).is_err(), true);

let runtime = RuntimeBuilder::new()
.set_render_mode(RenderingMode::Lax)
.build();
let runtime = StackFrame::new(&runtime, &globals);
assert_eq!(expression.evaluate(&runtime).unwrap(), Value::Nil);
}
}
36 changes: 36 additions & 0 deletions crates/core/src/runtime/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ use crate::model::{Object, ObjectView, Scalar, ScalarCow, Value, ValueCow, Value
use super::PartialStore;
use super::Renderable;

/// What mode to use when rendering.
pub enum RenderingMode {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should have Copy, Clone, Debug, PartialEq, Eq, and Default

/// Returns an error when a variable is not defined.
Strict,
/// Replaces missing variables with an empty string.
Lax,
}
Comment on lines +10 to +16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to my comments about ParseMode, I feel like this might be too general


/// State for rendering a template
pub trait Runtime {
/// Partial templates for inclusion.
Expand Down Expand Up @@ -36,6 +44,9 @@ pub trait Runtime {

/// Unnamed state for plugins during rendering
fn registers(&self) -> &Registers;

/// Used to set the mode when rendering
fn render_mode(&self) -> &RenderingMode;
}

impl<'r, R: Runtime + ?Sized> Runtime for &'r R {
Expand Down Expand Up @@ -78,12 +89,17 @@ impl<'r, R: Runtime + ?Sized> Runtime for &'r R {
fn registers(&self) -> &super::Registers {
<R as Runtime>::registers(self)
}

fn render_mode(&self) -> &RenderingMode {
<R as Runtime>::render_mode(self)
}
}

/// Create processing runtime for a template.
pub struct RuntimeBuilder<'g, 'p> {
globals: Option<&'g dyn ObjectView>,
partials: Option<&'p dyn PartialStore>,
render_mode: RenderingMode,
}

impl<'c, 'g: 'c, 'p: 'c> RuntimeBuilder<'g, 'p> {
Expand All @@ -92,6 +108,7 @@ impl<'c, 'g: 'c, 'p: 'c> RuntimeBuilder<'g, 'p> {
Self {
globals: None,
partials: None,
render_mode: RenderingMode::Strict,
}
}

Expand All @@ -100,6 +117,7 @@ impl<'c, 'g: 'c, 'p: 'c> RuntimeBuilder<'g, 'p> {
RuntimeBuilder {
globals: Some(values),
partials: self.partials,
render_mode: self.render_mode,
}
}

Expand All @@ -108,6 +126,16 @@ impl<'c, 'g: 'c, 'p: 'c> RuntimeBuilder<'g, 'p> {
RuntimeBuilder {
globals: self.globals,
partials: Some(values),
render_mode: self.render_mode,
}
}

/// Initialize with the provided rendering mode.
pub fn set_render_mode(self, mode: RenderingMode) -> RuntimeBuilder<'g, 'p> {
RuntimeBuilder {
globals: self.globals,
partials: self.partials,
render_mode: mode,
}
}

Expand All @@ -116,6 +144,7 @@ impl<'c, 'g: 'c, 'p: 'c> RuntimeBuilder<'g, 'p> {
let partials = self.partials.unwrap_or(&NullPartials);
let runtime = RuntimeCore {
partials,
render_mode: self.render_mode,
..Default::default()
};
let runtime = super::IndexFrame::new(runtime);
Expand Down Expand Up @@ -208,6 +237,8 @@ pub struct RuntimeCore<'g> {
partials: &'g dyn PartialStore,

registers: Registers,

render_mode: RenderingMode,
}

impl<'g> RuntimeCore<'g> {
Expand Down Expand Up @@ -268,13 +299,18 @@ impl<'g> Runtime for RuntimeCore<'g> {
fn registers(&self) -> &Registers {
&self.registers
}

fn render_mode(&self) -> &RenderingMode {
&self.render_mode
}
}

impl<'g> Default for RuntimeCore<'g> {
fn default() -> Self {
Self {
partials: &NullPartials,
registers: Default::default(),
render_mode: RenderingMode::Strict,
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions crates/core/src/runtime/stack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ impl<P: super::Runtime, O: ObjectView> super::Runtime for StackFrame<P, O> {
fn registers(&self) -> &super::Registers {
self.parent.registers()
}

fn render_mode(&self) -> &super::RenderingMode {
self.parent.render_mode()
}
}

pub(crate) struct GlobalFrame<P> {
Expand Down Expand Up @@ -162,6 +166,10 @@ impl<P: super::Runtime> super::Runtime for GlobalFrame<P> {
fn registers(&self) -> &super::Registers {
self.parent.registers()
}

fn render_mode(&self) -> &super::RenderingMode {
self.parent.render_mode()
}
}

pub(crate) struct IndexFrame<P> {
Expand Down Expand Up @@ -237,4 +245,8 @@ impl<P: super::Runtime> super::Runtime for IndexFrame<P> {
fn registers(&self) -> &super::Registers {
self.parent.registers()
}

fn render_mode(&self) -> &super::RenderingMode {
self.parent.render_mode()
}
}
13 changes: 13 additions & 0 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::sync;

use liquid_core::error::{Result, ResultLiquidExt, ResultLiquidReplaceExt};
use liquid_core::parser;
use liquid_core::parser::ParseMode;
use liquid_core::runtime;

use super::Template;
Expand All @@ -19,6 +20,7 @@ pub struct ParserBuilder<P = Partials>
where
P: partials::PartialCompiler,
{
mode: parser::ParseMode,
blocks: parser::PluginRegistry<Box<dyn parser::ParseBlock>>,
tags: parser::PluginRegistry<Box<dyn parser::ParseTag>>,
filters: parser::PluginRegistry<Box<dyn parser::ParseFilter>>,
Expand Down Expand Up @@ -110,6 +112,12 @@ where
.filter(stdlib::Where)
}

/// Sets the parse mode to lax.
pub fn in_lax_mode(mut self) -> Self {
self.mode = ParseMode::Lax;
self
}
Comment on lines +115 to +119
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd rather re-export the num and accept it.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll update, I did it this way since there's only 2 options currently, 1 of which is the default. So exposing a function allowed us to now have to worry about exposing the enum to the user.


/// Inserts a new custom block into the parser
pub fn block<B: Into<Box<dyn parser::ParseBlock>>>(mut self, block: B) -> Self {
let block = block.into();
Expand All @@ -136,12 +144,14 @@ where
/// Set which partial-templates will be available.
pub fn partials<N: partials::PartialCompiler>(self, partials: N) -> ParserBuilder<N> {
let Self {
mode,
blocks,
tags,
filters,
partials: _partials,
} = self;
ParserBuilder {
mode,
blocks,
tags,
filters,
Expand All @@ -152,13 +162,15 @@ where
/// Create a parser
pub fn build(self) -> Result<Parser> {
let Self {
mode,
blocks,
tags,
filters,
partials,
} = self;

let mut options = parser::Language::empty();
options.mode = mode;
options.blocks = blocks;
options.tags = tags;
options.filters = filters;
Expand All @@ -178,6 +190,7 @@ where
{
fn default() -> Self {
Self {
mode: Default::default(),
blocks: Default::default(),
tags: Default::default(),
filters: Default::default(),
Expand Down