Token stream must stay immutable after renderer call

[puzrin]

See details in #259

[cbreeden]

I took the sample markdown file on the homepage to see which plugins may also be mutating the token stream and found only two:

• markdown-it-container
• markdown-it-named-headers

Markdown-it-container, after two passes, yields:

<div class="warning" class="warning">

Markdown-it-named-headers, after two passes, yields:

<h2 id="h2-heading" id="h2-heading">h2 Heading</h2>

Just with ever header element. I'm curious how they implement this. Maybe there is a way to fix their as well, though it's probably best to place a issue request on their respective repos.

One last thing, I added the code:

        it("2Pass: " + fixture.header && options.head ? fixture.header : 'line ' + (fixture.first.range[0] - 1), function () {
          var tokens = md.parse(fixture.first.text, md.options);
          var render1 = md.renderer.render(tokens, md.options, {});
          var render2 = md.renderer.render(tokens, md.options, {});
          options.assert.strictEqual(render1, render2);
        });

to markdown-it-testgen here and found that everything passes except for block-fences.

[puzrin]

• markdown-it-container is our plugin, but it's renderer function is for demo only. I think, no changes in plugin needed
• markdown-it-named-headers is not our plugin (ask author to fix)

Can this issue be closed?

[cbreeden]

I haven't tested it but I trust you got it right, I can let you know if I run into troubles again.

[puzrin]

Released 6.1.0 with fix. Closing now. Feel free to create new issue if any problems still exists.

[puzrin]

I've looked at markdown-it-container. Since you don't need something super-flexible & extendable, override renderer with fixed opening tag content. Default renderer is really stupid, just to demonstrate feature. There was no hope that it can satiasfy anyone without change, but it could not be empty to make plugin work somehow without setup.

About markdown-it-named-headers - ask author to move attr logic into core chain. Modifying content in renderer is a useful cheat to make life easy. But solid implementation need more correct approach. Also see #28, why we still did not implemented this feature in core. May be you need to know about possible security problems.

Probably, we need to invent better solution to extend attrs output in render without mutation, but no good ideas right now. Cloning works, but is not effective. May be second optional param in attr renderer... need to think...

[cbreeden]

I remember reading #28 awhile back. It was a good post to read to understand the complexities involved with named headers in markdown. I believe that the HTML5 <section> was supposed to help web developers solve these kinds of problems with tag collisions, down-stepping h1's to h2's or h3's as necessary when necessary and stuff like that, but I don't think such a construct belongs in Markdown. Also the issue of getting the same rendering for a given input if you go the UID route. It's probably best to keep it as it is, and allow users create the plugins for their needs.

I took another look at how vscode is using markdown-it in their recent merge, and I came to realize that there seems to be no use whatsoever of the named headers. Certainly some users may want to compile their markdown with named headers, but this probably isn't something that belongs in the preview mode. I suspect that there was an idea of creating links for the headers so that clicking the link will send you back to the source (which could be a useful feature), but the correct route would be to use the source maps similar to what you did to sync the scrolls.