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

Change syntax for env vars #219

Merged

Conversation

oanhnn
Copy link
Contributor

@oanhnn oanhnn commented Mar 23, 2015

In this Pull request, I will implement feature of issue #211
Change syntax for env vars to {{ ... }} and support allow whitespaces ( {{deploy_path}} and {{ deploy_path }} return same value).

Sorry by create other pull request.
This pull request is rebase of PR #218

@antonmedv
Copy link
Member

Ok :)

@antonmedv antonmedv merged commit c673726 into deployphp:master May 4, 2015
@oanhnn
Copy link
Contributor Author

oanhnn commented May 6, 2015

Dear @Elfet ,
My pull request #219 has a problem.
With old parser (pattern /\{(.+?)\}/) detected all cases:

{abcxyz}
{abc-xyz}
{abc_xyz}
{abc.xyz}
{abc@zyx}

but new parser (pattern /\{\{\s*(\w+)\s*\}\}/) only detected cases:

{{abcxyz}}
{{abc_xyz}}

I can fix this, but i want specific requirements for env var.
I think we should only allow:

{{abcxyz}}
{{abc_xyz}}
{{abc.xyz}}

@gordalina
Copy link
Contributor

I vote for same pattern as normal variables (implemented on #219).
Having a dot . on a variable could be used in the future to be a property of an array.

@oanhnn
Copy link
Contributor Author

oanhnn commented May 6, 2015

@gordalina, Is your attention?

env('abc', array('xyz' => 1));

echo env()->parse('{{abc.xyz}}');

output:

1

@gordalina
Copy link
Contributor

I didn't know about parse.
Still, I keep my opinion, and would suggest making parse() private and env() would call parse() here

This way, properties are part of the DSL and the user doesn't need to know wether it is necessary to parse them or not.

What are your thoughts @oanhnn @Elfet?

@oanhnn
Copy link
Contributor Author

oanhnn commented May 6, 2015

I think:

  1. Method parse will be private. It is only call when using run().
  2. When call env($abc, $xyz) or env()->set($abc, $xyz), variable $abc will check and only allow a-z, A-Z, 0-9 and underscore character ( _ )
  3. When call env($abc) or env()->get($abc), variable $abc will check and only allow a-z, A-Z, 0-9, underscore character ( _ ) and dot character ( . ) . If having a dot on variable $abc could be used in the future to be a property of an array.

@gordalina
Copy link
Contributor

@oanhnn exactly!

@oanhnn oanhnn deleted the features/change-syntax-for-env-vars branch May 13, 2015 09:22
@oanhnn oanhnn restored the features/change-syntax-for-env-vars branch May 13, 2015 09:47
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

Successfully merging this pull request may close these issues.

3 participants