merged branch sun/yaml-indent (PR #5157)

Commits
-------

3e1a1ab Force the value of Dumper::setIndentation($num) to be of type integer.
5be7237 Added Yaml\Dumper::setIndentation() method to allow a custom indentation level of nested nodes.

Discussion
----------

[Yaml] Allow custom indentation level for nested nodes

YAML does not specify an absolute indentation level, but a consistent indentation of nested nodes only: http://yaml.org/spec/current.html#indentation%20space/

Projects that are generally using 2 spaces for indentation should be able to retain consistency with their coding standards by supplying a custom value for the new $indent parameter for Yaml::dump() and Dumper::dump().

The new parameter is a backwards-compatible API addition and defaults to the previous default of 4 (which was changed from 2 via PR #2242 only recently).

The old $indent parameter is renamed to $level, and remains to be used internally only.

---------------------------------------------------------------------------

by travisbot at 2012-08-03T00:24:22Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/2024289) (merged 56331202 into b1618d21).

---------------------------------------------------------------------------

by travisbot at 2012-08-03T00:29:02Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/2024315) (merged eeae28a4 into b1618d21).

---------------------------------------------------------------------------

by travisbot at 2012-08-03T02:41:42Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/2024905) (merged 9a82c438 into b1618d21).

---------------------------------------------------------------------------

by fabpot at 2012-08-03T07:53:50Z

This is indeed a BC break as the method signature changed in a non-BC way.

---------------------------------------------------------------------------

by fabpot at 2012-08-03T08:35:51Z

I think I would prefer to have a static method to be able to change the number of spaces to use on a global basis. It makes more sense and would prevent the BC break. What do you think?

---------------------------------------------------------------------------

by sun at 2012-08-03T13:23:23Z

Thanks for your feedback! — Took some time to think through the static proposal.  Here's what I think:

1. The call from DependencyInjection\Dumper\YamlDumper truly was a unexpected/nasty surprise for me.  That is, because it passes a parameter to Yaml\Dumper that is documented for internal use only, and the surrounding YamlDumper code generally hard-codes lots of assumptions of where exactly the nodes/collections will be output in the dumped YAML structure.  (I think that code can be simplified, cleaned up, and made faster at the same time (hence the todo), but indeed, that probably belongs into a separate issue.)
1. The essential problem with a static is closely bound to that though; a static property value for the indentation would "stick" and thus hi-jack the DependencyInjection\Dumper\YamlDumper, as it hard-codes and thus expects 4 spaces (contrary to any custom indentation).
1. Most code uses Yaml\Yaml as utility/helper service directly, so there'd be no clean way to prime the static with a custom value, aside from subclassing the entire thing - in which case this entire issue would sorta become moot, because at the point you're subclassing, you can as well go the extra mile and replace the entire Yaml\Dumper::dump() to implant the custom indentation level...
1. Another option would be to use a non-static Yaml\Dumper::$indent property, supplied through the constructor; i.e.:

        public function __construct($indent = 4)

  ...or alternatively, ::setIndentation(). Essentially requiring people to use and instantiate Yaml\Dumper directly, if they want to use a custom indentation.
1. Though in the end, I don't want to sound pedantic, but I *do* wonder a bit about the exact extent of the `@api` tags, as well as `@param`s that are explicitly documented as "internal use only"... :)  That is, because only Yaml\Yaml is tagged with `@api`, but nothing in Yaml\Dumper.  The same sorta applies to DependencyInjection\Dumper\YamlDumper::dump(), which is tagged with `@api`, but the :.addService() method being adjusted accordingly here is not.  So essentially, when taking those tags (plus the param's description) seriously and in a nitpicky way, then there is no BC break, since no one should rely on their exact implementation... ;)  (I perfectly realize that this is a long shot :))

So... depending on the final stance on the last point, I'd either move forward with the current proposal in this PR.  Otherwise, I'd suggest the non-static property on Yaml\Dumper - in which case we'd likely try to swap out the static Symfony\Component\Yaml\Yaml helper with a Drupal\Component\Yaml\Yaml in order to always instantiate the dumper with the custom indentation.  What do you think?

---------------------------------------------------------------------------

by sun at 2012-08-04T22:57:21Z

Alright.  While I believe I made some good points in my last comment, I've taken the fully backwards-compatible path:

- added the new $indent parameter only to Yaml::dump()
- added a new Dumper::$intendation property and Dumper::setIndentation() method, to control the indentation level within the scope of a single Dumper instance only.

Do you think this is acceptible? :)

---------------------------------------------------------------------------

by travisbot at 2012-08-05T06:16:22Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/2039120) (merged 5be7237b into c99f9d29).

---------------------------------------------------------------------------

by travisbot at 2012-08-07T07:51:04Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/2055164) (merged 3e1a1abd into c99f9d29).

---------------------------------------------------------------------------

by sun at 2012-08-07T07:53:18Z

Only one environment failed, and the [test failure](http://travis-ci.org/#!/symfony/symfony/jobs/2055165/L203) seems unrelated to this PR.

src/Symfony/Component/Yaml/Dumper.php

@@ -18,6 +18,23 @@ namespace Symfony\Component\Yaml;
  */
 class Dumper
 {
+    /**
+     * The amount of spaces to use for indentation of nested nodes.
+     *
+     * @var integer
+     */
+    protected $indentation = 4;
+
+    /**
+     * Sets the indentation.
+     *
+     * @param integer $num The amount of spaces to use for intendation of nested nodes.
+     */
+    public function setIndentation($num)
+    {
+        $this->indentation = (int) $num;
+    }
+
     /**
      * Dumps a PHP value to YAML.
      *
@@ -44,7 +61,7 @@ class Dumper
                     $prefix,
                     $isAHash ? Inline::dump($key).':' : '-',
                     $willBeInlined ? ' ' : "\n",
-                    $this->dump($value, $inline - 1, $willBeInlined ? 0 : $indent + 4)
+                    $this->dump($value, $inline - 1, $willBeInlined ? 0 : $indent + $this->indentation)
                 ).($willBeInlined ? "\n" : '');
             }
         }

src/Symfony/Component/Yaml/Yaml.php

@@ -97,14 +97,16 @@ class Yaml
      *
      * @param array   $array  PHP array
      * @param integer $inline The level where you switch to inline YAML
+     * @param integer $indent The amount of spaces to use for indentation of nested nodes.
      *
      * @return string A YAML string representing the original PHP array
      *
      * @api
      */
-    public static function dump($array, $inline = 2)
+    public static function dump($array, $inline = 2, $indent = 4)
     {
         $yaml = new Dumper();
+        $yaml->setIndentation($indent);
 
         return $yaml->dump($array, $inline);
     }