FilterNotFound

[candlerb]

I notice that when a filter is not found, Liquid ignores it silently.
I would like to have an exception to report the problem.

There appears to be a half-implementation of this already. In lib/
liquid/variable.rb:

begin
output = context.invoke(filter[0], output, *filterargs)
rescue FilterNotFound
raise FilterNotFound, "Error - filter '#{filter[0]}' in '#
{@markup.strip}' could not be found."
end

However this exception is never raised, because in Content#invoke it
says

def invoke(method, *args)
if strainer.respond_to?(method)
strainer.__send__(method, *args)
else
args.first <<<< should be raise FilterNotFound??
end
end

Would people be OK with me changing this to raise FilterNotFound?

Regards,

Brian.

[Tobias Lütke]

Liquid comes with two rendering modes.

render and render! . render! should raise exceptions ( all the way to
the top ) when such errors happen and render() should fail silently (
and hopefully put a warning into template#warnings / template#errors.
This would be a good addition to liquid but the normal .render mode
should be unaffected by the change.


Regards
-- tobi

[Brian Candler]

On Sat, Jun 06, 2009 at 12:31:00AM -0400, Tobias Lütke wrote:
> Liquid comes with two rendering modes.
>
> render and render! . render! should raise exceptions ( all the way to
> the top ) when such errors happen and render() should fail silently (
> and hopefully put a warning into template#warnings / template#errors.

Yep, I found (and use) that already.

> This would be a good addition to liquid but the normal .render mode
> should be unaffected by the change.

The difference is that it now renders an error message about the
non-existent filter into the output, just like other exceptions raised
during rendering.

The change is at
http://github.com/candlerb/liquid/commit/01c25a11a3923d9f3a8a00b821c0aba2b586da23

Note that I had to modify a number of specs to accommodate the new
behaviour, because the error is now rendered in-line. I do think this is a
change for the better, but if you don't want to change the behaviour, then
it can instead throw FilterNotThrown only if rethrow_errors is set. What do
you think?

BTW, I can't see the difference between 'test_no_instance_eval' and
'test_no_instance_eval_after_mixing_in_new_filter' in test/security-test.rb.
Was something omitted from the second test?

Regards,

Brian.

[Tobias Lütke]

I applied all your patches. Thank you for the great work.

Regards
-- tobi