Skip to content
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

trailing CR in </pre> in: Renderer.prototype.code() #449

Closed
warren-bank opened this issue Jul 17, 2014 · 8 comments
Closed

trailing CR in </pre> in: Renderer.prototype.code() #449

warren-bank opened this issue Jul 17, 2014 · 8 comments

Comments

@warren-bank
Copy link

minor fix/suggestion:

  • dev (lib/marked.js):
    search = '\n</code></pre>

    replace = '</code></pre>

  • prod (marked.min.js):
    search = "\n</code></pre>

    replace = "</code></pre>

  • notes:

    • this occurs twice.
    • it has to do with how the html for code blocks are built.
    • the trailing CR is inside a <pre>, so it adds visible white-space.
@Feder1co5oave
Copy link
Contributor

In my experience, a single newline at the end of the <pre> does not create additional space in the browser. Could you post an example?

@warren-bank
Copy link
Author

I wrote some tests to back-track my code and determine exactly where this problem was introduced. It is partially the result of my doing something "weird".

Within the "highlight" callback function to pre-process code blocks.. I was conditionally passing the code to highlight.js ..which rewrites the HTML text.. adding spans and classes and such. However, the one issue I found was that all of the color schemes (css themes) assume that the parent container (ie: the <code> tag) has the css class hljs. Since there's no direct access to this element from within the "highlight" callback function, I opted to wrap the output of the highlighter method in a <div class="hljs">, which results in the dom structure: <pre><code><div>...stuff...</div>\n</code></pre>.

So, it's my own fault for adding a block element.. which causes the trailing CR to occur on a new line.. creating the visible space I was seeing.

I suppose this is an edge case.. up to you if it's worth considering. I'm fairly new to github, so I'm not sure if I can attach an .html file containing some test code that very clearly illustrates the issue I'm seeing; I don't seem to be able to do so.. shame.

I apologize if this was a nuisance post. I do appreciate the fast response.

I've tried to copy the contents of the test file into this message; the only edit I made was to "escape" the code blocks in the embedded markdown. test.html file contents:

<html>
<head>

<link rel="stylesheet" href="http://cdnjs.cloudflare.com/ajax/libs/highlight.js/8.1/styles/default.min.css">
<script src="http://cdnjs.cloudflare.com/ajax/libs/highlight.js/8.1/highlight.min.js"></script>

<link rel="stylesheet" type="text/css" href="https://assets-cdn.github.com/assets/github2-84a1b6179d461213455892ab983182bc2052a7b5.css" />
<link rel="stylesheet" type="text/css" href="https://assets-cdn.github.com/assets/github-c534ad575b5bb8c3cc3dce9c571df7aa7400dbe9.css" />
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.1/jquery.min.js"></script>
<script type="text/javascript" src="https://cdn.rawgit.com/chjj/marked/master/marked.min.js"></script>
<script type="text/javascript">
window.onload = function(){
    var get_html_string = function(add_hljs_div_wrapper){
        var html_string = marked(gfmd_string, {
            "highlight": function(code, lang){
                var hljs_code = (hljs.highlight(lang, code, false)).value;

                if (add_hljs_div_wrapper){
                    hljs_code = '<div class="hljs">' + hljs_code + '</div>';
                }

                return hljs_code;
            }
        });
        return html_string;
    };

    var run_test = function(add_hljs_div_wrapper){
        var html_string = get_html_string(add_hljs_div_wrapper);

        jQuery('body').append('<hr>scenario:<br>&nbsp;&nbsp;&nbsp;&nbsp;additional wrapper &lt;div&gt; = ' + (add_hljs_div_wrapper).toString() + '<br>raw html output:<hr><textarea style="width:100%;height:10em;">' +html_string+ '</textarea>');
        jQuery('body').append('<hr>DOM created by jQuery:<hr><div class="markdown-body">' +html_string+ '</div>');
    };

    var gfmd_string = jQuery('#gfmd_text').val();

    run_test(false);
    run_test(true);
};
</script>
</head>
<body>
    <div style="display:none;">
        <textarea id="gfmd_text">
## block #01: javascript

[```]javascript
var hello = "world";
var hello = "world";
[```]

## block #02: perl

[```]perl
$hello='world';
$hello='world';
[```]
        </textarea>
    </div>
</body>
</html>

@Feder1co5oave
Copy link
Contributor

I believe the newline is there for mere sake of readability of the generated html code. Let's hear if @chjj would like to give up a bit of readability in favor of better compatibility.

@ghost
Copy link

ghost commented Sep 14, 2016

An extra newline before the closing </code> tag does add space in the browser. Here is an example:

My example below is incorrect. The newline marked adds does not add space in the browser. I'm leaving my mistaken example and comment here only for history.

<html>
  <style>pre { background-color: #eee; border: 1px solid blue; }</style>
  <body>
    <pre><code>Hello
    </code></pre>

    <pre><code>Hello</code></pre>
  </body>
</html>

pre-newline

The screenshot is from Chrome 52 on Mac. I see the same behavior in Safari 9.1.1.

I agree with the originally proposed suggestion: Remove the \n before </code></pre> in the built-in renderer.

@Feder1co5oave
Copy link
Contributor

<html>
  <style>pre { background-color: #eee; border: 1px solid blue; }</style>
  <body>
    <pre><code>Hello
    </code></pre>   <==== THIS

    <pre><code>Hello</code></pre>
  </body>
</html>

That's not just a trailing newline. That's a newline plus 4 blanks.
My chrome 52 on Linux still ignores a simple trailing newline in a <pre><code> block.
The default marked renderer does not add blanks to indent html. If you generated this with a custom renderer I think it is your responsibility.

@ghost
Copy link

ghost commented Sep 14, 2016

So sorry, my mistake. I had generated the HTML by hand and forgot to remove the indenting.

It turns out the real issue is with a separate documentation tool we are using which changes the HTML that marked generates. The tool is adding a newline between the closing </pre> and </code> tags. There's nothing marked can do about this.

Thanks for the fast feedback.

@Feder1co5oave
Copy link
Contributor

No problem!

@joshbruce
Copy link
Member

#983

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants