Skip to content

[Yaml][TwigBridge] Use JSON_UNESCAPED_SLASHES for lint commands output #19922

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
Sep 13, 2016

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Sep 12, 2016

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Slashes are escaped when sing the lint:twig and lint:yaml commands with the format option set to json, giving such results:

[
    {
        "file": "yaml\/wrong\/1.yml",
        "valid": false,
        "message": "Unable to parse at line 1 (near \";:cc`\")."
    }
]

That's not convenient as file paths may be reused (e.g. copy-pasted).
Results stay fine as error messages are already escaped:

[
    {
        "file": "yaml/wrong/1.yml",
        "valid": false,
        "message": "Unable to parse at line 1 (near \";:cc`\")."
    }
]

@chalasr
Copy link
Member Author

chalasr commented Sep 12, 2016

@fabpot fabbot seems to have failed. He wants me to add two spaces after the | between json constants.

@fabpot
Copy link
Member

fabpot commented Sep 12, 2016

hmmm, I think fabbot drinks too much French wine :) Can you report the issue on FriendsOfPhp/php-cs-fixer?

@fabpot
Copy link
Member

fabpot commented Sep 12, 2016

JSON_UNESCAPED_SLASHES has been introduced in PHP 5.4, but 2.7 also supports PHP 5.3, so you need to take care of when the constant is not defined.

@chalasr
Copy link
Member Author

chalasr commented Sep 13, 2016

@fabpot There is already a check for JSON_PRETTY_PRINT which has been introduced in the same time, I guess it's fine to add the two if one of them is defined, isn't it? :)

Can you report the issue on FriendsOfPhp/php-cs-fixer?

Sure! See PHP-CS-Fixer/PHP-CS-Fixer#2177

@fabpot
Copy link
Member

fabpot commented Sep 13, 2016

@chalasr indeed, looks good to me.

@fabpot
Copy link
Member

fabpot commented Sep 13, 2016

Thank you @chalasr.

@fabpot fabpot merged commit 0427594 into symfony:2.7 Sep 13, 2016
fabpot added a commit that referenced this pull request Sep 13, 2016
…mands output (chalasr)

This PR was merged into the 2.7 branch.

Discussion
----------

[Yaml][TwigBridge] Use JSON_UNESCAPED_SLASHES for lint commands output

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Slashes are escaped when sing the `lint:twig` and `lint:yaml` commands with the `format` option set to `json`, giving such results:

```json
[
    {
        "file": "yaml\/wrong\/1.yml",
        "valid": false,
        "message": "Unable to parse at line 1 (near \";:cc`\")."
    }
]
```

That's not convenient as file paths may be reused (e.g. copy-pasted).
Results stay fine as error messages are already escaped:

```json
[
    {
        "file": "yaml/wrong/1.yml",
        "valid": false,
        "message": "Unable to parse at line 1 (near \";:cc`\")."
    }
]
```

Commits
-------

0427594 Use JSON_UNESCAPED_SLASHES for lint commands output
@chalasr chalasr deleted the lint_json_unescaped_slashes branch September 14, 2016 00:06
@chalasr chalasr restored the lint_json_unescaped_slashes branch September 14, 2016 23:10
chalasr added a commit to chalasr/symfony that referenced this pull request Sep 14, 2016
@chalasr chalasr deleted the lint_json_unescaped_slashes branch September 14, 2016 23:10
chalasr added a commit to chalasr/symfony that referenced this pull request Sep 14, 2016
fabpot added a commit that referenced this pull request Sep 15, 2016
This PR was merged into the 3.2-dev branch.

Discussion
----------

[Yaml] Port of #19922 for 3.2

| Q             | A
| ------------- | ---
| Branch?       | master
| Tests pass?   | yes
| License       | MIT

The command has been moved in 3.2 so the new path is unmerged.

Commits
-------

017e88b [Yaml] Port of #19922 for 3.2
KorsaR-ZN pushed a commit to KorsaR-ZN/symfony that referenced this pull request Sep 19, 2016
This was referenced Oct 3, 2016
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