Skip to content

Add replace_last and remove_last filters#515

Merged
sebastienros merged 5 commits intosebastienros:mainfrom
danielmpetrov:feature/gh-494
Oct 18, 2022
Merged

Add replace_last and remove_last filters#515
sebastienros merged 5 commits intosebastienros:mainfrom
danielmpetrov:feature/gh-494

Conversation

@danielmpetrov
Copy link
Copy Markdown
Contributor

Bonus - add a missing assert statement in RemovesReturnsInputWhenArgumentIsEmpty unit test.

Resolve #494

@sebastienros
Copy link
Copy Markdown
Owner

Awesome!

Can you also add the missing test cases from here:
https://github.com/Shopify/liquid/pull/1422/files#diff-783fde0f471731937b1ede37eff3107dae698e98dc76dda994391f49ca73958aR542-R566

This contains some with types that are not strings (I see it's implemented but not tested).

@danielmpetrov
Copy link
Copy Markdown
Contributor Author

Thanks for the quick feedback.

No problem, I can add those. Need to run now, but will update the PR later today or likely tomorrow.

@hishamco
Copy link
Copy Markdown
Collaborator

Can you use Theory instead to examine different use cases for both unit tests

1 similar comment
@hishamco
Copy link
Copy Markdown
Collaborator

Can you use Theory instead to examine different use cases for both unit tests

@sebastienros sebastienros merged commit 63bade9 into sebastienros:main Oct 18, 2022
@sebastienros
Copy link
Copy Markdown
Owner

I added an extra unit test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement replace_last and replace_first filters

3 participants