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

Nested <pre> tags cause output errors #71

Open
williamcanin opened this Issue Jan 15, 2016 · 14 comments

Comments

Projects
None yet
4 participants
@williamcanin

williamcanin commented Jan 15, 2016

First, congratulations for the plugin.
Next, when using "linenos" the highlight ...
Example:
{% highlight ruby linenos %}
... my block that is between the "highlight" is not compressed properly in html.

The question is whether there is configuration to work around this problem or if this is a weak point in the plugin.

Thanks

@jnvsor

This comment has been minimized.

Show comment
Hide comment
@jnvsor

jnvsor Jan 15, 2016

Collaborator

Could you provide the uncompressed output HTML?

Most likely it's because highlighting is wrapped in a <pre> tag - the whole point is that it preserves whitespace in code and the like (Otherwise everything in your block would be on the same line and with no indentation, just how html works)

Collaborator

jnvsor commented Jan 15, 2016

Could you provide the uncompressed output HTML?

Most likely it's because highlighting is wrapped in a <pre> tag - the whole point is that it preserves whitespace in code and the like (Otherwise everything in your block would be on the same line and with no indentation, just how html works)

@williamcanin

This comment has been minimized.

Show comment
Hide comment
@williamcanin

williamcanin Jan 15, 2016

I auditioned with a simple Jekyll project:
$ jekyll new project
Just put the "linenos" on the "highlight"

williamcanin commented Jan 15, 2016

I auditioned with a simple Jekyll project:
$ jekyll new project
Just put the "linenos" on the "highlight"

@jnvsor

This comment has been minimized.

Show comment
Hide comment
@jnvsor

jnvsor Jan 15, 2016

Collaborator

I can confirm this is because of the <pre> tag.

HTML collapses whitespace by default, the <pre> tag is there to preserve whitespace in the source - commonly used (As in the case of highlight) to keep newlines and indentation in the end result.

For reference, here's what it would look like if we collapsed that part like you asked:

You can read more about pre and white-space on MDN.

Collaborator

jnvsor commented Jan 15, 2016

I can confirm this is because of the <pre> tag.

HTML collapses whitespace by default, the <pre> tag is there to preserve whitespace in the source - commonly used (As in the case of highlight) to keep newlines and indentation in the end result.

For reference, here's what it would look like if we collapsed that part like you asked:

You can read more about pre and white-space on MDN.

@jnvsor jnvsor closed this Jan 15, 2016

@jnvsor jnvsor added invalid bug and removed invalid labels Jan 15, 2016

@jnvsor

This comment has been minimized.

Show comment
Hide comment
@jnvsor

jnvsor Jan 15, 2016

Collaborator

