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

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

Closed
Lomanic opened this issue Nov 25, 2015 · 10 comments
Closed

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

Lomanic opened this issue Nov 25, 2015 · 10 comments

Comments

@Lomanic
Copy link

Lomanic commented Nov 25, 2015

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
Copy link
Collaborator

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 😉 😃)

@theshka
Copy link
Collaborator

theshka commented Nov 27, 2015

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.

@theshka theshka closed this as completed Nov 27, 2015
@PhrozenByte
Copy link
Collaborator

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

@theshka
Copy link
Collaborator

theshka commented Nov 27, 2015

Well then, even better! 😆

@Lomanic
Copy link
Author

Lomanic commented Nov 27, 2015

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.

Lomanic added a commit to Lomanic/Pico-Editor-Plugin that referenced this issue Nov 27, 2015
@PhrozenByte
Copy link
Collaborator

@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! 👍 😃

@theshka
Copy link
Collaborator

theshka commented Nov 27, 2015

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:

  • creating, opening, saving, and deleting markdown files that are in sub/ directories.
  • currently it needs to be cloned into, or renamed PicoEditor to function, if you do not specify, it gets cloned as Pico-Editor-Plugin
  • perhaps a small asset-manager?

Some things you already mentioned

  • better login
  • better error handling
  • better code in general

It would be very much helpful if you were able to take care of this! 👍 😄

@Lomanic
Copy link
Author

Lomanic commented Nov 27, 2015

@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
Copy link
Contributor

dav-m85 commented Nov 28, 2015

Nice @Lomanic. What do you mean by

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

@Lomanic
Copy link
Author

Lomanic commented Nov 29, 2015

@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.

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

No branches or pull requests

4 participants