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

Supported multiple arguments with nested calls #701

Closed
theofidry opened this issue Mar 18, 2017 · 4 comments
Closed

Supported multiple arguments with nested calls #701

theofidry opened this issue Mar 18, 2017 · 4 comments
Milestone

Comments

@theofidry
Copy link
Member

What's supported:

  • <f(<g()>)>
  • <f(<g()>, 'a')>
  • <f(<g('a')>)>

What's not:

  • <f(<g('b', 'e')>, 'd')>

/cc @dantleech

@theofidry theofidry added the Bug label Mar 18, 2017
@theofidry theofidry added this to the 3.x milestone Mar 18, 2017
@theofidry
Copy link
Member Author

Note that I don't think this should be a blocker for the 3.0.0 release. But I think it's easier to solve the problem by tackling #601.

@marinoska
Copy link
Contributor

Hi @theofidry I see it comes from the same code as #775 645 as parsing arguments relies on this regexp:
preg_match_all('/[[^[]+]|[^,\s]+/', $argumentsString, $argumentsList);
And I have no idea how to extract arguments correctly from such pattern as
<f(<g(<b()>, e)>, d, <c()>)>
using only regexps.
I can fix it looping over the symbols in the argument string but you might have already had such a method implemented in the alice code, don't you?

@theofidry
Copy link
Member Author

I partially tackled that one by tokenizing functions (that's what the FUNCTION_START__function__ was about: https://github.com/nelmio/alice/blob/master/tests/FixtureBuilder/ExpressionLanguage/Lexer/LexerIntegrationTest.php#L456).

That's clearly not enough and your fix may have helped. But as you said it's a problem were regexes are bad at. A cleaner solution would be #601 and #712. I believe @Hywan started some bits, but that's a significant piece of work for sure.

@theofidry
Copy link
Member Author

I'll be closing this as IMO the real fix is #601.

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