revolution
revolution copied to clipboard
modxcms
Enhance debugging of chunks
Summary
Knowing what placeholders are available inside a chunk is more or less a gamble. You can read the documentation or study the code. Both methods are not very reliable and can be quite time-consuming.
I propose to add a generic placeholder to getChunk ([[+chunk.properties]]). It will contain all properties given to getChunk and can be output inside the chunk:
pdoMenu / wayfinder chunk:
<!-- [[+chunk.properties]] -->
<li>[[+menutitle]]<li>
Solution
Add a new placeholder to getChunk: #1872 - https://github.com/modxcms/revolution/blob/4beca05219ea5e3b4011ff54268dc54e8623163a/core/model/modx/modx.class.php
public function getChunk($chunkName, array $properties= array ()) {
// add debug placeholder here
$properties['chunk']['properties'] = print_r($properties,true);
//
$output= '';
if ($this->getParser()) {
$chunk= $this->parser->getElement('modChunk', $chunkName);
if ($chunk instanceof modChunk) {
$chunk->setCacheable(false);
$output= $chunk->process($properties);
}
}
return $output;
Questions:
- I'm not sure about the placeholder name. Do you have a better one?
- Can this possibly break something?
Regards, pepebe
Edit: I guess this should also be added to parseChunk()
Please also add chunk.name because it can take a hell of a time to find out which chunk created an output.
Being able to put the name of a chunk into a comment line would help a lot – especially when a chunk gets renamed or is being duplicated.
Nice idea, but this placeholder generation should only be enabled with a system setting.
@Jako Opt-in is always a good thing!
@mindeffects And chunk.id would also be a nice thing. I have written an issue about this: https://github.com/modxcms/revolution/issues/12510
I'd use placeholders instead of properties for the naming - just to keep things accurate - but I can't really argue against the idea.
Though I do wonder.. how come @mindeffects get to have 10 votes?! [/random]
@Mark-H Good point!
Picking up on something Mark mentioned in the other thread ( https://github.com/modxcms/revolution/issues/12510 ), a generic prefix could also be a good idea:
[[+_self.placeholders]], [[+_self.id]], [[+_self.name]]
I suggest that [[+_self.placeholders]] should contain the names of the passed properties only since the values can be called the "regular" way.
Excellent idea, but I don't like variant with an underscore. [[+self.placeholders]] looks more readable.
With underscore there is a lower risk of overwriting a property set by a user/developer.
It doesn't matter because I can use custom placeholder with _ too. Better to log a warning when such placeholder used but keep these things simple. Any special symbol increases the complexity of MODX because you need to remember about it.
Then I suggest naming it this. People who work with objects are already familiar with $this.
My first idea was the same, too: Why use „self“ and not „this“? Let‘s use „this“!
Yes, [[+this.placeholders]] is much more consistent with our experience and expectations.
I'll write the necessary changes and try my luck with a PR.
I wonder how we could "protect" "this." from being used by other extensions. Is anyone aware of such a placeholder?
I wonder how we could "protect" "this." from being used by other extensions. Is anyone aware of such a placeholder?
Log warn or error into the log.
Nice idea. But I would do it through a new modifier "print" to output arrays. Sometimes it can be useful for snippet's output when it returns an array.
[[+this:print]] // It prints an array of the modChunk object (the placeholder "this" is obtained from the modChunk::toArray() method).
[[+this.placeholders:print]]
