picocms / Pico

Pico is a stupidly simple, blazing fast, flat file CMS.
http://picocms.org/
MIT License
3.82k stars 616 forks source link

Unable to call `link` filter in onPageRendering hook in plugin #288

Closed Lomanic closed 8 years ago

Lomanic commented 8 years ago

I am backporting my version of the Pico-Editor plugin using @theshka's fork as a new base (Lomanic/Pico-Editor-Plugin#2) to make it Pico v1 compatible.

Currently I can't call {{ editor_url | link }} (with editor_url being the customizable admin endpoint, 'admin' by default) in editor.twig inside the onPageRendering(&$twig, &$twigVariables, &$templateName) hook. I presumed it is defined after this hook but...

This hook is defined at https://github.com/picocms/Pico/blob/c34afad/lib/Pico.php#L365 Even if the link filter is defined inside the registerTwigFilter() function it is finally used used in the onTwigRegistration hook... before the onPageRendering hook?!

Here is the code currently: https://github.com/Lomanic/Pico-Editor-Plugin/tree/picov1. I'm using the current master on nginx and php 5.6.14.

Here are the errors while logging into the editor:

<br />
<b>Fatal error</b>:  Uncaught exception 'Twig_Error_Syntax' with message 'Unknown &quot;link&quot; filter in &quot;editor.twig&quot; at line 20.' in /var/www/X/public_html/vendor/twig/twig/lib/Twig/ExpressionParser.php:601
Stack trace:
#0 /var/www/X/public_html/vendor/twig/twig/lib/Twig/ExpressionParser.php(458): Twig_ExpressionParser-&gt;getFilterNodeClass('link', 20)
#1 /var/www/X/public_html/vendor/twig/twig/lib/Twig/ExpressionParser.php(443): Twig_ExpressionParser-&gt;parseFilterExpressionRaw(Object(Twig_Node_Expression_Name))
#2 /var/www/X/public_html/vendor/twig/twig/lib/Twig/ExpressionParser.php(302): Twig_ExpressionParser-&gt;parseFilterExpression(Object(Twig_Node_Expression_Name))
#3 /var/www/X/public_html/vendor/twig/twig/lib/Twig/ExpressionParser.php(194): Twig_ExpressionParser-&gt;parsePostfixExpression(Object(Twig_Node_Expression_Name))
#4 /var/www/X/public_html/vendor/twig/twig/lib/Twig/ExpressionParser.php(84): Twig_ExpressionParser-&gt;parsePrimaryExpression()
#5 /var/www/X/public_html/ in <b>/var/www/X/public_html/vendor/twig/twig/lib/Twig/ExpressionParser.php</b> on line <b>601</b><br />
PhrozenByte commented 8 years ago

You're not using Pico's Twig_Environment (onPageRendering()'s $twig parameter), but your own Twig_Environment ($twig_editor), that simply doesn't know Pico's custom filters. The reason for this might be the need of another Twig loader (specifically a Twig_Loader_Filesystem instance pointing to the plugins template dir), but that's no good solution anyway. Call $twig->setLoader($loader) instead, change $twigVariables and $templateName appropriately and remove the echo $twig_editor->render() and exit calls (same applies to @theshka's fork btw :wink: :smiley:)

theshka commented 8 years ago

This has been basically fixed in my fork, I think this needs more discussion though, in a new issue...

If @Lomanic is interested in developing the Pico-Editor-Plugin source to be on par with Pico 1.0 (and possibly use as Pico's 'official' admin plugin) by all means, please do. I basically copied it verbatim from gilbitron's repo, and wrapped it in @PhrozenByte's DummyPlugin for Pico 1.0, and included some of the outstanding PR's on the original fork.

IMO, it needs a rewrite. There is lots of missing functionality, and it's very finicky/rigid.

PhrozenByte commented 8 years ago

I have planned to write a admin plugin from scratch for Pico 1.1, just fyi. :wink:

theshka commented 8 years ago

Well then, even better! :laughing:

Lomanic commented 8 years ago

Hello @theshka @PhrozenByte

Thanks @PhrozenByte for the tip, it will be in my next commit.

I'm indeed interested in developing this plugin, what do you think should be rewritten @theshka ? What will be the differences with your own plugin @PhrozenByte ?

I was planning perhaps to add to it an anti-bruteforce system (fail2ban-like), improve error detection when using AJAX calls (not enough rights to write/create/delete .md file mostly), perhaps making the AJAX admin/{new|open|save|delete} endpoints REST-like using HTTP error codes, use the awesome woofmark JS editor too instead of epiceditor (hard for me as I am really bad at making beautiful HTML and CSS). Nothing fancy, I'm waiting for your suggestions, I'm totally OK about building a new plugin from scratch if you think it is a better thing to do.

PhrozenByte commented 8 years ago

@Lomanic: I don't have a concrete plan yet, just telling the necessity to do it. I think building a new admin plugin from scratch gives a better result. Your ideas sound excellent. It would be great if you take care of this! :+1: :smiley:

theshka commented 8 years ago

I'm a fan of your ideas for the plugin as well @Lomanic

Some things it seriously needs to address in it's current state:

Some things you already mentioned

It would be very much helpful if you were able to take care of this! :+1: :smile:

Lomanic commented 8 years ago

@theshka I was planning a minor change for a more flexible plugin dir too, just checking the current dir name and pass it to the editor.twig renderer like the $this->url var (I don't know if it is the same idea you have about this issue). But it would be a problem with #203 perhaps.

dav-m85 commented 8 years ago

Nice @Lomanic. What do you mean by

perhaps to add to it an anti-bruteforce system (fail2ban-like). ?

Lomanic commented 8 years ago

@dav-m85 The plugin would log in a file failed login attempts for each IP address, if an IP address fails X times (X being configurable in config/config.php) it gets rejected during a defined time (customizable in config/config.php too).

@theshka https://github.com/coexec/pico_admin provides an asset manager I think.