Syntax highlighter cleanup #1352
Conversation
haarg
force-pushed
the
syntax-highlighter-cleanup
branch
4 times, most recently
from
bc03eef to
15ccbc8
Compare
|
I've added some more to this PR, including highlighting multiple lines, re-highlighting when you click on lines, and icons for the source view sidebar. |
| @@ -55,21 +55,10 @@ | |||
|
|
|||
| <div class="content"> | |||
| <% IF !module.binary %> | |||
| <pre class="brush: <% filetype %>; class-name: 'highlight'; toolbar: false;" id="source"><% source %></pre> | |||
| <pre id="source" class="line-numbers pod-toggle<% IF module.sloc > 0; " pod-hidden"; END %>" data-pod-lines="<% | |||
| module.pod_lines.map(->(l){ l.0+1 _ "-" _ (l.0+l.1) }).join(', ') | |||
There was a problem hiding this comment.
Would you mind using a different variable name in the map?
It can be hard to tell the difference between the lowercase L and the 1...
and with the template syntax for array subscripting...
when I first saw this line I thought it said 1.0+1.1 and wondered why...
|
I'm glad you moved the stuff to a separate file... I was thinking about doing that last night :-) |
haarg
force-pushed
the
syntax-highlighter-cleanup
branch
from
0d0575d to
d91656a
Compare
|
Rebased, fixing the variable name in the template. |
haarg
force-pushed
the
syntax-highlighter-cleanup
branch
2 times, most recently
from
ed22a1e to
c769426
Compare
| @@ -109,7 +109,7 @@ sub view : Private { | |||
| br => [], | |||
| caption => [], | |||
| center => [], | |||
| code => [], | |||
There was a problem hiding this comment.
Could you comment on what values might be coming through this attribute and from where?
I don't see why we should allow the class attribute to come through from pod for any tag.
|
@haarg This stuff is great, thanks! |
|
I've added some comments and done some minor refactoring to reduce duplication. The basic idea with the data- attributes is that they can be provided in the markup. We set some on the source page and some using javascript, but they could also be included in the markup returned by the API for Pod documents. That's why they are allowed by the HTML filter. This means the Pod can include syntax highlighting hints, like requested in #605. I've written a Pod::Simple extension we'll be able to use to provide this, Pod::Simple::XHTML::WithHighlightConfig. |
haarg
force-pushed
the
syntax-highlighter-cleanup
branch
from
7f82378 to
70c2bf2
Compare
|
Also, the particular attributes/classes I'm using are based on Prism, an alternate syntax highlighter I'm considering trying out, which is smaller and hopefully has less bugs. Even if we don't change the syntax highlighter, I think it's a reasonable way to mark up the data. Much better than the syntax highlighter's built in method that abuses the class attribute using a braced comma separated format. |
haarg
force-pushed
the
syntax-highlighter-cleanup
branch
2 times, most recently
from
4ba6401 to
307b3b9
Compare
This makes the classes used for highlighting match up with the HTML5 recommendations, and removes the use of the ugly braced config stuffed into the class attribute. It also opens up the option of the pod->html converter providing syntax highlighting configuration in a reasonable format.
Instead of applying styles to syntax highlighter blocks and trying to match verbatim sections to that, just use the normal <pre> styles. This simplifies the rules needed, and removes the need to extend the syntax highlighter's css rules.
haarg
force-pushed
the
syntax-highlighter-cleanup
branch
from
307b3b9 to
d3d4d21
Compare
|
totally awesome. |
|
Rebased and merged as 7c1d100 |
This cleans up the syntax highlighter markup and styles.
Rather than using the syntax highlighter's built in configuration mechanism that abuses the class attribute, use distinct attributes and proper classes to configure it in the markup. Then read that information in javascript and feed it to the syntax highlighter. The new markup matches up with the recommendations in the HTML5 spec. It also allows the API to provide syntax highlighting configuration, which is the first half of implementing #605.
Also, stop trying to apply custom styles to the syntax highlighter for the border and background, etc. Instead, reset the syntax highlighter itself to inherit from parent attributes, and wrap it in a pre tag to get the borders, background, fonts, etc.