getzola / zola

A fast static site generator in a single binary with everything built-in. https://www.getzola.org
https://www.getzola.org
MIT License
13.54k stars 947 forks source link

Two shortcode problems #2564

Open xpe opened 3 months ago

xpe commented 3 months ago

Bug Report

Correct behavior

To illustrate that my example is working... both of the following work:

Both render the contents of foobar.html as expected. In the second case, the "x" is discarded, which makes sense, since there is no <body> in the footbar.html.

Incorrect behavior

{% foobar() %}{% end %} does not render the shortcode. Instead, it renders itself; i.e. {% foobar() %}{% end %}

This should render the shortcode, just like {{ foobar() }}. Zola has the information it needs to do this.

Unhelpful behavior

{% foobar() %} renders itself; i.e. {% foobar() %}

This is virtually never what a user wants, so this behavior isn't helpful. I would suggest a "fail fast" error instead.

Instead, Zola could detect the missing end tag and offer a suggestion to either:

Contents of foobar.html:

<small>Rendered using custom Hogwarts magic.</small>

Environment

Zola version: zola 0.18.0

Keats commented 3 months ago

The shortcode parser only handles valid shortcodes calls, eg {% foobar() %} is not a valid shortcode so it is ignored. This is (IMO) the right behaviour since otherwise you have to escape everything that looks like Jinja/Tera in your Markdown.

We could make {% foobar() %}{% end %} work though, it seems reasonable.

xpe commented 3 months ago

The shortcode parser only handles valid shortcodes calls...

Ok. Let's put this in a broader context: many parsers do more than handle valid calls. Lots of parsers help the user with error recovery too.

Second, given that {% foobar() %} is invalid, could it ever be the case that it makes sense to continue processing? (Maybe you explained this above in saying "since otherwise you have to escape everything...", but I'm not seeing the connection yet.)

Keats commented 3 months ago

{% foobar() %} is not an error for the shortcode parser. It's just some random content that is not a shortcode. The shortcode with a body has to have an end: https://github.com/getzola/zola/blob/master/components/markdown/src/content.pest#L55. Write {% foobar() %}x{% nd %} (missing e in the tag end) and the parser will ignore it entirely - no error involved.

Second, given that {% foobar() %} is invalid, could it ever be the case that it makes sense to continue processing? (Maybe you explained this above in saying "since otherwise you have to escape everything...", but I'm not seeing the connection yet.)

We don't stop processing anything when we see only {% foobar() %}, it's just seen as text by the parser. We could write a different parser that errors on anything that looks like shortcodes, that's just not the way it's done right now.

Maybe mimicking the syntax of Tera was not a good idea in practice since many people confuse shortcodes with macros because they kinda look the same.