Skip to content
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

Add generic pipe filters #60

Merged
merged 5 commits into from
Jun 3, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions src/Filter/PipeInputFilter.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<?php
/**
* MiniAsset
* Copyright (c) Mark Story (http://mark-story.com)
*
* Licensed under The MIT License
* For full copyright and license information, please see the LICENSE.txt
* Redistributions of files must retain the above copyright notice.
*
* @copyright Copyright (c) Mark Story (http://mark-story.com)
* @since 0.0.1
* @license http://www.opensource.org/licenses/mit-license.php MIT License
*/
namespace MiniAsset\Filter;

use MiniAsset\Filter\AssetFilter;
use MiniAsset\Filter\CssDependencyTrait;

/**
* Pre-processing filter that adds support for command pipes
*/
class PipeInputFilter extends AssetFilter
{
use CssDependencyTrait {
getDependencies as getCssDependencies;
}

protected $_settings = array(
'ext' => '.css',
'command' => '/bin/cat',
'dependencies' => 'none', // none, css, other
'optional_dependency_prefix' => false,
'path' => '/bin',
);

/**
* It will use prefixed files if they exist.
*
* @var string
*/
protected $optionalDependencyPrefix = null;

public function getDependencies(\MiniAsset\AssetTarget $file)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this class be added to the use list? That would be more consistent with the rest of this project.

{
if ($this->_settings['dependencies'] !== 'none' && $this->_settings['dependencies'] !== 'css') {
return false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should return [] here to be consistent with other implementations.

}

if ($this->_settings['dependencies'] === 'css') {
$this->optionalDependencyPrefix = $this->_settings['optional_dependency_prefix'];

return $this->getCssDependencies($file);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you know that the file being processed is CSS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the dependencies setting is true the CSS-like dependencies are used. I was previously using a text setting in case there were more dependency types added later but I've decided to simplify. It should be set to false when there's no CSS-like imports.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't a safer default be false then? In the docblock for the settings/filter the usage could be explained.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't a safer default be false then? In the docblock for the settings/filter the usage could be explained.

I think I did it because the default extension is .css. Maybe I should change that too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@markstory, please, review the settings changes in the new commit.

}

return [];
}

/**
* Runs command against any files that match the configured extension.
*
* @param string $filename The name of the input file.
* @param string $input The content of the file.
* @return string
*/
public function input($filename, $input)
{
if (substr($filename, strlen($this->_settings['ext']) * -1) !== $this->_settings['ext']) {
return $input;
}
$filename = preg_replace('/ /', '\\ ', $filename);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use escapeshellarg() instead.

$bin = $this->_settings['command'] . ' ' . $filename;
$return = $this->_runCmd($bin, '', array('PATH' => $this->_settings['path']));

return $return;
}
}
47 changes: 47 additions & 0 deletions src/Filter/PipeOutputFilter.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php
/**
* MiniAsset
* Copyright (c) Mark Story (http://mark-story.com)
*
* Licensed under The MIT License
* For full copyright and license information, please see the LICENSE.txt
* Redistributions of files must retain the above copyright notice.
*
* @copyright Copyright (c) Mark Story (http://mark-story.com)
* @since 0.0.1
* @license http://www.opensource.org/licenses/mit-license.php MIT License
*/
namespace MiniAsset\Filter;

use MiniAsset\Filter\AssetFilter;

/**
* Output pipe processor
*/
class PipeOutputFilter extends AssetFilter
{

/**
* Settings for PipeOutputFilter
*
* - `command` Command to execute
*/
protected $_settings = array(
'command' => '/bin/cat',
'path' => '/bin',
);

/**
* Run command against the output and compress it.
*
* @param string $filename Name of the file being generated.
* @param string $input The raw contents for $filename.
* @return string Processed contents.
*/
public function output($filename, $input)
{
$cmd = $this->_settings['command'];

return $this->_runCmd($cmd, $input, array('PATH' => $this->_settings['path']));
}
}
6 changes: 5 additions & 1 deletion src/Output/FreshTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,11 @@ public function isFresh(AssetTarget $target)

