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

Deprecate parser context for v3 #342

Closed
theofidry opened this issue May 7, 2016 · 1 comment
Closed

Deprecate parser context for v3 #342

theofidry opened this issue May 7, 2016 · 1 comment
Milestone

Comments

@theofidry
Copy link
Member

After working on the parsers (#340), I noticed that a context, accessible in the parsed files, can be set at the parsers level.

For example:

use Nelmio\Alice\Fixtures\Parser\Methods\Php as PhpParser;

$context = ['foo' => 'bar'];
$parser = new PhpParser($context);
// parsed_file.php
// has access to $context
return [
  'Dummy' => [
    'dummy0' => [
      'id' => 0,
      'name' => $context['foo'],
    ]
  ]
]

In YAML it is a bit more tricky as you cannot use raw PHP. Instead you have to convert your yaml file into a PHP one first:

use Nelmio\Alice\Fixtures\Parser\Methods\Yaml as YamlParser;

$context = ['foo' => 'bar'];
$parser = new YamlParser($context);
# parsed_file.yml.php
# has access to $context
Dummy:
    dummy0:
      id: 0
      name: <?php echo $context['foo']; ?> # has to be followed by a blank line... ugly workaround

# ...

I find this feature questionable and it requires some workaround to make it possible. Removing it would be much simpler. The only useful usage I found was in the code base to inject the loader itself to inject the parameters. Taking a look at this diagram, this could be handled in the Builder instead.

@tshelburne WDYT?

@theofidry theofidry added this to the 2.x milestone May 7, 2016
@tshelburne
Copy link
Collaborator

@theofidry Agreed - when I was reworking the library for v2, I found the loader injection to be quite confusing for a while. I think moving the step somehow to the Builder would be a great call.

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