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 1 commit
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
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