Skip to content
This repository was archived by the owner on Feb 24, 2021. It is now read-only.

Using markdown-it-attrs with code fences#33

Closed
simondean wants to merge 2 commits intoMermade:masterfrom
simondean:attrs
Closed

Using markdown-it-attrs with code fences#33
simondean wants to merge 2 commits intoMermade:masterfrom
simondean:attrs

Conversation

@simondean
Copy link

@simondean simondean commented May 16, 2018

This PR is related to issue #30

@simondean
Copy link
Author

Hi @MikeRalphson. I was wondering whether this PR would be suitable for including in shins. It provides an implementation/fix for #30.

The approach the PR takes is based on the following couple of comments:

The code I've implemented in the fence method of the PR is based on markdown-it's fence method in https://github.com/markdown-it/markdown-it/blob/master/lib/renderer.js#L39.

Let me know if there's any info I can add. Thanks

@MikeRalphson
Copy link
Contributor

Looks good. I just need to find the time to test it doesn't affect the existing output. Thanks for your patience.

@simondean
Copy link
Author

No worries. Thanks @MikeRalphson

@MikeRalphson
Copy link
Contributor

When comparing the before and after output, I see each of the code blocks differs as per this example:

-<pre class="highlight tab tab-shell"><code><span class="hljs-comment"># You can also use wget</span>
+<pre><code class="highlight tab tab-shell"><span class="hljs-comment"># You can also use wget</span>

Could the class attributes be moved back to the <pre> element, as some people may be depending on that?

@ribrewguy
Copy link

I've tried this update, but it doesn't seem to add the style. The linked markdown issue suggests that adding the attributes should be in the form:

    ```http {.some-class}
    GET /foo/bar HTTP/1.1
    ...
    ```

But that doesn't seem to do anything in the generated code block. I would expect that class="some-class" would be added to the pre or code element.

@MikeRalphson
Copy link
Contributor

Please re-raise over on slant if still an issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants