Skip to content
This repository

Rainbow doesn't respect nodes without "data-language" #104

Open
Nijikokun opened this Issue · 9 comments

2 participants

Nijiko Yonskai Craig Campbell
Nijiko Yonskai

It chooses all pre and code elements even if they are already highlighted and re-highlights or escapes the previously highlighted html.

I have a commit in line to fix this.

Craig Campbell
Owner

Rainbow should never re-highlight. It adds a class to each element when it is highlighted and if the element has that class it does not get processed again. Do you have an example?

Nijiko Yonskai

Here you go:

https://www.mashape.com/nijikokun/parsify#getting-started

Pre-highlighted by parsify gfm, and re-highlighted by rainbow.

It doesn't respect pre/code that only have "data-language" attribute, I think I may be wording this wrong. It doesn't respect pre/code elements that DONT have data-language attribute.

My code change was lines 671 - 689:

        var pre_blocks = node.querySelectorAll('pre[data-language]'),
            code_blocks = node.querySelectorAll('code[data-language]'),
            i,
            final_blocks = [];

        // @see http://stackoverflow.com/questions/2735067/how-to-convert-a-dom-node-list-to-an-array-in-javascript
        // we are going to process all <code> blocks
        for (i = 0; i < code_blocks.length; ++i) {
            final_blocks.push(code_blocks[i]);
        }

        // loop through the pre blocks to see which ones we should add
        for (i = 0; i < pre_blocks.length; ++i) {

            // if the pre block has no code blocks then process it directly
            if (!pre_blocks[i].querySelectorAll('code[data-language]').length) {
                final_blocks.push(pre_blocks[i]);
            }
        }
Craig Campbell
Owner

Oh, unless I am misunderstanding, that is by design. If you don't include the language attribute then it doesn't touch the code. You can also use class="lang-whatever" or class="language-whatever" if you want.

Nijiko Yonskai

I changed my to support data-language only, this way it doesn't conflict with other highlighters or static highlighting. Maybe a strict model so that way it doesn't conflict with others.

Plus, it's not stated on the site that it has this functionality so dropping support of it isn't much of a big deal.

Craig Campbell
Owner

Hmm, well querySelectorAll is not going to work on IE or other older browsers (not that rainbow works particularly well there either), but also this means that using a class will no longer work. I guess I am confused what the problem is.

If Rainbow finds a pre block without a language specified it should not do anything to it.

https://github.com/ccampbell/rainbow/blob/master/js/rainbow.js#L621

Nijiko Yonskai

The box has a language specified but by class, not data-language attribute, I'm saying since your examples on the site only signify that data-language is used, that you shouldn't do classes as well, this was a quick hack.

Craig Campbell
Owner

Well I didn't originally support it, but it was added in #34, and I think makes sense to support.

Nijiko Yonskai

I don't feel that is the right path to take, data attributes make more sense for checking which should be highlighted, and the class for which should be styled via CSS. Seperating logic from style.

This also conflicts with pre-styled code blocks.

Nijiko Yonskai

I will bring this issue up with W3C as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.