From 3fd92f070433a6d938189e8e50ae6c687e540a0b Mon Sep 17 00:00:00 2001 From: James Prior Date: Fri, 16 Aug 2024 08:47:00 +0100 Subject: [PATCH 1/2] Handle interrupts in tablerow tags --- CHANGES.md | 4 +++ liquid/builtin/tags/tablerow_tag.py | 43 +++++++++++++++++++--- liquid/future/environment.py | 2 ++ liquid/future/tags/__init__.py | 2 ++ liquid/future/tags/_tablerow_tag.py | 14 ++++++++ liquid/golden/tablerow_tag.py | 55 +++++++++++++++++++++++++++++ 6 files changed, 116 insertions(+), 4 deletions(-) create mode 100644 liquid/future/tags/_tablerow_tag.py diff --git a/CHANGES.md b/CHANGES.md index 53fd57d6..d56f9f28 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,10 @@ - Fixed `{% case %}` / `{% when %}` behavior. When using [`liquid.future.Environment`](https://jg-rp.github.io/liquid/api/future-environment), we now render any number of `{% else %}` blocks and allow `{% when %}` tags to appear after `{% else %}` tags. The default `Environment` continues to raise a `LiquidSyntaxError` in such cases. +**Changed** + +- Changed `{% break %}` and `{% continue %}` tag handling when they appear inside a `{% tablerow %}` tag. Now, when using `liquid.future.Environment`, interrupts follow Shopify/Liquid behavior introduced in [#1818](https://github.com/Shopify/liquid/pull/1818). Python Liquid's default environment is unchanged. + ## Version 1.12.1 **Fixes** diff --git a/liquid/builtin/tags/tablerow_tag.py b/liquid/builtin/tags/tablerow_tag.py index 1dd68827..228dbb60 100644 --- a/liquid/builtin/tags/tablerow_tag.py +++ b/liquid/builtin/tags/tablerow_tag.py @@ -12,6 +12,8 @@ from liquid.ast import ChildNode from liquid.ast import Node from liquid.context import Context +from liquid.exceptions import BreakLoop +from liquid.exceptions import ContinueLoop from liquid.expression import NIL from liquid.expression import LoopExpression from liquid.limits import to_int @@ -156,6 +158,9 @@ def step(self) -> None: class TablerowNode(Node): """Parse tree node for the built-in "tablerow" tag.""" + interrupts = False + """If _true_, handle `break` and `continue` interrupts inside a tablerow loop.""" + __slots__ = ("tok", "expression", "block") def __init__( @@ -194,18 +199,33 @@ def render_to_output(self, context: Context, buffer: TextIO) -> Optional[bool]: } buffer.write('\n') + _break = False with context.extend(namespace): for item in tablerow: namespace[name] = item buffer.write(f'') - self.block.render(context=context, buffer=buffer) + + try: + self.block.render(context=context, buffer=buffer) + except BreakLoop: + if self.interrupts: + _break = True + else: + raise + except ContinueLoop: + if not self.interrupts: + raise + buffer.write("") if tablerow.col_last and not tablerow.last: buffer.write(f'\n') - buffer.write("\n") + if _break: + break + + buffer.write("\n") return True async def render_to_output_async( @@ -227,18 +247,33 @@ async def render_to_output_async( } buffer.write('\n') + _break = False with context.extend(namespace): for item in tablerow: namespace[name] = item buffer.write(f'') - await self.block.render_async(context=context, buffer=buffer) + + try: + await self.block.render_async(context=context, buffer=buffer) + except BreakLoop: + if self.interrupts: + _break = True + else: + raise + except ContinueLoop: + if not self.interrupts: + raise + buffer.write("") if tablerow.col_last and not tablerow.last: buffer.write(f'\n') - buffer.write("\n") + if _break: + break + + buffer.write("\n") return True def children(self) -> List[ChildNode]: diff --git a/liquid/future/environment.py b/liquid/future/environment.py index e4807d52..6bd3dbd0 100644 --- a/liquid/future/environment.py +++ b/liquid/future/environment.py @@ -6,6 +6,7 @@ from ..environment import Environment as DefaultEnvironment from ..template import FutureBoundTemplate from .filters import split +from .tags import InterruptingTablerowTag from .tags import LaxCaseTag from .tags import LaxIfTag from .tags import LaxUnlessTag @@ -31,3 +32,4 @@ def setup_tags_and_filters(self) -> None: self.add_tag(LaxCaseTag) self.add_tag(LaxIfTag) self.add_tag(LaxUnlessTag) + self.add_tag(InterruptingTablerowTag) diff --git a/liquid/future/tags/__init__.py b/liquid/future/tags/__init__.py index f33af1f1..d3b2b08e 100644 --- a/liquid/future/tags/__init__.py +++ b/liquid/future/tags/__init__.py @@ -1,8 +1,10 @@ from ._case_tag import LaxCaseTag # noqa: D104 from ._if_tag import LaxIfTag +from ._tablerow_tag import InterruptingTablerowTag from ._unless_tag import LaxUnlessTag __all__ = ( + "InterruptingTablerowTag", "LaxCaseTag", "LaxIfTag", "LaxUnlessTag", diff --git a/liquid/future/tags/_tablerow_tag.py b/liquid/future/tags/_tablerow_tag.py new file mode 100644 index 00000000..47ed5708 --- /dev/null +++ b/liquid/future/tags/_tablerow_tag.py @@ -0,0 +1,14 @@ +from liquid.builtin.tags.tablerow_tag import TablerowNode +from liquid.builtin.tags.tablerow_tag import TablerowTag + + +class InterruptingTablerowNode(TablerowNode): + """A _tablerow_ node with interrupt handling enabled.""" + + interrupts = True + + +class InterruptingTablerowTag(TablerowTag): + """A _tablerow_ tag that handles `break` and `continue` tags.""" + + node_class = InterruptingTablerowNode diff --git a/liquid/golden/tablerow_tag.py b/liquid/golden/tablerow_tag.py index b2648aa9..96aeb64f 100644 --- a/liquid/golden/tablerow_tag.py +++ b/liquid/golden/tablerow_tag.py @@ -282,6 +282,61 @@ "\n" ), ), + Case( + description="break from a tablerow loop", + template=( + r"{% tablerow n in (1..3) cols:2 %}" + r"{{n}}{% break %}{{n}}" + r"{% endtablerow %}" + ), + expect='\n1\n', + future=True, + ), + Case( + description="continue from a tablerow loop", + template=( + r"{% tablerow n in (1..3) cols:2 %}" + r"{{n}}{% continue %}{{n}}" + r"{% endtablerow %}" + ), + expect=( + '\n' + '1' + '2' + "\n" + '' + '3' + "\n" + ), + future=True, + ), + Case( + description="break from a tablerow loop inside a for loop", + template=( + r"{% for i in (1..2) -%}\n" + r"{% for j in (1..2) -%}\n" + r"{% tablerow k in (1..3) %}{% break %}{% endtablerow -%}\n" + r"loop j={{ j }}\n" + r"{% endfor -%}\n" + r"loop i={{ i }}\n" + r"{% endfor -%}\n" + r"after loop\n" + ), + expect="\n".join( + [ + r'\n\n', + r'', + r'\nloop j=1\n\n', + r'', + r'\nloop j=2\n\nloop i=1\n\n\n', + r'', + r'\nloop j=1\n\n', + r'', + r"\nloop j=2\n\nloop i=2\n\nafter loop\n", + ] + ), + future=True, + ), # Case( # description="cols is non number string", # template=( From 9c2ddf06cde7aa6055f444a412c6b657f3bdab94 Mon Sep 17 00:00:00 2001 From: James Prior Date: Fri, 16 Aug 2024 09:02:29 +0100 Subject: [PATCH 2/2] Fix lint issues --- liquid/context.py | 4 +- liquid/expression.py | 15 ++---- liquid/parse.py | 1 - pyproject.toml | 107 ++++++++++++++++++++----------------- tests/filters/test_misc.py | 13 ++--- 5 files changed, 67 insertions(+), 73 deletions(-) diff --git a/liquid/context.py b/liquid/context.py index 24c658cd..03038eb0 100644 --- a/liquid/context.py +++ b/liquid/context.py @@ -72,9 +72,7 @@ class BuiltIn(Mapping[str, object]): """Mapping-like object for resolving built-in, dynamic objects.""" def __contains__(self, item: object) -> bool: - if item in ("now", "today"): - return True - return False + return item in ("now", "today") def __getitem__(self, key: str) -> object: if key == "now": diff --git a/liquid/expression.py b/liquid/expression.py index c685bbac..2f07204e 100644 --- a/liquid/expression.py +++ b/liquid/expression.py @@ -1,4 +1,5 @@ """Liquid expression objects.""" + from __future__ import annotations import sys @@ -79,9 +80,7 @@ class Empty(Expression): def __eq__(self, other: object) -> bool: if isinstance(other, Empty): return True - if isinstance(other, (list, dict, str)) and not other: - return True - return False + return isinstance(other, (list, dict, str)) and not other def __repr__(self) -> str: # pragma: no cover return "Empty()" @@ -107,9 +106,7 @@ def __eq__(self, other: object) -> bool: return True if isinstance(other, (list, dict)) and not other: return True - if isinstance(other, Blank): - return True - return False + return isinstance(other, Blank) def __repr__(self) -> str: # pragma: no cover return "Blank()" @@ -131,9 +128,7 @@ class Continue(Expression): __slots__ = () def __eq__(self, other: object) -> bool: - if isinstance(other, Continue): - return True - return False + return isinstance(other, Continue) def __repr__(self) -> str: # pragma: no cover return "Continue()" @@ -1106,7 +1101,7 @@ def compare(left: object, op: str, right: object) -> bool: # noqa: PLR0911, PLR right = right.__liquid__() def _type_error(_left: object, _right: object) -> NoReturn: - if type(_left) != type(_right): + if type(_left) != type(_right): # noqa: E721 raise LiquidTypeError(f"invalid operator for types '{_left} {op} {_right}'") raise LiquidTypeError(f"unknown operator: {type(_left)} {op} {type(_right)}") diff --git a/liquid/parse.py b/liquid/parse.py index 4d07ae65..c6e2ed41 100644 --- a/liquid/parse.py +++ b/liquid/parse.py @@ -615,7 +615,6 @@ def parse_filter(self, stream: TokenStream) -> expression.Filter: filter_name = stream.current.value stream.next_token() - # args = [] kwargs = {} diff --git a/pyproject.toml b/pyproject.toml index 8a1a1a2c..b897e8fb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -3,7 +3,7 @@ build-backend = "hatchling.build" requires = ["hatchling"] [project] -authors = [{name = "James Prior", email = "jamesgr.prior@gmail.com"}] +authors = [{ name = "James Prior", email = "jamesgr.prior@gmail.com" }] classifiers = [ "Development Status :: 5 - Production/Stable", "Intended Audience :: Developers", @@ -18,7 +18,11 @@ classifiers = [ "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", ] -dependencies = ["python-dateutil>=2.8.1", "typing-extensions>=4.2.0", "importlib-resources>=5.10.0"] +dependencies = [ + "python-dateutil>=2.8.1", + "typing-extensions>=4.2.0", + "importlib-resources>=5.10.0", +] description = "A Python engine for the Liquid template language." dynamic = ["version"] license = "MIT" @@ -126,6 +130,56 @@ warn_unused_configs = true warn_unused_ignores = false [tool.ruff] + +# Exclude a variety of commonly ignored directories. +exclude = [ + ".bzr", + ".direnv", + ".eggs", + ".git", + ".hg", + ".mypy_cache", + ".nox", + ".pants.d", + ".pytype", + ".ruff_cache", + ".svn", + ".tox", + ".venv", + "__pypackages__", + "_build", + "buck-out", + "build", + "dist", + "node_modules", + "venv", +] + +# Same as Black. +line-length = 88 + +# Assume Python 3.10. +target-version = "py310" + +[tool.ruff.lint.isort] +force-single-line = true + +[tool.ruff.lint.pydocstyle] +convention = "google" + +[tool.ruff.lint.per-file-ignores] +"liquid/__about__.py" = ["D100"] +"liquid/__init__.py" = ["D104", "I001"] +"liquid/builtin/filters/__init__.py" = ["D104", "I001"] +"liquid/builtin/loaders/__init__.py" = ["D104", "I001"] +"liquid/builtin/tags/__init__.py" = ["D104", "I001"] +"scripts/__init__.py" = ["D104", "I001"] +"tests/*" = ["D100", "D101", "D104", "D103", "D102", "D209", "D205", "SIM117"] + +[tool.ruff.lint] +# Allow unused variables when underscore-prefixed. +dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" + select = [ "A", "ARG", @@ -153,6 +207,7 @@ select = [ "TCH", "YTT", ] + # TODO: review ignores ignore = [ "S105", @@ -169,51 +224,3 @@ ignore = [ fixable = ["I", "SIM", "D202"] unfixable = [] - -# Exclude a variety of commonly ignored directories. -exclude = [ - ".bzr", - ".direnv", - ".eggs", - ".git", - ".hg", - ".mypy_cache", - ".nox", - ".pants.d", - ".pytype", - ".ruff_cache", - ".svn", - ".tox", - ".venv", - "__pypackages__", - "_build", - "buck-out", - "build", - "dist", - "node_modules", - "venv", -] - -# Same as Black. -line-length = 88 - -# Allow unused variables when underscore-prefixed. -dummy-variable-rgx = "^(_+|(_+[a-zA-Z0-9_]*[a-zA-Z0-9]+?))$" - -# Assume Python 3.10. -target-version = "py310" - -[tool.ruff.isort] -force-single-line = true - -[tool.ruff.pydocstyle] -convention = "google" - -[tool.ruff.per-file-ignores] -"liquid/__about__.py" = ["D100"] -"liquid/__init__.py" = ["D104", "I001"] -"liquid/builtin/filters/__init__.py" = ["D104", "I001"] -"liquid/builtin/loaders/__init__.py" = ["D104", "I001"] -"liquid/builtin/tags/__init__.py" = ["D104", "I001"] -"scripts/__init__.py" = ["D104", "I001"] -"tests/*" = ["D100", "D101", "D104", "D103", "D102", "D209", "D205", "SIM117"] diff --git a/tests/filters/test_misc.py b/tests/filters/test_misc.py index f0588942..c479f44b 100644 --- a/tests/filters/test_misc.py +++ b/tests/filters/test_misc.py @@ -1,4 +1,5 @@ """Test miscellaneous filter functions.""" + import datetime import decimal import platform @@ -32,9 +33,7 @@ def __init__(self, val): self.val = val def __eq__(self, other): - if isinstance(other, MockDrop) and self.val == other.val: - return True - return False + return bool(isinstance(other, MockDrop) and self.val == other.val) def __str__(self): return "hello mock drop" @@ -48,9 +47,7 @@ def __init__(self, val): self.val = val def __eq__(self, other): - if isinstance(other, NoLiquidDrop) and self.val == other.val: - return True - return False + return bool(isinstance(other, NoLiquidDrop) and self.val == other.val) def __str__(self): return "hello no liquid drop" @@ -63,9 +60,7 @@ def __init__(self, val): def __eq__(self, other): if isinstance(other, bool) and self.val == other: return True - if isinstance(other, FalsyDrop) and self.val == other.val: - return True - return False + return bool(isinstance(other, FalsyDrop) and self.val == other.val) def __str__(self): return "falsy drop"