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

Allow nested shortcodes #2748

Merged
merged 5 commits into from
Jan 2, 2025

Conversation

ARitz-Cracker
Copy link

Fixes #515

Hello, and Happy Christmas 🎄

This PR iterates on #2630 with the given feedback of not requiring the use of the markdown filter to execute nested shortcodes. Additionally, unlike the previous PR, nested nth invocation increments follows a more intuitive pattern.

Before:

  • 1
  • 2
    • 5
    • 6
      • 9
      • 10
    • 7
    • 8
  • 3
  • 4

After:

  • 1
  • 2
    • 3
    • 4
      • 5
      • 6
    • 7
    • 8
  • 9
  • 10

Sanity check:

Code changes

  • The shortcode parser has been changed to stop at the appropriate {% end %}, instead of the first-encountered {% end %}. (Thanks to Allow recursively nested shortcodes #1475)
  • A clonable shared ShortcodeInvocationCounter has been added
  • The markdown::shortcode::parser::Shortcode struct now has inner: Vec<Shortcode>
  • The markdown::shortcode::parser::parse_for_shortcodes function now also parses shortcode bodies
  • Added a flatten method to markdown::shortcode::parser::Shortcode which renders the inner-shortcodes into the shortcode body
  • The render method on markdown::shortcode::parser::Shortcode now calls flatten
  • Are you doing the PR on the next branch?

If the change is a new feature or adding to/changing an existing one:

  • Have you created/updated the relevant documentation page(s)?
    • As far as I'm aware, the documentation does not explicitly prohibit nested short-codes.

Let me know what you think!

components/markdown/src/shortcode/parser.rs Show resolved Hide resolved
components/markdown/src/shortcode/parser.rs Outdated Show resolved Hide resolved
components/utils/src/templates.rs Outdated Show resolved Hide resolved
@ARitz-Cracker
Copy link
Author

Oh, another thing, I noticed that when it comes to MD shortcodes calling HTML shortcodes, the HTML shortcode must be multi-line otherwise its result will get wrapped in a <p> tag. Where would be the appropriate place to document this?

@Keats
Copy link
Collaborator

Keats commented Dec 26, 2024

Oh, another thing, I noticed that when it comes to MD shortcodes calling HTML shortcodes, the HTML shortcode must be multi-line otherwise its result will get wrapped in a

tag. Where would be the appropriate place to document this?

Not sure honestly. Sounds like a bug maybe?

@ARitz-Cracker
Copy link
Author

Oh, another thing, I noticed that when it comes to MD shortcodes calling HTML shortcodes, the HTML shortcode must be multi-line otherwise its result will get wrapped in a tag. Where would be the appropriate place to document this?

Not sure honestly. Sounds like a bug maybe?

I... Actually don't think so, iirc if you have a single line with a <span> tag in an MD document, it being a part of a paragraph is expected behaviour, and after skimming through pulldown-cmark's source, it doesn't seem to have has inherent knowledge on whether or not an HTML element is an inline element (like a span) or a block element (like a div) resolving such issues seems outside the scope of this PR.

@ARitz-Cracker
Copy link
Author

Applied the feedback given, lmk what you think!

Copy link
Collaborator

@Keats Keats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Keats Keats merged commit 336533d into getzola:next Jan 2, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants