Code Highlighting Unreadable

[CodeLenny]

I tried to include a code snippet, but found that the colors chosen for highlighting render strings nearly invisible.

And it's even harder to see at a normal resolution:

Would it be possible to change the color scheme to make snippets more readable? It looks like the grey background is assigned by foundation to all code blocks, but hljs is using a monoki theme for the foreground colors.

The code block is .lang-js. Would it be possible to select code[class^="lang-"], code[class*=" lang-"] and set the background color to a monoki background, or have some other fix?

<div class="doc-copy">
                <section class="swagger-operation-description"> <pre><code class="lang-js">echo(<span class="hljs-string">"Hello World"</span>);
</code></pre>
                  <!-- text removed -->
                    </section>
              </div>

Thanks!

[CodeLenny]

In addition, I just noticed that multi-line snippets aren't highlighted in a continuous block.

[auscaster]

Thanks for the bug report, I'm current not using code blocks in descriptions so haven't come across this issue yet. I'll try to find a solution when time permits, or would welcome a PR in the mean time.

[CodeLenny]

Thanks! Would you have a more elegant solution than a new CSS rule that selects code[class^="lang-"], code[class*=" lang-"] and sets the background to Monokai's background? I could PR that CSS rule.

[auscaster]

That would be an option, but I would prefer the flexibility of a CLI option so you could specify the highlight theme at compile time, such as c=monokai or similar. Also, a couple of CSS tweaks to stop foundation getting in the way might also be necessary. Feel free to have a go, or I'll try to find some time during the week.

[CodeLenny]

Foundation is currently highlighting all <code> snippets in the light grey color, instead of using the code block theme. Would you like to keep that, or change all <code> snippets to be using the code highlighter's style? (my ugly lang- selector is only needed to select code snippets that have syntax highlighting, universally highlighting all blocks would be nice and simple)

I'm happy to add Monokai's background to the Monokai file and tweak Foundation's CSS, but might leave the CLI option to choose a highlighting style for you.

[auscaster]

Sounds greeat, would welcome the PR :)

[auscaster]

Just to clarify though, only code blocks inside pre tags should be hilighted - inline code blocks should retain simple grey markup

[CodeLenny]

@auscaster Here's what I'm seeing:

1. Inline code ("Hello World, this is a `test`.") produce

<p>Hello World, this is a <code>test</code>.</p>

2. Code blocks without a language

\```
test
\```

Produce

<pre><code><span class="hljs-built_in">test</span>
</code></pre>

3. Code blocks with a language

\```js
echo("Hello World");
\```

Produce

<pre><code class="lang-js">echo(<span class="hljs-string">"Hello World"</span>);
</code></pre>

So the CSS selector I proposed only targets multi-line code blocks that have selected a language for highlighting (case 3). All others will default to the standard Foundation styling.

Sounds good?

[CodeLenny]

Oh, I just read through the Monoki docs, and found that there's already the following rule:

.hljs {
  display: block;
  overflow-x: auto;
  padding: 0.5em;
  background: #23241f;
}

Looks like the hljs class isn't getting set. Should the templates be modified to insert .hljs, or should the CSS be changed to match the existing code?

[auscaster]

Hi Ryan, my vote would be to add the .hljs class so we can keep the CSS files standard

…

On 9 January 2017 at 16:01, Ryan Leonard ***@***.***> wrote: Oh, I just read through the Monoki docs, and realized that there's already the following rule: .hljs { display: block; overflow-x: auto; padding: 0.5em; background: #23241f; } Looks like the hljs class isn't getting set. Should the templates be modified to insert .hljs, or should the CSS be changed to match the existing code? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGKDAvYn98MAqp5wSmhsUV4BTVTp-J0ks5rQkuygaJpZM4LdHxA> .

[CodeLenny]

@auscaster I'm less familiar with your JavaScript setup than quickly tweaking the CSS files. It might be easier for you to do it yourself, but I'm happy to see what I can do if I'm pointed in the right direction.

It looks like HighlightJS isn't inserting the hljs class. Should that be inserted into the output?

I found the following methods in common.js which look applicable. I would like to confirm that these should be edited before continuing, to make sure the changes meet your requirements.

var cheerio = require('cheerio');
var marked = require('marked');
var highlight = require('highlight.js');

var common = {
  highlight: function(code, name) {
      var highlighted;
      if (name) {
          highlighted = highlight.highlight(name, code).value;
      } else {
          highlighted = highlight.highlightAuto(code).value;
      }
      return highlight.fixMarkup(highlighted); //highlighted; //
  },

  /**
   * Render a markdown formatted text as HTML.
   * @param {string} `value` the markdown-formatted text
   * @param {boolean} `stripParagraph` the marked-md-renderer wraps generated HTML in a <p>-tag by default.
   *      If this options is set to true, the <p>-tag is stripped.
   * @returns {string} the markdown rendered as HTML.
   */
  markdown: function(value, stripParagraph) {
     if (!value) {
         return value;
     }
     var html = marked(value);
     // We strip the surrounding <p>-tag, if
     if (stripParagraph) {
         var $ = cheerio("<root>" + html + "</root>");
         // Only strip <p>-tags and only if there is just one of them.
         if ($.children().length === 1 && $.children('p').length === 1) {
             html = $.children('p').html();
         }
     }
     return html;
  },

Thanks!

[CodeLenny]

Actually, after a little digging, I found someone else had added the hljs class by changing the Marked options. Would you like me to implement a similar change?

evancz/elm-markdown@ae1d137

[auscaster]

If it works as stated I'll be more than happy to accept the PR.

…

On 9 January 2017 at 18:31, Ryan Leonard ***@***.***> wrote: Actually, after a little digging, I found someone else had added the hljs class by changing the Marked options. Would you like me to implement a similar change? ***@***.*** <evancz/elm-markdown@ae1d137> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#26 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAGKDB7o0sNFoFxYyMQ95yocJg7Kgw7Yks5rQm7wgaJpZM4LdHxA> .

[CodeLenny]

@auscaster I've made the change and PRed. I've tested it on my API docs, and it works for me.