8
\$\begingroup\$

I've been working on implementing Google Code Prettify and finally got it to work correctly. You can see my test file here to see what it looks like. I worked from a template and tried to improve it.

There is a lot of HTML boilerplate code that apparently is needed (mostly JavaScript links). I tried to remove some of them to reduce clutter but it broke every time, so I left it all in from the template. For reference:

prettify-test.html

<!doctype html>
<!-- paulirish.com/2008/conditional-stylesheets-vs-css-hacks-answer-neither/ -->
<!--[if lt IE 7]> <html class="no-js lt-ie9 lt-ie8 lt-ie7" lang="en"> <![endif]-->
<!--[if IE 7]>    <html class="no-js lt-ie9 lt-ie8" lang="en"> <![endif]-->
<!--[if IE 8]>    <html class="no-js lt-ie9" lang="en"> <![endif]-->
<!--[if gt IE 8]><!--> <html class="no-js" lang="en"> <!--<![endif]-->
<head>
  <meta charset="utf-8">
  <link rel="dns-prefetch" href="//ajax.googleapis.com" />

  <title>Google Prettify testing</title>

  <meta name="viewport" content="width=device-width">
  <link rel="stylesheet" href="http://fonts.googleapis.com/css?family=Open+Sans:400,700">
  <link rel="stylesheet" href="color-themes-for-google-code-prettify-gh-pages/css/style.css">

  <link rel="stylesheet" href="color-themes-for-google-code-prettify-gh-pages/css/themes/github.css">

  <link rel="sitemap" type="application/xml" title="Sitemap" href="color-themes-for-google-code-prettify/sitemap.xml" />
  <link rel="canonical" href="color-themes-for-google-code-prettify-gh-pages/github/" />

  <script src="color-themes-for-google-code-prettify-gh-pages/js/modernizr-2.5.3.min.js"></script>

</head>
<body style="margin-left: 15px;">
<h2>Google Prettify Testing: Github default</h2>

<h3>Color Reference</h3>
    <code class="pln ">Plain text</code><br /> 
    <code class="str">"String content"</code><br />
    <code class="kwd">Keyword</code><br />
    <code class="com">// Comment</code><br />
    <code class="typ">Type name</code><br />
    <code class="lit">Literal value 42</code><br />
    <code class="pun">Punctuation...</code><br />
    <code class="dec">Declaration</code><br />
    <code class="var">Variable name</code><br />
    <code class="fun">Function name</code><br />
    <br />
    <code class="opn">Lisp {open bracket</code><br />
    <code class="clo">Lisp close} bracket</code><br />
    <br />
    <code class="tag">&lt;Markup tag name&gt;</code><br />
    <code class="atn">&lt;Markup attribute-name=""&gt;</code><br />
    <code class="atv">&lt;Markup attribute="value"&gt;</code><br />
    <br />

<h3>C#/LINQ</h3>

<pre class="prettyprint linenums lang-csharp lang-linq">
// this is a comment
public static class Evaluate
{
    public static string FizzBuzz(int start, int end)
    {
        return Enumerable.Range(start, (end - start) + 1)
            .Select(FizzOrBuzz)
            .Aggregate(String.Empty, (y, x) =&gt; String.Format("{0} {1}", y, x))
            .Trim();
    }

    public static string FizzOrBuzz(int n)
    {
        if (n % 15 == 0) return "fizzbuzz";
        if (n % 3 == 0) return "fizz";
        if (n % 5 == 0) return "buzz";
        return n.ToString();
    }
}
</pre>


<!-- more code examples removed -->

  <footer></footer>
  <script src="//platform.twitter.com/widgets.js"></script>
  <script src="//assets.pinterest.com/js/pinit.js"></script>
  <script src="//ajax.googleapis.com/ajax/libs/jquery/1.7.2/jquery.min.js"></script>
  <script>window.jQuery || document.write('<script src="/js/jquery-1.7.2.min.js"><\/script>')</script>

  <script src="color-themes-for-google-code-prettify-gh-pages/js/prettify.js"></script>
  <script src="color-themes-for-google-code-prettify-gh-pages/js/lang-css.js"></script>

  <script src="color-themes-for-google-code-prettify-gh-pages/js/jquery.smooth-scroll.min.js"></script>
  <script src="color-themes-for-google-code-prettify-gh-pages/js/script.min.js"></script>

</body>
</html>

I would like mostly the CSS reviewed, although I am open for suggestions on HTML as well. I tried to make the template in a way that is simple to use. Please do note I have no control at all over the CSS class names (like .str, .kwd, etc.) those are what Prettify uses and if you change them it breaks everything, unless you go and change all the JavaScript files that reference those classes.

PS: There appear to be a few merge conflicts in the linked file. Please disregard them. I removed them in the code block below. Conflicts have been resolved.

color-themes-for-google-code-prettify-gh-pages/css/themes/github.css

/* GitHub Theme */
.prettyprint {
  background: white;
  font-family: Monaco, Consolas, Courier New, monospace;
  font-size: 12px;
  line-height: 1.5;
  border: 1px solid #ccc;
  padding: 10px;
}

code {
  font-family: Monaco, Consolas, Courier New, monospace;
  font-size:12px;
}

/* Plain text */
.pln {
  color: #333333;
}

/* String content */
  .str {
    color: #dd1144;
  }
/* Keyword */
  .kwd {
    color: #333333;
  }
/* Comment */
  .com {
    color: #999988;
  }
/* Type name */
  .typ {
    color: #445588;
  }
/* Literal value */
  .lit {
    color: #445588;
  }
/* Punctuation */
  .pun {
    color: #333333;
  }
