Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

Subexpressions don't work with context resolving #80

Merged
merged 1 commit into from
Aug 29, 2014

Conversation

JustBlackBird
Copy link
Contributor

In the current implementation subexpressions do not work with context at all.

I've created several tests to illustrate the problem.

@fzerorubigd
Copy link
Contributor

Its not like that, I think.
For example in {{concat a (concat c b)}} with array('a' => 'A', 'b' => 'B', 'c' => 'C') context :

result of concat c b is CB , so the final concat is concat a CB and CB is not in context and so the result is null (for CB), the real result we get is only 'c' and its correct.

I change your tests, If you think this is wrong, please explain it with more example.

Thank you!

@fzerorubigd fzerorubigd merged commit 0432d99 into XaminProject:master Aug 29, 2014
fzerorubigd added a commit that referenced this pull request Aug 29, 2014
@JustBlackBird
Copy link
Contributor Author

@fzerorubigd you are wrong with your changes. In all my cases concat helper returns strings, not variables names. And that is the main problem -- there is no way to tell subexpressions that you return just strings.

Here is the example:

{{concat (concat a "\"\'[]") b}}

"\"\'[]" is a correct string, but not a correct variable name. If I try to render the template above I get an InvalidArgumentException with "variable name is invalid" message.

Moreover, according to your changes concat helper tries to get variable from the context and if it is not found it uses just it's name. It is the same as render {{var}} to "var" string if the var variable is not found in the context.

@JustBlackBird JustBlackBird deleted the subexpr_context_test branch August 29, 2014 08:52
@fzerorubigd
Copy link
Contributor

We need a better argument parser and a simple way to detect variables from the "string" inside helpers.
In current API, the helper must detect this, but the better way is to pass an Arguments class to helpers (with ability to transform into string for preventing BC break) And this need much more work.

@everplays
Copy link
Contributor

Before going for parsing arguments and stuff like that, I think we can use
a warpper class like safe string to make difference between variable names
and static strings.
On Aug 29, 2014 1:26 PM, "Fzerorubigd" [email protected] wrote:

We need a better argument parser and a simple way to detect variables from
the "string" inside helpers.
In current API, the helper must detect this, but the better way is to pass
an Arguments class to helpers (with ability to transform into string for
preventing BC break) And this need much more work.


Reply to this email directly or view it on GitHub
#80 (comment)
.

@JustBlackBird
Copy link
Contributor Author

@fzerorubigd, I agree. I think the current problem is tightly related with #74. It seems that all arguments stuff must be done in Handlebars core, not in a helper.

As for subexpressions, they are useless without proper work with context resolving.

@JustBlackBird
Copy link
Contributor Author

@everplays, we cannot return string wrapper from a helper. Here any wrapper will be converted to string, so the parent helper will know nothing about such wrappers.

@everplays
Copy link
Contributor

That needs to be fixed as well. However, Regardless of #74, we need a way
to know what the result of a subexpression would mean. Subexpression might
return the text "a". When it is referring to the variable a and when it
means the static string a? We can fix this (which fixes this issue as well)
by having a wrapper for static string.
On Aug 29, 2014 1:38 PM, "Dmitriy S. Simushev" [email protected]
wrote:

@everplays https://github.com/everplays, we cannot return string
wrapper from a helper. Here
https://github.com/XaminProject/handlebars.php/blob/master/src/Handlebars/Template.php#L359
any wrapper will be converted to string, so the parent helper will know
nothing about such wrappers.


Reply to this email directly or view it on GitHub
#80 (comment)
.

@fzerorubigd
Copy link
Contributor

I don't know exactly, How handlebars.js handle this? for example in concat a b how we can detect if this is plain a or variable name a? or simple we could use concat "a" "b" and concat a b the first for string and other for variable?

@JustBlackBird
Copy link
Contributor Author

Here is Handlebars.js specs for subexpressions: https://github.com/wycats/handlebars.js/blob/master/spec/subexpressions.js

As I can see arguments in subexpressions are the same as in normal helpers:

  • "smth" and 'smth' are strings;
  • all other things are variables.

@everplays
Copy link
Contributor

https://github.com/wycats/handlebars.js/blob/f4b8c5260c203d6ab7631fa394696f5c7a442586/spec/subexpressions.js#L125 is what we need which, from my understanding, is what I have described.

@fzerorubigd, @JustBlackBird What do you think?

@JustBlackBird
Copy link
Contributor Author

@everplays, as I can see helpers always return strings, not variables names.

@fzerorubigd I found one more problem with tests. Here the test helper does not resolve variable name, but it should to! The correct helper's body is:

return $context->get($arg) . 'Test.';

As the result the tests are false positive, because they describe incorrect behavior.

@everplays, @fzerorubigd May be I should create a separate issue for discussion?

@fzerorubigd
Copy link
Contributor

@JustBlackBird yes. Please create a seperate issue for that one too.
Thank you!

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

Successfully merging this pull request may close these issues.

3 participants