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

2.0.11: Macro was called without parameter condition #11745

Closed
wiesl opened this issue May 5, 2016 · 20 comments
Closed

2.0.11: Macro was called without parameter condition #11745

wiesl opened this issue May 5, 2016 · 20 comments
Assignees
Labels
bug A bug report status: medium Medium
Milestone

Comments

@wiesl
Copy link

wiesl commented May 5, 2016

After upgrading to 2.0.11 I'm getting the following error: Macro was called without parameter condition

Reverting to 2.0.10 works well.

{%- macro conditionaldate(condition, tdate, ttime, tz) %}
    {% if condition %} from <br/>{{ tdate }}, {{ ttime }} {{ tz }}{% endif %}
{%- endmacro %}

{{ conditionaldate(count_data > 0, tdate, ttime, tz) }}

Looks like it comes from the following change:

Fixed Phalcon\Mvc\View\Engine\Volt::callMacro bug. Now it's correctly calling call_user_func_array instead of call_user_func

Any ideas?
https://forum.phalconphp.com/discussion/11419/2011-macro-was-called-without-parameter-condition

Well actually it was done to fix custom user functions, filters etc beacause they weren't working(they weren't passing parameters because call_user_func was executed, they are calling callMacro too, and working right now). I don't know why it broked macros.

Im not sure what has to be done here to make both of them working. You should create an issue on github. You can copy my above explanation why this change was made.

Im not sure you can use it like this. Can you first try to set count_data > 0 to same variable and then call macro with variable ? Also it's weird that it was working on 2.0.10, it shouldn't too, beacause nothing was changed there, I only changed one function which shouldn't have any effect on this.

@Jurigag
Copy link
Contributor

Jurigag commented May 5, 2016

I think it's not a problem with call_user_func_array, beacause it's not even executed in this case. I think the condition can be just passed as count_data > 0 here. It expects variable i think, not expression.

Not sure how it works on 2.0.10 though.

@wiesl
Copy link
Author

wiesl commented May 5, 2016

{% set countdataflag = count_data > 0 %}

{{ conditionaldate(countdataflag, tdate, ttime, tz) }}

But still the same.

@Jurigag
Copy link
Contributor

Jurigag commented May 5, 2016

Well, then i don't know what's going on. Try to pass value of count_data > 0 as view variable, just to test it it's not a some problem in this case.

@wiesl
Copy link
Author

wiesl commented May 5, 2016

I guess it comes from the following change:
Fixed Phalcon\Mvc\View\Engine\Volt::callMacro bug. Now it's correctly calling call_user_func_array instead of call_user_func

@Jurigag
Copy link
Contributor

Jurigag commented May 5, 2016

Not it doesn't. callMacro is not executed here. Checked what i wrote above maybe.

@sergeyklay sergeyklay added this to the 2.0.12 milestone May 5, 2016
@Jurigag
Copy link
Contributor

Jurigag commented May 6, 2016

Did you checked maybe what will happen if you will set in php in controller count_data > 0 and pass value of it as variable to view and use this variable ? Is it still happening ?

For me it's for 99% that this change is not causing this issue. CompileMacro is executed once compiling view, CallMacro is executed well, when executing macro in compiled view(php). And this exception is thrown only in compileMacro.

@sergeyklay
Copy link
Contributor

It gets fixed if we mark that parameter as optional.
https://github.com/phalcon/cphalcon/blob/master/phalcon/mvc/view/engine/volt.zep#L288

array arguments = []

@wiesl
Copy link
Author

wiesl commented May 6, 2016

Can you push the fix?

@Jurigag
Copy link
Contributor

Jurigag commented May 6, 2016

@sergeyklay

But this exception which he has is thrown here:

https://github.com/phalcon/cphalcon/blob/master/phalcon/mvc/view/engine/volt/compiler.zep#L1911

Is it first called and then compiled then ?

@sergeyklay
Copy link
Contributor

I'll try to deal

@sergeyklay sergeyklay self-assigned this May 6, 2016
@rvanderfeer
Copy link

Not sure if this is related; I am calling a macro with 3 arguments, the first of which is an object.

I now get the error; Cannot use object of type [TheObjectType] as array when running the compiled macro where it was working fine before the update to 2.0.11.

It appears that the compiled macro expects to be called with a single array argument function(array $arg){} , where it is actually (still) called with consecutive arguments function($arg1, $arg2, ... $arg_n){}

This obviously breaks existing macros so is quite a serious bug for us.

@jellisii
Copy link

jellisii commented May 9, 2016

When can we expect this on the PPA? We just updated this morning and are having the same problem.

@sergeyklay
Copy link
Contributor

@jellisii We'll try to fix it in v2.0.12 and release this version in the coming days

@jellisii
Copy link

jellisii commented May 9, 2016

Thank you sir. I've reverted to a custom build using commit a5f615d as its source for the time being.

@Jurigag
Copy link
Contributor

Jurigag commented May 9, 2016

@rvanderfeer

Yea, it must be a bug in compiler, it must call function with arguments as array, for me from 2.0.5 i think were callMacro was introduced it broke functions, filters etc, so that's why i had to do this commit which was mention here. Well it's pretty obvious that was needed to changed, just forgot that there are macros as well and it might broke it.

andresgutierrez pushed a commit that referenced this issue May 10, 2016
* Fixed issue #11745

* Amended Volt tests

* Small cleanup
@sergeyklay
Copy link
Contributor

@wiesl Can you please check the 2.0.x branch?

@sergeyklay
Copy link
Contributor

@jellisii

@sergeyklay
Copy link
Contributor

Thanks for your report. This issue has been fixed in 2.0.x branch. We will release v2.0.12 in coming days with this bug fix.

@wiesl
Copy link
Author

wiesl commented May 18, 2016

Thnx, verified on master.

@jellisii
Copy link

Testing looks good. Thanks.

@niden niden added bug A bug report status: medium Medium and removed Bug - Medium labels Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report status: medium Medium
Projects
None yet
Development

No branches or pull requests

6 participants