/* Lisp open bracket */
  .opn {
    color: #333333;
  }
/* Lisp close bracket */
  .clo {
    color: #333333;
  }
/* Mark-up tag name */
  .tag {
    color: navy;
  }
/* Mark-up attribute name */
  .atn {
    color: teal;
  }
/* Markup attribute value */
  .atv {
    color: #dd1144;
  }
/* Declaration */
  .dec {
    color: #333333;
  }
/* Variable name */
  .var {
    color: teal;
  }
/* Function name */
  .fun {
    color: #990000;
  }
}
@media print, projection {
  .str {
    color: #006600;
  }

  .kwd {
    color: #006;
    font-weight: bold;
  }

  .com {
    color: #600;
    font-style: italic;
  }

  .typ {
    color: #404;
    font-weight: bold;
  }

  .lit {
    color: #004444;
  }

  .pun, .opn, .clo {
    color: #444400;
  }

  .tag {
    color: #006;
    font-weight: bold;
  }

  .atn {
    color: #440044;
  }

  .atv {
    color: #006600;
  }
}
/* Specify class=linenums on a pre to get line numbering */
ol.linenums {
  margin-top: 0;
  margin-bottom: 0;
}

/* IE indents via margin-left */
li.L0,
li.L1,
li.L2,
li.L3,
li.L4,
li.L5,
li.L6,
li.L7,
li.L8,
li.L9 {
  /* */
}

/* Alternate shading for lines */
li.L1,
li.L3,
li.L5,
li.L7,
li.L9 {
  /* */
}
\$\endgroup\$
4
\$\begingroup\$

For your li elements, unless you need to support IE8, it would be more elegant to use nth-child to do your alternating line styles:

ul.linenums li:nth-child(odd) {
    /* styles here */
}

It would be more elegant to use descendant selectors than to spell out each class individually:

ul.linenums li {
    /* styles here */
}

Also, the contrast is rather poor for your .typ class. To me, it is just barely distinguishable from the surrounding black text.

\$\endgroup\$
  • \$\begingroup\$ Yeah I plan on improving the coloring itself, this just uses a simile of the Github code display settings. Good suggestions on the linenums I will try that! \$\endgroup\$ – Phrancis Jan 27 '15 at 19:26
3
\$\begingroup\$

Running your CSS through the W3C CSS validator, I found three errors:

  1. You need to put font names that include spaces in quotations: "Courier New" (this error occurred twice).
  2. You have an extra brace here:
/* Function name */
  .fun {
    color: #990000;
  }
}

Other than this, I think it looks fine, other than the occasional empty comment.

Your HTML validates just fine at the HTML validator.

\$\endgroup\$
  • \$\begingroup\$ Good catch on that extra brace, I forgot to include @media screen { at the top of all those classes, which is what that brace was supposed to close! \$\endgroup\$ – Phrancis Jan 27 '15 at 17:32
2
\$\begingroup\$

on Code like this

/* IE indents via margin-left */
li.L0,
li.L1,
li.L2,
li.L3,
li.L4,
li.L5,
li.L6,
li.L7,
li.L8,
li.L9 {
  /* */
}

/* Alternate shading for lines */
li.L1,
li.L3,
li.L5,
li.L7,
li.L9 {
  /* */
}

I normally see it being presented like

/* IE indents via margin-left */
li.L0, li.L1, li.L2, li.L3, li.L4, li.L5, li.L6, li.L7, li.L8, li.L9 {
  /* */
}

/* Alternate shading for lines */
li.L1, li.L3, li.L5, li.L7, li.L9 {
  /* */
}

As far as this goes it is not very DRY. This just doesn't look right to me, I would think that the Javascript would handle creating the lines and giving them an alternating class, so that the content can expand without you having to add more li.L## to this CSS file.

I haven't looked at the code long enough, but I think there might be a better way of doing this with the JavaScript and a simple Class Name.

\$\endgroup\$
  • 1
    \$\begingroup\$ AFAIK it only needs L0 through L9 then it just repeats for additional lines, idem. for alternate shading for lines. If you look at the VBA example for instance, it has 41 lines, and so if you set an alternate shading it would keep shading every 2nd line all the way through. \$\endgroup\$ – Phrancis Jan 27 '15 at 17:51
2
\$\begingroup\$

Other reviewers have already covered the CSS well.

As for this HTML:

<h3>Color Reference</h3>
    <code class="pln ">Plain text</code><br /> 
    <code class="str">"String content"</code><br />
    <code class="kwd">Keyword</code><br />

I think <br/> is not a good practice in general. I recommend to get rid of those, and do something like this instead:

<h3>Color Reference</h3>
<ul class="examples">
    <li><code class="pln ">Plain text</code></li>
    <li><code class="str">"String content"</code></li>
    <li><code class="kwd">Keyword</code></li>
</ul>

In the <head> of your tester page (decidedly not in the prettify CSS file), add custom <style> to format examples.p appropriately.

\$\endgroup\$
  • \$\begingroup\$ Those aren't paragraphs though, that just looks like a misuse of the paragraph element to me. \$\endgroup\$ – cimmanon Jan 27 '15 at 19:22
  • \$\begingroup\$ <p> is not a bad idea, since <code> is meant to be for inline code (as part of a paragraph). Most times though it will be used with <pre> code blocks. \$\endgroup\$ – Phrancis Jan 27 '15 at 19:28
  • \$\begingroup\$ No. If anything, an unordered list would be most appropriate. \$\endgroup\$ – cimmanon Jan 27 '15 at 19:32
  • \$\begingroup\$ @cimmanon ah, good call! I updated, thanks for the tip! \$\endgroup\$ – janos Jan 27 '15 at 19:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.