Add option to dump only leafs in --watch #175

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants

While working with many files that are combined under single asset I noticed how slow it is when script has to dump 10+ leafs when only one changes. The new option allows to dump only leafs. And, since I'm using assetic in debug mode without controllers, that's exactly what I need. I think this should benefit everyone who's developing javascript heavy applications with more than 5 files combined. Especially when using compilers (coffeescript, less etc).

@stof stof commented on an outdated diff Mar 12, 2013

Command/DumpCommand.php
@@ -103,8 +106,18 @@ private function watch(InputInterface $input, OutputInterface $output)
while (true) {
try {
foreach ($this->am->getNames() as $name) {
- if ($this->checkAsset($name, $previously)) {
- $this->dumpAsset($name, $output);
+ if ($leafsOnly) {
+ $asset = $this->am->get($name);
+ foreach ($asset as $leaf) {
@stof

stof Mar 12, 2013

Member

This is wrong if the asset is not an AssetCollection

@stof stof commented on an outdated diff Mar 12, 2013

Command/DumpCommand.php
@@ -134,8 +147,14 @@ private function watch(InputInterface $input, OutputInterface $output)
*/
private function checkAsset($name, array &$previously)
{
- $formula = $this->am->hasFormula($name) ? serialize($this->am->getFormula($name)) : null;
- $asset = $this->am->get($name);
+ if (is_object($name) && $name instanceof \Assetic\Asset\BaseAsset) {
@stof

stof Mar 12, 2013

Member

is_object($name) is useless. $name instanceof \Assetic\Asset\BaseAsset will return false if it is not an object

@stof stof commented on an outdated diff Mar 12, 2013

Command/DumpCommand.php
@@ -134,8 +147,14 @@ private function watch(InputInterface $input, OutputInterface $output)
*/
private function checkAsset($name, array &$previously)
{
- $formula = $this->am->hasFormula($name) ? serialize($this->am->getFormula($name)) : null;
- $asset = $this->am->get($name);
+ if (is_object($name) && $name instanceof \Assetic\Asset\BaseAsset) {
+ $asset = $name;
+ $name = $asset->getSourcePath();
@stof

stof Mar 12, 2013

Member

This looks weird to me. It is not a formula name

Member

stof commented Mar 12, 2013

This will break if you have an asset for which you force being combined as the combined asset will not be dumped when using --leafs-only whereas you need it.

@artursvonda artursvonda Updated according to feedback
- Now honours "combine";
- Ignore assets that are not a collection;
- Added comments for clarity.
a286ea2

Thanks for feedback. Made some changes. If there's anything else I can fix/add, please, do tell.

@kriswallsmith kriswallsmith commented on the diff Mar 12, 2013

Command/DumpCommand.php
@@ -82,8 +83,10 @@ protected function execute(InputInterface $input, OutputInterface $output)
*
* @param InputInterface $input The command input
* @param OutputInterface $output The command output
+ * @param bool $leafsOnly
+ * @return void
@kriswallsmith

kriswallsmith Mar 12, 2013

Contributor

Why do you add this @return? Does it make a difference in your IDE?

Contributor

kriswallsmith commented Mar 12, 2013

I think this is a valid feature request, but I'm not sure about the implementation. I'll spend time looking at this in the next few days.

Sure, take your time. I'll admit it's not the most elegant solution. Maybe I'll figure out how to improve it over next few days.

@stof stof commented on the diff Mar 20, 2013

Command/DumpCommand.php
@@ -124,6 +137,26 @@ private function watch(InputInterface $input, OutputInterface $output)
}
}
+ private function shouldHandleLeafsSeparately($name)
+ {
+ $asset = $this->am->get($name);
+ // Ignore asset is not a collection
+ if (!($asset instanceof \Assetic\Asset\AssetCollectionInterface)) {
+ return false;
+ }
+ // If asset has no formula, no reason not to handle leafs separately
+ if (!$this->am->hasFormula($name)) {
+ return true;
+ }
+ $formula = $this->am->getFormula($name);
+ // Force single file if combine is true-ish
+ if (!empty($formula[2]['combine'])) {
@stof

stof Mar 20, 2013

Member

Actually, if combine is not defined, it would default to debug which itself default to $am->getDebug().

So you need to compute the actual value of combine

Is this related to symfony#123 ?

Yes, it tries to solve the same problem.

ruudk commented May 28, 2013

@kriswallsmith Any news on this?

Closing. Replaced by symfony#233

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment