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

Pipelining inside expressions #84

Open
mvrhov opened this issue May 10, 2018 · 6 comments
Open

Pipelining inside expressions #84

mvrhov opened this issue May 10, 2018 · 6 comments

Comments

@mvrhov
Copy link

mvrhov commented May 10, 2018

i'd like to use pipelinig inside a yield expression to pass a translation to a block

{{yield inputField(id="email", name="email", type="email", label="login.form.label.email"|T:"Email",map(),"account", icon="fa-envelope", required=true)}}
the parser seems to die inside "label"

I have also tried to use alternate syntax:
{{ label := "login.form.label.email"|T:"Email",map(),"account"}}
{{yield inputField(id="email", name="email", type="email", label=label, icon="fa-envelope", required=true)}}
which also seems to not work. (it reports the same error)

@annismckenzie
Copy link
Member

annismckenzie commented May 12, 2018

The things we come up with as humans writing template files. 😆

So yeah, the parser is probably failing on the first version. The second one, though, looks correct (I use blocks and yields a lot in our projects). What's the error? Does it fail on the first line or the second? Does the label variable contain the translated content? Also, you may want to try the plain old function invocation syntax which should work inside the yield as T("Email", etc.).

As for the first version and making it work: I don't think I want to (sorry about that). 🙈 As a member on a team having to grasp that statement… it doesn't look very readable. Add to that, that as you've written it the parser couldn't easily distinguish between when the pipeline ends and the next parameter begins (yes, the next parameter is a parameter= token but still).

Just to be clear on who I am: I try to help people out with syntax and questions but haven't actually done any work on the engine (parser, lexer etc.) and I always defer to José. Sorry about that.

@mvrhov
Copy link
Author

mvrhov commented May 13, 2018

edit:
The error is unexpected "|" in command.

Also, you may want to try the plain old function invocation syntax which should work inside the yield as T("Email", etc.)

Even if this works this once again brings the inconsistency in template writing.

@annismckenzie
Copy link
Member

I stand by my comment. Having written a lot of templates I really really don't want to make the first version work. There's just too many things going on. With #89 you will also be able to break up the yield into multiple lines and put T("login.form.label.email", "Email", …) as the value of the label parameter, making it readable and explicit. Does that sound fair? Please close the issue in that case.

@mvrhov
Copy link
Author

mvrhov commented Jul 3, 2018

Even though that you don't like the translate version example... there is the same issue if one want's to use "default" in pipelinig fashion.

e.g {{yield inputField(id="email", name="email", type="email", label=label, icon="fa-envelope", required=true error=formErrosMap["email"]|default("") )}}

I'll say it here again like in another issue. This is the inconsistency with pipelinig that should be fixed.

@annismckenzie
Copy link
Member

I see your point. I'll put it on the list. The problem that I see there is that it'll probably blow up even if I make the command understand pipelining.

Just out of curiosity, is this just the example or wouldn't you be able to use Go's default return value of an empty string when accessing non-existant map keys? 🤔

I'll start with a failing test case and then we can iterate on that. Maybe it's worth it to drop the prefix version of pipelining (something|FunctionName:"param1", "param2") in favor of the consistent something|FunctionName("param1", "param2") in order to reduce the complexity of the parser and so that the inherent ambiguity of those statements in compound expressions, block parameters and yield statements goes away. That would bump the major to v3 which I'd be fine with.

@mvrhov
Copy link
Author

mvrhov commented Jul 4, 2018

Below are 4 examples and before reading an answer try to figure out which ones should work.

var data jet.VarMap
data.Set("errs", make(map[string]string))


{{block test1(error="")}}
  {{if isset(error) && error}}
  {{error}}
  {{end}}
{{end}}

{{block test2(error="")}}
 {{if error}}
  {{error}}
 {{end}}
{{end}}

{{block test3(error)}}
 {{if isset(error) && error}}
  {{error}}
 {{end}}
{{end}}

{{block test4(error)}}
 {{if error}}
  {{error}}
 {{end}}
{{end}}

{{ yield test1(error=errs["NonExisting"]) }}
{{ yield test2(error=errs["NonExisting"]) }}
{{ yield test3(error=errs["NonExisting"]) }}
{{ yield test4(error=errs["NonExisting"]) }}

Answering the 2nd question
IMO it's totally worth dropping the prefix version. Because this would get rid of the "inconsistency" I was talking about and it would also simplify "cognitive" load to developer when reading/writing templates as it would be one syntax anomaly less.

Now for the trick question:
Of examples above only test1 & test3 work.
Other die with level=error msg="Jet Runtime Error(\"included/blocks.html.jet\":8): identifier \"error\" is not available in the current scope map[error:<invalid Value>]"

I'd say that also test2 should work as it has a default value (e.g there should be no need for an isset inside a block, and or {{ yield test1(error=errs["NonExisting"]|default("")) }} a pipe inside a yield).
However simplifying the parser and template syntax with pipelining and allowing it inside a yield would be a big 👍

@sauerbraten sauerbraten changed the title Pipelining and yielding Pipelining inside expressions Nov 9, 2020
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

No branches or pull requests

2 participants