cleaned up escaping in ProcessBuilder

Author: kriswallsmith

src/Assetic/Filter/CompassFilter.php

@@ -241,7 +241,7 @@ public function filterLoad(AssetInterface $asset)
             $pb->add('--config')->add($this->config);
         }
 
-        $pb->add('--sass-dir')->add('""')->add('--css-dir')->add('""');
+        $pb->add('--sass-dir')->add('')->add('--css-dir')->add('');
 
         // compass choose the type (sass or scss from the filename)
         if (null !== $this->scss) {

src/Assetic/Util/ProcessBuilder.php

@@ -25,7 +25,6 @@ class ProcessBuilder
     private $timeout = 60;
     private $options = array();
     private $inheritEnv = false;
-    private $escaper;
 
     public function add($part)
     {
@@ -80,22 +79,18 @@ public function setOption($name, $value)
         return $this;
     }
 
-    public function setEscaper(\Closure $escaper)
-    {
-        $this->escaper = $escaper;
-
-        return $this;
-    }
-
     public function getProcess()
     {
-        $escaper = $this->escaper ?: function($value)
-        {
-            return false === strpos($value, ' ') ? $value : escapeshellarg($value);
-        };
+        if (!count($this->parts)) {
+            throw new \LogicException('You must add() command parts before calling getProcess().');
+        }
+
+        $parts = $this->parts;
+        $cmd = array_shift($parts);
+        $script = escapeshellcmd($cmd).' '.implode(' ', array_map('escapeshellarg', $parts));
 
         $env = $this->inheritEnv ? ($this->env ?: array()) + $_ENV : $this->env;
 
-        return new Process(implode(' ', array_map($escaper, $this->parts)), $this->cwd, $env, $this->stdin, $this->timeout, $this->options);
+        return new Process($script, $this->cwd, $env, $this->stdin, $this->timeout, $this->options);
     }
 }

tests/Assetic/Test/Util/ProcessBuilderTest.php

@@ -24,7 +24,7 @@ public function shouldInheritEnvironmentVars()
         $_ENV = $expected = array('foo' => 'bar');
 
         $pb = new ProcessBuilder();
-        $pb->inheritEnvironmentVariables();
+        $pb->add('foo')->inheritEnvironmentVariables();
         $proc = $pb->getProcess();
 
         $this->assertEquals($expected, $proc->getEnv(), '->inheritEnvironmentVariables() copies $_ENV');
@@ -42,8 +42,11 @@ public function shouldNotReplaceExplicitlySetVars()
         $expected = array('foo' => 'baz');
 
         $pb = new ProcessBuilder();
-        $pb->setEnv('foo', 'baz');
-        $pb->inheritEnvironmentVariables();
+        $pb
+            ->setEnv('foo', 'baz')
+            ->inheritEnvironmentVariables()
+            ->add('foo')
+        ;
         $proc = $pb->getProcess();
 
         $this->assertEquals($expected, $proc->getEnv(), '->inheritEnvironmentVariables() copies $_ENV');