Skip to content

Commit

Permalink
Fix split filter compatibility issues (#135)
Browse files Browse the repository at this point in the history
* Add failing `split` filter test cases. See #134.

* Add compatible implementation of `split`.

* Fix left matches repr of argument.

* Simplify by combining condition.

* Add more to doc comment.

* Update change log and bump version number.
  • Loading branch information
jg-rp authored Dec 14, 2023
1 parent c6af741 commit a60531c
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 1 deletion.
6 changes: 6 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Python Liquid Change Log

## Version 1.11.0 (unreleased)

**Fixes**

- Added an additional implementation of the `split` filter, which resolves some compatibility issues between Python Liquid's and the reference implementation. Previously, when given an empty string to split or when the string and the delimiter were equal, we used Python's `str.split()` behavior of producing one or two element lists containing empty strings. We now match Shopify/Liquid in returning an empty list for such cases. The new `split` filter will be enabled by default when using [`liquid.future.Environment`](https://jg-rp.github.io/liquid/api/future-environment), and can optionally be registered with `liquid.Environment` for those that don't mind the behavioral change. See [#135](https://github.com/jg-rp/liquid/pull/135).

## Version 1.10.1

**Fixes**
Expand Down
2 changes: 1 addition & 1 deletion liquid/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@

from . import future

__version__ = "1.10.1"
__version__ = "1.11.0"

__all__ = (
"AwareBoundTemplate",
Expand Down
6 changes: 6 additions & 0 deletions liquid/future/environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"""
from ..environment import Environment as DefaultEnvironment
from ..template import FutureBoundTemplate
from .filters import split


class Environment(DefaultEnvironment):
Expand All @@ -19,3 +20,8 @@ class Environment(DefaultEnvironment):
"""

template_class = FutureBoundTemplate

def setup_tags_and_filters(self) -> None:
"""Add future tags and filters to this environment."""
super().setup_tags_and_filters()
self.add_filter("split", split)
3 changes: 3 additions & 0 deletions liquid/future/filters/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from ._split import split # noqa: D104

__all__ = ("split",)
23 changes: 23 additions & 0 deletions liquid/future/filters/_split.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
"""An implementation of the `split` filter."""

from typing import List

from liquid import soft_str
from liquid.filter import string_filter


@string_filter
def split(val: str, sep: str) -> List[str]:
"""Split string _val_ on delimiter _sep_.
If _sep_ is empty or _undefined_, _val_ is split into a list of single
characters. If _val_ is empty or equal to _sep_, an empty list is returned.
"""
if not sep:
return list(val)

sep = soft_str(sep)
if not val or val == sep:
return []

return val.split(sep)
54 changes: 54 additions & 0 deletions liquid/golden/split_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,58 @@
template=r'{{ "Hello there" | split: nosuchthing | join: "#" }}',
expect="H#e#l#l#o# #t#h#e#r#e",
),
Case(
description="empty string argument",
template=(
r'{% assign a = "abc" | split: "" %}'
r"{% for i in a %}#{{ forloop.index0 }}{{ i }}{% endfor %}"
),
expect="#0a#1b#2c",
future=True,
),
Case(
description="argument does not appear in string",
template=(
r'{% assign a = "abc" | split: "," %}'
r"{% for i in a %}#{{ forloop.index0 }}{{ i }}{% endfor %}"
),
expect="#0abc",
future=True,
),
Case(
description="empty string and empty argument",
template=(
r'{% assign a = "" | split: "" %}'
r"{% for i in a %}{{ forloop.index0 }}{{ i }}{% endfor %}"
),
expect="",
future=True,
),
Case(
description="empty string and single char argument",
template=(
r'{% assign a = "" | split: "," %}'
r"{% for i in a %}{{ forloop.index0 }}{{ i }}{% endfor %}"
),
expect="",
future=True,
),
Case(
description="left matches argument",
template=(
r'{% assign a = "," | split: "," %}'
r"{% for i in a %}{{ forloop.index0 }}{{ i }}{% endfor %}"
),
expect="",
future=True,
),
Case(
description="left matches string repr of argument",
template=(
r'{% assign a = "1" | split: 1 %}'
r"{% for i in a %}{{ forloop.index0 }}{{ i }}{% endfor %}"
),
expect="",
future=True,
),
]

0 comments on commit a60531c

Please sign in to comment.