-
Notifications
You must be signed in to change notification settings - Fork 990
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
Shortcode names and arguements. #165
Comments
It should be fixed in 0.2.2 that was released today but I guess the package managers are not updated yet |
I updated to the current release and it still doesn't seem to work as expected.
It's similar with the brackets in the string.
I might be able to write at least a test case if not give a little more info to troubleshoot with if you point me in the right direction. |
That's just the markdown renderer splitting on the markdown characters, there's a test for it but obviously it wasn't enough :( #160 is planning to refactor all of the markdown rendering so that should fix it for real |
We'll be addressing this and adding more tests as part of the work being done in #174. |
I don't think it's possible to fix this by just changing how events generated by extern crate pulldown_cmark;
use pulldown_cmark as cmark;
fn main() {
let markdown1 = r#"{{ foo(bar="`baz`") }}"#;
let parser1 = cmark::Parser::new(markdown1);
let markdown2 = r#"{{ foo(bar="``baz``") }}"#;
let parser2 = cmark::Parser::new(markdown2);
for (ev1, ev2) in parser1.zip(parser2) {
println!("{:>25} == {}", format!("{:?}", ev1), format!("{:?}", ev2));
}
} which prints
Note that the events are exactly the same, even though the I see three ways to correctly handle shortcodes: 1. Change the shortcode delimiters to suppress Markdown parsingIf the expression in the shortcode tag was enclosed in a Markdown code span, such as For shortcodes with a body, the body would also need to be enclosed in a Markdown code block to prevent it from being parsed by the Markdown parser, e.g.
I'm fairly sure this strategy will work, although getting it correct might be difficult, and it's not very pretty. 2. Parse and render shortcodes before parsing MarkdownThis strategy is to convert the content to HTML in two stages: Unlike with the previous approach, if a Markdown code block contains text that looks like a shortcode tag and you don't want it to be rendered as a shortcode, you have to escape it (since the Markdown isn't parsed until after the shortcodes are rendered). To do this, it would be possible to add
which would become
after rendering shortcodes. (I provided this method in my implementation.) 3. Treat the content as a Tera templateThis is the same as the previous strategy, except instead of implementing a custom shortcode parser/renderer, you can just use Tera. This approach is nice because it's the simplest to implement, you know that shortcodes (i.e. Tera macros) will behave the same in the content and in templates, and the user has the full power of Tera templates at their disposal when writing their content. Similar to the previous approach, you can escape text that looks like a Tera tag by using Note that currently Tera doesn't have macros that take a body. It would be possible to add macros with a body to Tera, or you could achieve the same functionality with string arguments. For example, you could have the macro
and then call it like
What do you think? Personally, my preference is strategy 3. |
I like strategy 3 the best as well. But it seems to me like adding body arguments to Tera would also be a nice thing to do to keep the syntax the same. @reillysiemens and I are currently working on rewriting the markdown parser to clean it up. We can also switch the code to treating the body as a Tera template, which will simplify the reimplementation a lot! @jturner314 if you have time and would like to, maybe you could implement the new body syntax in Tera? Or we'll add it to our list in our current yak shave 😆 |
I agree. I also realized that handling nested shortcodes/macros works a lot better if macros can have a body. (Ideally, shortcode parsing/rendering would be applied to the body of a "shortcode with a body" but not to any string literals passed as arguments.)
Sure, I can work on that. I'll also add support for Rust-style string literals. |
I'm not sure macros with a body make sense in Tera itself, I don't see a usage for it. Not sure about Rust strings as well although I can see the need when you want to have {% set message = `Hello "world"` %}
{% set message = r#"Hello "world""# %} I find the first one more readable and the 2nd is only understandable by Rust devs. I'm a bit tired but isn't there solution 4?
This way is a bit slower as you go line by line but I don't think it will make a huge difference and you can keep the current syntax. |
I agree that it's somewhat unusual for most use cases, although Gutenberg shortcodes are one example where this would be useful. Additionally, it's worth noting that Jinja has this feature. (See the first example in the Call section of the docs. The
Yeah, this is probably somewhat unusual. I came across this when I tried to write a
I think the combination of Rust-style regular string literals and raw string literals is really convenient, although I understand the concern, and Javascript-style string literals with backslash escapes and Now that I think about it, what's your opinion on Python-style literals (
If I understand correctly, that's effectively the same as option 2 (custom shortcode parser, with The primary reason why I suggested using Tera (option 3) instead of using a custom shortcode parser (option 2) for rendering shortcodes before parsing Markdown is to avoid having to effectively reimplement portions of the Tera parser in Gutenberg with regexes/Pest. However, it's certainly possible to use a custom shortcode parser separate from Tera. I'm willing to work on the implementation however you'd like it done. |
I'm aware of the
I would say that points to solving that in Gutenberg and not Tera. We don't need the shortcode calls to be limited to what's possible in Tera, and we only really want bool/numbers/string as values there.
I think adding all kinds of quotes/ticks as kind of synonyms make sense. Someone is working on string concat (Keats/tera#222) so if you want to avoid escaping you could do For the parsing, I would definitely go for solution 2 to get the best UX as an editor. To avoid the line-looking-like-shortcode issues, a simple hack could be to check if we are in a code block while iterating and do nothing if it's the case. Code blocks are the only moment where I believe you could have a line looking like a shortcode in a markdown document, unless you're writing weird content. |
Okay, that works for me.
That's a reasonable approach.
Reliably determining whether the shortcode parser (i.e. the iterator over lines) is currently in a code block is surprisingly complex. For example, the parser needs to know the difference between block and inline HTML tags. Consider the following Markdown document:
which results in the HTML: <div>
~~~
<p>{{ foo() }}</p> In this case,
the resulting HTML is: <p><span></p>
<pre><code>
{{ foo() }}
</code></pre> Whether or not Maybe we could fork I expect that text that looks like a shortcode is pretty rare anyway, so the user won't need to use Another consideration is that if Gutenberg adds support for markup languages other than Markdown, it's a lot easier to deal with
Yes, I think it's possible to use regexes instead of something like Pest. |
Hmm we still need to check if we are in a code block, unless we force users to wrap If it ends up being added, I would call them |
Yes, that's what I'm proposing.
I assume you mean something like this? let mut in_code_block = None;
for line in lines {
in_code_block = match in_code_block {
None => {
if the line matches regex "^ {0,3}(?P<fence>`{3,}|~{3,}).*$" {
Some(captures.fence)
}
}
Some(fence) => {
if the line matches regex format!("^ {{0,3}}{}{}* *$", fence, fence[0]) {
None
}
}
};
} These are the costs/benefits to the two approaches:
That's true. Another (edge) case where a naive check will fail is this:
In this case, the code block started by
then The difference is that
Fair enough. I guess I just weigh the costs/benefits differently. I think as long as you require shortcodes to:
a naive check will be correct unless
but there may be some additional edge cases that I haven't considered. Edit: One example where inline shortcodes would be useful is for mathematics. For example, you could write |
That's pretty much what's required right now for shortcodes to work. Thoughts on that @RadicalZephyr @reillysiemens ? |
I think I'm more inclined towards the conceptual simplicity of not checking whether a shortcode is inside a code block, since it's another piece of mutable state to track on top of whether we're inside raw/ignore tags. I think it will also make explaining to users when they need to use the i.e. without any checking for context of shortcodes, the rule is:
Versus with the code check:
And as a user, I don't think I would be annoyed by having to escape code that looks like a shortcode. The only time I write code in a blog post that looks like that is when I'm writing about a templating language, or possibly writing a post about using Gutenberg itself, which for the "typical" user is probably fairly rare. I also suspect that the confusion stemming from when Also, once we introduce a naive code block check, overtime it will probably become very tempting to make it less naive, and the end result of that is rewriting the markdown parser. This is definitely a bit of an ad absurdum argument, but that's where I see it heading and it sounds like a headache to support it to me. But if you really want the code block check, it's not a dealbreaker for me 😁 Also, possibly the "rightest" way to solve this is to patch the parser in pulldown-cmark so that we don't lose the information we need/want for parsing shortcodes, or make their parsing extensible so we can modify it to actually emit shortcode events. But that's a whole other yak to shave. |
Majority rules! I'll take PRs for that and we can bikeshed on the |
Are you going to collaborate on some branch? I'm guessing this would simplify the markdown rendering tremendously. |
Sorry about the delay; Thanksgiving holiday was last week in the US. The shortcode parsing is fairly independent of the Markdown parsing. (The shortcode parsing/rendering is a In the meantime, @RadicalZephyr can use this stub in use std::borrow::Cow;
use context::Context;
use errors::Result;
pub fn render_shortcodes<'a>(content: &'a str, context: &Context) -> Result<Cow<'a, str>> {
Ok(Cow::from(content))
} Then, use context::Context;
use errors::Result;
use short_code::render_shortcodes;
use table_of_contents::Header;
pub fn markdown_to_html(content: &str, context: &Context) -> Result<(String, Vec<Header>)> {
let after_shortcodes = render_shortcodes(content, context)?;
// parse `after_shortcodes` instead of `content`
// ...
} Does that sound like a good plan? |
Sounds perfect to me! |
I'll release 0.3.0 this week since there are quite a few bugfixes already accumulated and hoping/working to have that in the major version after that |
Hello. I came here from #225 where I ran into problems with inadequate flexibility of shortcodes. I briefly read the discussion above. My conclusion so far is none of the proposals so far help me to achieve what I'd like. From where I'm standing, there are two problems with shortcodes:
I've given the problem a bit of thought and I've come with the following idea. The parsing problem would be solved by dropping shortcodes and replacing them with "Tera blocks" parsed before Markdown. The context problem would be solved by evaluating the Tera blocks later in the pipeline. This is what a Tera block looks like in my mind: <?tera ...arbitrary Tera code goes here... ?> This syntax will probably be weird/ugly to you at first, but there are two reasons for using it:
The parsing and evaluation would go as follows:
Because of the late evaluation, Tera blocks would have to be forbidden in page summary. Otherwise it seems to me like this offers the greatest deal of flexibility while not getting in the way of Markdown parsing (to my best knowledge). I know this is a radical change not likely to be met with much of acceptance, but I think I'm just going to implement this, because I need a static site generator with support for picture galleries soon-ish and other static site generators are generally horrible... |
I believe passing the site config would be easy to add, the page as well probably. It's not done currently because I wanted to have the markdown rendering have as few dependencies on the rest of Gutenberg as possible but that's doable.
That doesn't work for re-usability. Let's say I want to have a |
Hm. I'm not sure this would work... For example, I think assets are listed later than Markdown parsing.
My idea was to use Tera macros. Ie. all the macros that are available to templates should also be available to Tera blocks. Which should also enable shipping them with themes. Ie. for example |
Nope, Markdown rendering is done once all pages are loaded as we need to have all pages/sections permalinks to resolve internal links.
Macros have to be imported and namespaced to avoid clashing: a theme might have a What would your photo gallery macro/shortcode call/implementation look like? |
Ah, okay.
I forgot about the import bit :-( (But perhaps that could be solved somehow.) As for clashes, use namespaces? Ie. the theme might use its namespace and so could you...
I uploaded an example of my WIP blogpost in the picture thread. Presumably, in Gutenberg / Tera, the macro/shortcode could be something like this: {% for thumb in img_scale(imgs=page.assets, w=..., h=...) %}
{# ... render elements in a way similar to the example... #}
{% endfor %} (I haven't considered the "screen" resize yet I use in my Pelican setup ...) |
Ok, regarding the context, I guess this is just a matter of adding page metadata to the context, which should be straightforward. Thanks for the clarification on that. Regarding macro import, this could be done with auto-importing macros at the start of each tera code, kind of like Rust inserts the std prelude automatically. That is, built-in stuff like ... I'm just throwing ideas around, basically this is just me needing a working prototype. I'll try to implement something like this over this weekend hopefully and then given time we'll see if these ideas are worthwhile or not ... |
I have to say I don't really see the advantages of macros vs shortcodes. Macros can call other macros (same namespace or another one) so that can become kind of mess to deal with without handling namespaces yourself. |
Okay, I suppose that's a good point. |
Looking at the shortcode parsing code made me realize it won't work when a string argument contains a comma ( Then again, I think you are right about macros not being very suitable as a replacement. So here's another idea: Perhaps guteberg could load all the shortcodes from the shortcodes directory and make them available as global functions, |
I finally started working on the rewrite, and so far it's looking like it's going to be something like:
In the end I'm doing a pest parser like @jturner314 did 🙇♂️ |
It appears as though shortcode file names cannot have an underscore in them and that the value that argument values cannot contain special characters.
To reproduce the filename issue:
./templates/shortcodes/contact_info.html
.To reproduce the argument issue:
./templates/shortcodes/contact.html
.This is exacerbated because it appears the dev server will fall back to a previous state when it cannot build correctly...and then sometimes gets stuck and needs to be manually restarted.
The text was updated successfully, but these errors were encountered: