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.

Sign up for GitHub

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 snippets support #147

Merged
merged 10 commits into from
Jul 2, 2019
Merged

Add snippets support #147

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
ff3716e
#146 - Implement snippet helper
johanjanssens Jun 30, 2019
99bc02f
#146 - Fix typo
johanjanssens Jun 30, 2019
739e75e
#146 - Do not unpack params if we only have one method parameter
johanjanssens Jul 1, 2019
d0e2b6d
#146 - Cleanup whitespace before " and >
johanjanssens Jul 1, 2019
32367e8
Merge branch 'master' into feature/146-snippets
johanjanssens Jul 1, 2019
0df1956
#146 - Do not pass params to the helper constructor
johanjanssens Jul 1, 2019
9a1acd9
#146 - Use php template engine to expand a snippet
johanjanssens Jul 1, 2019
33219b3
#146 - Find single whitespace before " or before > in html tags and r…
johanjanssens Jul 1, 2019
dbc115a
#146 - Add invoke method to allow defining and expanding in the same way
johanjanssens Jul 1, 2019
39c92e4
#146 - Throw RuntimeException if the snippet doesn't exist
johanjanssens Jul 1, 2019
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
8 changes: 7 additions & 1 deletion code/site/components/com_pages/template/default.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,13 @@ public function invokeHelper($identifier, ...$params)
$params = array_merge($this->getParameters()->toArray(), $params);
}

return $helper->$function(...$params);
if(is_numeric(key($params))) {
$result = $helper->$function(...$params);
} else {
$result = $helper->$function($params);
}

return $result;
}

public function createHelper($helper, $config = array())
Expand Down
40 changes: 40 additions & 0 deletions code/site/components/com_pages/template/helper/snippet.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php

class ComPagesTemplateHelperSnippet extends ComPagesTemplateHelperAbstract
{
private $__snippets = array();

public function define(string $name, string $snippet, $overwrite = false)
{
if(!isset($this->__snippets[$name]) || $overwrite) {
$this->__snippets[$name] = $snippet;
}
}

public function expand(string $name, array $variables = array())
{
$result = false;

if(isset($this->__snippets[$name]))
{
$snippet = $this->__snippets[$name];

//Use the stream buffer to evaluate the partial
$str = "<?php \n return <<<SNIPPET\n$snippet\nSNIPPET;\n";

$buffer = $this->getObject('filesystem.stream.factory')->createStream('koowa-buffer://temp', 'w+b');
$buffer->truncate(0);
$buffer->write($str);
Copy link
Member

Choose a reason for hiding this comment

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

@johanjanssens Does it make sense or is it possible to cache these buffers instead of the snippets? Would save us some processing?

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m happy to work on all these smaller changes, but right now I already lost more then a week on this ‘make html in helpers’ flexible. Starting to feel like micro-optimisations for problems we don’t immediately have.

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. It should be quite fast compared to template rendering anyway so let's roll with it.

Copy link
Member Author

Choose a reason for hiding this comment

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

@ercanozkaya Not giving up that easily. I changed the implementation to use the php template engine. This way we have build in caching out of the box. See: 9a1acd9


extract($variables, EXTR_OVERWRITE);

$result = include $buffer->getPath();

//Cleanup whitespace
$result = str_replace(array(' >', ' "'), array('>', '"'), $result);
Copy link
Member

Choose a reason for hiding this comment

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

@johanjanssens would this not cleanup the quotes in string literals too? say you have a string like click "subscribe" to proceed in your snippet. This would turn it into click"subscribe" to proceed right?

Copy link
Member Author

Choose a reason for hiding this comment

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

True, this needs a more advanced regex, haven’t gotten to that yet. Reason for this is because it generated clean html which was a request from Robin.

Copy link
Member

Choose a reason for hiding this comment

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

@johanjanssens Once you encounter this problem it's very hard to track it down and work around it so I would leave this out.

Copy link
Member Author

Choose a reason for hiding this comment

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

Optimised this with a simple regex. I’m now only stripping single whitespace before “ and > in html tags. See: 33219b3

}

return $result;
}
}