$filters = $this->filterRegistry->collection($target);
foreach ($filters->filters() as $filter) {
foreach ($filter->getDependencies($target) as $child) {
$dependencies = $filter->getDependencies($target);
if ($dependencies === false) {
return false;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like conflating concerns. Overloading the dependency detection to hand mtime overrides feels wrong. Could we add a new filter hook that allows filters to overrule the mtime value and only implement in the pipe filters.

foreach ($dependencies as $child) {
$time = $child->modifiedTime();
if ($time >= $buildTime) {
return false;
Expand Down
112 changes: 112 additions & 0 deletions tests/TestCase/Filter/PipeInputFilterTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
<?php
/**
* MiniAsset
* Copyright (c) Mark Story (http://mark-story.com)
*
* Licensed under The MIT License
* For full copyright and license information, please see the LICENSE.txt
* Redistributions of files must retain the above copyright notice.
*
* @copyright Copyright (c) Mark Story (http://mark-story.com)
* @since 0.0.1
* @license http://www.opensource.org/licenses/mit-license.php MIT License
*/
namespace MiniAsset\Test\TestCase\Filter;

use MiniAsset\AssetTarget;
use MiniAsset\File\Local;
use MiniAsset\Filter\PipeInputFilter;
use PHPUnit\Framework\TestCase;

class PipeInputFilterTest extends TestCase
{

public function setUp(): void
{
parent::setUp();
$this->_cssDir = APP . 'css' . DS;
$this->filter = new PipeInputFilter();
$this->filter->settings([
'paths' => [$this->_cssDir],
'ext' => '.scss',
]);
}

public function testParsing()
{
$this->filter->settings(array('command' => '/bin/cat'));

$content = file_get_contents($this->_cssDir . 'test.scss');
$result = $this->filter->input($this->_cssDir . 'test.scss', $content);
$expected = file_get_contents($this->_cssDir . 'test.scss');
$this->assertEquals($expected, $result);
}

public function testGetDependenciesNone()
{
$this->filter->settings(array(
'dependencies' => 'none',
'optional_dependency_prefix' => false,
));

$files = [
new Local($this->_cssDir . 'test.scss')
];
$target = new AssetTarget('test.css', $files);
$result = $this->filter->getDependencies($target);

$this->assertEmpty($result);
}

public function testGetDependencies()
{
$this->filter->settings(array(
'dependencies' => 'css',
'optional_dependency_prefix' => '_',
));

$files = [
new Local($this->_cssDir . 'test.scss')
];
$target = new AssetTarget('test.css', $files);
$result = $this->filter->getDependencies($target);

$this->assertCount(3, $result);
$this->assertEquals('colors.scss', $result[0]->name());
$this->assertEquals('_utilities.scss', $result[1]->name());
$this->assertEquals('_reset.scss', $result[2]->name());
}

public function testGetDependenciesAlwaysRun()
{
$this->filter->settings(array(
'dependencies' => 'other',
'optional_dependency_prefix' => false,
));

$files = [
new Local($this->_cssDir . 'test.scss')
];
$target = new AssetTarget('test.css', $files);
$result = $this->filter->getDependencies($target);

$this->assertFalse($result);
}

public function testGetDependenciesMissingDependency()
{
$this->filter->settings(array(
'dependencies' => 'css',
'optional_dependency_prefix' => '_',
));

$files = [
new Local($this->_cssDir . 'broken.scss')
];
$target = new AssetTarget('test.css', $files);
$result = $this->filter->getDependencies($target);

$this->assertCount(1, $result);
$this->assertEquals('colors.scss', $result[0]->name());
}
}
36 changes: 36 additions & 0 deletions tests/TestCase/Filter/PipeOutputFilterTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php
/**
* MiniAsset
* Copyright (c) Mark Story (http://mark-story.com)
*
* Licensed under The MIT License
* For full copyright and license information, please see the LICENSE.txt
* Redistributions of files must retain the above copyright notice.
*
* @copyright Copyright (c) Mark Story (http://mark-story.com)
* @since 0.0.1
* @license http://www.opensource.org/licenses/mit-license.php MIT License
*/
namespace MiniAsset\Test\TestCase\Filter;

use MiniAsset\Filter\PipeOutputFilter;
use PHPUnit\Framework\TestCase;

class PipeOutputFilterTest extends TestCase
{

public function setUp(): void
{
parent::setUp();
$this->_cssDir = APP . 'css' . DS;
$this->filter = new PipeOutputFilter();
}

public function testOutput()
{
$content = file_get_contents($this->_cssDir . 'unminified.css');
$result = $this->filter->output($this->_cssDir . 'unminified.css', $content);
$expected = file_get_contents($this->_cssDir . 'unminified.css');
$this->assertEquals($expected, $result);
}
}