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

feat: Add strip_p filter to remove p tag around text #368

Closed
wants to merge 1 commit into from

Conversation

pawamoy
Copy link
Member

@pawamoy pawamoy commented Feb 1, 2022

Sometimes we convert a single line of Markdown and we don't want to keep the surrounding <p> tags. This filter allows to strip them.

Example use-case:

<details class="{{ section.value.kind }}">
  <summary>{{ section.title|convert_markdown(heading_level, html_id)|strip_p }}</summary>
  {{ section.value.contents|convert_markdown(heading_level, html_id) }}
</details>

The filter does nothing if the markup is not wrapped in <p>.

Copy link
Member

@oprypin oprypin left a comment

Choose a reason for hiding this comment

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

Wait, that's not so nice to do 😅
I'll try if there's a nicer alternative

@oprypin
Copy link
Member

oprypin commented Feb 2, 2022

I have provided my attempt in #369. It's comparatively heavy, though :/

Which things I thought might be downsides and that I managed to resolve:

  • I feel like the convert_markdown filter itself should be removing the tag, with a parameter. The current approach implies that there isn't any way to not have that tag there in the first place. But why (other than implementation details) should it be a separate step?

  • What if the content is several paragraphs but it still tries to remove them? <p>Foo</p><p>Bar</p>

  • (Far-fetched but) what if the p tag has attributes? <p x="y">Bar</p>
    Or generally this approach could somehow remove the start tag but not the end tag.

@pawamoy pawamoy closed this in #369 Feb 2, 2022
@pawamoy
Copy link
Member Author

pawamoy commented Feb 2, 2022

But why (other than implementation details) should it be a separate step?

No reason at all, I tried adding the option to the convert_markdown filter and it was ugly 😅
Thanks again for your better PR 🙂

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