Skip to content

Commit

Permalink
minor #22557 [VarDumper] Add iconv dependency (julienfalque)
Browse files Browse the repository at this point in the history
This PR was merged into the 2.8 branch.

Discussion
----------

[VarDumper] Add iconv dependency

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

Alternative to #20927.

I still think adding `@requires extension iconv` to the related tests would be nice though: on some PHP setups the iconv extension is not available by default, those tests will fail and result in red errors when running the test suite, even when working on something unrelated. The test would still run on CI so it's fine IMO.

Should I add `ext-iconv` and/or `symfony/polyfill` to `require-dev`?

/cc @nicolas-grekas

Commits
-------

13f1707 Add iconv extension to suggested dependencies
  • Loading branch information
fabpot committed Apr 29, 2017
2 parents e9e4c79 + 13f1707 commit 04e48bd
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/Symfony/Component/VarDumper/Dumper/AbstractDumper.php
Expand Up @@ -167,6 +167,9 @@ protected function echoLine($line, $depth, $indentPad)
*/
protected function utf8Encode($s)
{
if (!function_exists('iconv')) {
throw new \RuntimeException('Unable to convert a non-UTF-8 string to UTF-8: required function iconv() does not exist. You should install ext-iconv or symfony/polyfill-iconv.');
}
if (false !== $c = @iconv($this->charset, 'UTF-8', $s)) {
return $c;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Symfony/Component/VarDumper/composer.json
Expand Up @@ -20,12 +20,14 @@
"symfony/polyfill-mbstring": "~1.0"
},
"require-dev": {
"ext-iconv": "*",
"twig/twig": "~1.20|~2.0"
},
"conflict": {
"phpunit/phpunit": "<4.8.35|<5.4.3,>=5.0"
},
"suggest": {
"ext-iconv": "To convert non-UTF-8 strings to UTF-8 (or symfony/polyfill-iconv in case ext-iconv cannot be used).",
"ext-symfony_debug": ""
},
"autoload": {
Expand Down

0 comments on commit 04e48bd

Please sign in to comment.