Ah, it seems I misunderstood - when doing a "Double highlight" like that you get nested pre tags - that's something we haven't implemented (Because I never thought I'd actually see them in the wild)

It seems more like a bug in the highlighter but I'll take a look

Collaborator

jnvsor commented Jan 15, 2016

Ah, it seems I misunderstood - when doing a "Double highlight" like that you get nested pre tags - that's something we haven't implemented (Because I never thought I'd actually see them in the wild)

It seems more like a bug in the highlighter but I'll take a look

@jnvsor jnvsor reopened this Jan 15, 2016

@jnvsor jnvsor changed the title from Highlight with linenos contains error. to Nested <pre> tags cause output errors Jan 15, 2016

@williamcanin

This comment has been minimized.

Show comment
Hide comment
@williamcanin

williamcanin Jan 15, 2016

Exactly, that's it. thank you

williamcanin commented Jan 15, 2016

Exactly, that's it. thank you

@jnvsor

This comment has been minimized.

Show comment
Hide comment
@jnvsor

jnvsor Jan 15, 2016

Collaborator

@williamcanin I know from experience that getting nested anything to work in liquid is really really hard so I wouldn't expect this to get done any time soon if I were you.

Instead it might be more productive to tweak Jekyll itself at tags/highlight.rb:116 to remove the outer <pre> tag then style away the difference:

"<figure class=\"highlight\"><code #{code_attributes}>#{code.chomp}</code></figure>"
Collaborator

jnvsor commented Jan 15, 2016

@williamcanin I know from experience that getting nested anything to work in liquid is really really hard so I wouldn't expect this to get done any time soon if I were you.

Instead it might be more productive to tweak Jekyll itself at tags/highlight.rb:116 to remove the outer <pre> tag then style away the difference:

"<figure class=\"highlight\"><code #{code_attributes}>#{code.chomp}</code></figure>"
@williamcanin

This comment has been minimized.

Show comment
Hide comment
@williamcanin

williamcanin Jan 15, 2016

Thanks for the sugestion. I will do that.

williamcanin commented Jan 15, 2016

Thanks for the sugestion. I will do that.

@DeXP

This comment has been minimized.

Show comment
Hide comment
@DeXP

DeXP Feb 22, 2016

I have the same issue with rouge highlighter, the "linenos" breaks html output. Yep, rouge doubles "pre" tags. But it is the only way now on github's servers...

DeXP commented Feb 22, 2016

I have the same issue with rouge highlighter, the "linenos" breaks html output. Yep, rouge doubles "pre" tags. But it is the only way now on github's servers...

@DeXP

This comment has been minimized.

Show comment
Hide comment
@DeXP

DeXP Feb 24, 2016

Workaround to avoid this bug. Initially you have the code like:

{% highlight AnyLanguage linenos %}
Some code
{% endhighlight %}

Change it to:

{% capture _code %}{% highlight AnyLanguage linenos %}
Some code
{% endhighlight %}{% endcapture %}{% include fixlinenos.html %}{{ _code }}

_include/fixlinenos.html content:

{% if _code contains '<pre class="lineno">' %}
    {% assign _code = _code | replace: "<pre><code", "<code" %}
    {% assign _code = _code | replace: "</code></pre>", "</code>" %}
{% endif %}

DeXP commented Feb 24, 2016

Workaround to avoid this bug. Initially you have the code like:

{% highlight AnyLanguage linenos %}
Some code
{% endhighlight %}

Change it to:

{% capture _code %}{% highlight AnyLanguage linenos %}
Some code
{% endhighlight %}{% endcapture %}{% include fixlinenos.html %}{{ _code }}

_include/fixlinenos.html content:

{% if _code contains '<pre class="lineno">' %}
    {% assign _code = _code | replace: "<pre><code", "<code" %}
    {% assign _code = _code | replace: "</code></pre>", "</code>" %}
{% endif %}
@LDVSOFT

This comment has been minimized.

Show comment
Hide comment
@LDVSOFT

LDVSOFT Jun 14, 2016

So, will be any fixes in this project?

LDVSOFT commented Jun 14, 2016

So, will be any fixes in this project?

@jnvsor

This comment has been minimized.

Show comment
Hide comment
@jnvsor

jnvsor Jun 15, 2016

Collaborator

Probably not going to happen. Nested pre tags don't usually happen in manmade html, and using a depth counter would at least triple build times

Collaborator

jnvsor commented Jun 15, 2016

Probably not going to happen. Nested pre tags don't usually happen in manmade html, and using a depth counter would at least triple build times

@LDVSOFT

This comment has been minimized.

Show comment
Hide comment
@LDVSOFT

LDVSOFT Jun 15, 2016

@jnvsor OK, I have inserted hack by @DeXP to jekyll source (tags/highlight.rb), but to preserve some CSS rules working, I did this:

    rendered_output = add_code_tag(output)
    # FIX start: remove double pre
    if rendered_output.include? "<pre class=\"lineno\""
        rendered_output.gsub!("<pre><code ", "<div class=\"code-wrap\"><code ")
        rendered_output.gsub!("</code></pre>", "</code></div>")
    end
    # FIX end
    prefix + rendered_output + suffix

and them in every CSS file add .code-wrap selector to all pre selectors.

Works absolutely perfect.

LDVSOFT commented Jun 15, 2016

@jnvsor OK, I have inserted hack by @DeXP to jekyll source (tags/highlight.rb), but to preserve some CSS rules working, I did this:

    rendered_output = add_code_tag(output)
    # FIX start: remove double pre
    if rendered_output.include? "<pre class=\"lineno\""
        rendered_output.gsub!("<pre><code ", "<div class=\"code-wrap\"><code ")
        rendered_output.gsub!("</code></pre>", "</code></div>")
    end
    # FIX end
    prefix + rendered_output + suffix

and them in every CSS file add .code-wrap selector to all pre selectors.

Works absolutely perfect.

@jnvsor

This comment has been minimized.

Show comment
Hide comment
@jnvsor

jnvsor Jun 15, 2016

Collaborator

Great. Though if you're at the point where you're modifying jekyll source perhaps you'd be better with a dedicated build platform like Grunt or Gulp and a dedicated HTML minifier

Collaborator

jnvsor commented Jun 15, 2016

Great. Though if you're at the point where you're modifying jekyll source perhaps you'd be better with a dedicated build platform like Grunt or Gulp and a dedicated HTML minifier

@LDVSOFT

This comment has been minimized.

Show comment
Hide comment
@LDVSOFT

LDVSOFT Jun 15, 2016

@jnvsor No, I'm just too lazy to write any new code or use new tools, so editing locally installed Jekyll seems fine)

LDVSOFT commented Jun 15, 2016

@jnvsor No, I'm just too lazy to write any new code or use new tools, so editing locally installed Jekyll seems fine)

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