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 a few helpful template filters #336

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

orsinium
Copy link
Contributor

@orsinium orsinium commented Oct 5, 2020

ContainsAny

I patched ContainsAny filter to be able to check if the given string contains any of the given substrings.

For example, this one:

{{test or (Contains .title "dephell") (Contains .title "flakehell") (Contains .title "orsinium.dev")}}

Can now be transformed into much simpler and shorter expression:

{{test ContainsAny .title "dephell" "flakehell" "orsinium.dev"}}

If only 2 strings are passed into the filter, it behaves as it used to, checking every symbol from the second string to be represented in the first one. While I cannot imagine a use case for it in beehive, let's keep the backward compatibility. It should be ok because ContainsAny looking only for one substring would be the same as just Contains.

ContainsAll

If we have ContainsAny, why not to have ContainsAll? I've added a new filter to check if a string contains all substrings.

Array

There is no syntax in the template engine to create an array but some functions accept arrays. It can be emulated with Split but it's not nice looking. So, let's make one.

{{Array "flakehell" "dephell"}}

More tests

I've added more tests for string functions. Since there is no way to edit the wiki from the master branch (where is it, BTW?), let's at least have the code "self-documented".

@muesli
Copy link
Owner

muesli commented Oct 10, 2020

If only 2 strings are passed into the filter, it behaves as it used to, checking every symbol from the second string to be represented in the first one. While I cannot imagine a use case for it in beehive, let's keep the backward compatibility. It should be ok because ContainsAny looking only for one substring would be the same as just Contains.

I really don't like this kind of ambiguity. Why not introduce a separate function for that behavior?

Array

Nice!

More tests

Awesome!

I've added more tests for string functions. Since there is no way to edit the wiki from the master branch (where is it, BTW?), let's at least have the code "self-documented".

GitHub wikis are in a separate git repository: https://github.com/muesli/beehive.wiki.git

@orsinium
Copy link
Contributor Author

I really don't like this kind of ambiguity.

Me too. However, the new behavior (check if any of substrings is in the string) is what I would expect from ContainsAny. I'd remove the old behavior because ContainsAny "something" "abc" is the same as ContainsAny "something" "a" "b" "c" from now on but it would be a breaking change.

Why not introduce a separate function for that behavior?

How would you name it?

@muesli
Copy link
Owner

muesli commented Oct 15, 2020

I can't come up with anything better than ContainsAnyString 😞 Still open for suggestions 😃

@orsinium
Copy link
Contributor Author

A few suggestions:

  1. Accept an array of strings as the second argument for the new behavior instead of variadic args.
  2. Screw the backward compatibility, return an error if only 2 arguments were passed like it is done in ContainsAll. The old implementation of ContainsAny rename to ContainsAnyRune.
  3. Don't touch ContainsAny, extend Contains instead.
  4. Don't worry about mixing behaviors (the implementation in this PR).

@orsinium
Copy link
Contributor Author

Does it make sense for me to keep the PR open any longer or should I just close it? It's ok to not want to merge some random contributions. I'm long enough in open source to understand :)

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.

2 participants