Skip to content

[Process] change the syntax of portable command lines #34848

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

Merged
merged 1 commit into from
Dec 10, 2019

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekas nicolas-grekas commented Dec 6, 2019

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #34838
License MIT
Doc PR symfony/symfony-docs#12772

An alternative to #34845

Right now, portable command lines use "$FOO" for placeholders.
But because we validate that a corresponding variable exists before running the command, this fails with Command line is missing a value for key "$FOO" when FOO is not defined.

This PR proposes to use "${:FOO}" instead. The difference with the previous syntax is that this cannot collide with existing shell scripts as it is invalid for them.

When this is merged, we'll have to update https://symfony.com/blog/new-in-symfony-4-1-prepared-commands too.

@jstolp
Copy link

jstolp commented Dec 6, 2019

I think this is a clever and elegant solution 👍

nicolas-grekas added a commit that referenced this pull request Dec 10, 2019
…olas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[Process] change the syntax of portable command lines

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #34838
| License       | MIT
| Doc PR        | symfony/symfony-docs#12772

An alternative to #34845

Right now, portable command lines use `"$FOO"` for placeholders.
But because we validate that a corresponding variable exists before running the command, this fails with `Command line is missing a value for key "$FOO"` when `FOO` is not defined.

This PR proposes to use `"${:FOO}"` instead. The difference with the previous syntax is that this cannot collide with existing shell scripts as it is invalid for them.

When this is merged, we'll have to update https://symfony.com/blog/new-in-symfony-4-1-prepared-commands too.

Commits
-------

3c7b775 [Process] change the syntax of portable prepared command lines
@nicolas-grekas nicolas-grekas merged commit 3c7b775 into symfony:4.4 Dec 10, 2019
@nicolas-grekas nicolas-grekas deleted the proc-portable branch December 13, 2019 13:57
This was referenced Dec 19, 2019
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Dec 20, 2019
…rekas)

This PR was merged into the 4.4 branch.

Discussion
----------

Update paragraph about portable command lines

Follows symfony/symfony#34848

Commits
-------

35556da Update paragraph about portable command lines
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants