allejo / jekyll-anchor-headings

A GitHub Pages compatible way of adding anchors to your headings without a plug-in or JavaScript :octocat:
https://pure-liquid.allejo.org/
MIT License
186 stars 31 forks source link

Raw text outside tags causes broken HTML to be rendered #13

Closed allejo closed 4 years ago

allejo commented 4 years ago

My Markdown HTML

Some text<p data-name="short_description">Defines which sample file the region will play.</p>

<p>Possibly the most important opcode, this is the one that tells the sampler which
sample file to actually play. This should include a relative file path from the
folder where the SFZ file is.</p>

<h2 id="examples">Examples</h2>

Snippet Usage

{% include anchor_headings.html html=content %}

Expected HTML

Some text<p data-name="short_description">Defines which sample file the region will play.</p>

<p>Possibly the most important opcode, this is the one that tells the sampler which
sample file to actually play. This should include a relative file path from the
folder where the SFZ file is.</p>

<h2 id="examples">Examples <a href="#examples"></a></h2>

Actual HTML

<hSome text<p data-name="short_description">Defines which sample file the region will play.</p>

<p>Possibly the most important opcode, this is the one that tells the sampler which
sample file to actually play. This should include a relative file path from the
folder where the SFZ file is.</p>

<h2 id="examples">Examples <a href="#examples"></a></h2>

Notes

Reported by the @sfzformat team

redtide commented 4 years ago

Just tested the hotfix and it fixed the tag issue.

I wonder if there is a need to document that specific case BTW, where the script works 2 times (adding 2 heading anchors) when called by 2 layouts used in different pages, where 1 inherits the other. The behavior looks obvious to me but IDK if there is also a way to handle some nested case like that from within the script.

allejo commented 4 years ago

Just tested the hotfix and it fixed the tag issue.

:tada:

I wonder if there is a need to document that specific case BTW, where the script works 2 times (adding 2 heading anchors) when called by 2 layouts used in different pages, where 1 inherits the other. The behavior looks obvious to me but IDK if there is also a way to handle some nested case like that from within the script.

Documenting this behavior wouldn't hurt; https://github.com/allejo/jekyll-anchor-headings/pull/14/commits/e01729ab3ac7876839e8411ce3781c6c056d9b09.

As for adding support for nesting, I think it would be limited; I could check to see if there's already an anchor in the heading and skip adding a new one if there is. I'm just curious to know if there's a use case for this behavior where this script would legitimately be used twice.

redtide commented 4 years ago

As for adding support for nesting, I think it would be limited; I could check to see if there's already an anchor in the heading and skip adding a new one if there is. I'm just curious to know if there's a use case for this behavior where this script would legitimately be used twice.

That is what happens in the sfzformat website; one layout is the default for all pages without particular requirements, the other is for some pages that uses both default plus some specific additions (tables and some other info generated from a yml file) added on it. The double heading anchor can be seen in the 'Example' heading on one of the pages referred in the home in the test branch. That's why I used that workaround, to use the script only once.

allejo commented 4 years ago

Does using the script only in the default layout not suffice? I thought a child layout's content would be passed up to its parent and if its parent is in charge of the anchors, then it'd work. If that's not the case, I can add the check.

redtide commented 4 years ago

You are right, it seems I was confused by the other issue and I haven't tried the obvious logic.

redtide commented 4 years ago

Updated, thank you for your help!

allejo commented 4 years ago

Sure thing! Thanks a lot for bringing this issue to my attention :+1: