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

Inconsistency with lower & upper #28

Open
wchresta opened this issue Jun 10, 2018 · 2 comments
Open

Inconsistency with lower & upper #28

wchresta opened this issue Jun 10, 2018 · 2 comments

Comments

@wchresta
Copy link
Contributor

wchresta commented Jun 10, 2018

ginger decided that there is no difference between functions, tests and filters. Unfortunately, there is a clash for the tests and filters lower and upper:

In jinja2:
|upper returns an uppercased string which is always truthy
is upper returns true only if the string is already uppercased

The same goes for lower respectively

This blocks #18

@wchresta wchresta changed the title Inconsistency with upper Inconsistency with lower & upper Jun 10, 2018
@tdammers
Copy link
Owner

As discussed, potential solutions:

  1. Extend the AST to track how exactly a CallE was invoked (function call, filter, test, macro invocation), and then hand that information to functions, which can then change their behavior accordingly. This would allow us to write a gfnLower that acts as an "all-lowercase" test when invoked through is, and as a "make lowercase" function otherwise. This is terrible.
  2. Abandon the "tests and filters and functions and macros are the same thing" paradigm and put them in separate namespaces. Breaks the API though, and it would be a real shame, because for use cases where 100% jinja compatibility isn't needed, this interchangeability is actually super convenient (write a macro, use it as a filter, holy cow, that's awesome!)
  3. Add some magic to is that mangles the test name to prefer some kind of specially-named builtin (ideally one that you cannot produce from within the template). E.g., foobar is lower would then parse into CallE (VarE "is-lower") [(Nothing, VarE "foobar")] (note the is- prefix on the variable name). This would work, but I'm really reluctant to add this kind of hard-to-explain magic.

So all the actual solutions kind of suck.

Additionally, I can't really think of a reasonable use case where you'd say "oh, hmm, I need to check whether everything in this string is lowercase" - the only situations I can think of are ones where you'd want to actually convert the string to lowercase anyway.

So I'm inclined to say that we should just accept this incompatibility (is lower doing something somewhat unintuitive, and deviating from jinja), document it as such, and call it a day.

@tdammers
Copy link
Owner

On further thought, I believe option 3 might not be so bad after all.

The thing is that there are more tests in Jinja2, and many of them do make sense in Ginger, like for example the is number test.

So here's how I think it should work:

  1. The is syntax construct should take care of prepending is_ to the function name, thus foo is bar should be desugared to is_bar(foo). This also implies that is syntax is going to be a lot more restrictive than filter or function call syntax: only literal function names are allowed for the filter name, so unlike filters or functions, you cannot use arbitrary expressions as tests. This isn't going to be a huge problem, because Jinja2 has the same restriction. It also means that the is construct should have very high precedence, i.e., foo + bar is even should parse as foo + (bar is even), not (foo + bar) is even.
  2. Built-in tests will be exposed to the template with the is_ prefix added. This also exposes them as plain old functions, true to the "tests and filters and functions are all the same thing" paradigm, but it avoids name clashes between things like lower and is_lower.
  3. Tests, in this approach, can also be user-defined - as long as the is_ prefix is added, it will work just fine.

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

No branches or pull requests

2 participants