11ty / eleventy-plugin-syntaxhighlight

A pack of Eleventy plugins for syntax highlighting in Markdown, Liquid, and Nunjucks templates.
https://www.11ty.dev/docs/plugins/syntaxhighlight/
MIT License
130 stars 32 forks source link

Highlighting code with CRLF end-of-lines results in double line feeds in output when using Nunjucks #59

Closed rmcginty closed 2 years ago

rmcginty commented 3 years ago

This works fine in Markdown, but in Nunjucks, having \r\n at the end of a line results in two blank lines. Apparently Prism is outputting multiple \n instead of preserving the line ending which results in the Nunjucks template code adding an extra blank line for each line in the input.

Solution seems to be to remove all \r prior to highlighting. Submitting a PR.

JKC-Codes commented 3 years ago

I've managed to recreate this issue by using either alwaysWrapLineHighlights: true or lineSeparator: "\r\n" as options.

I don't think the issue is with Prism. I think the issue is with the way we're splitting lines here: https://github.com/11ty/eleventy-plugin-syntaxhighlight/blob/e992469c4fd3e910e1dab3865f0e23ee481ea8b9/src/HighlightPairedShortcode.js#L23-L32

By only splitting on '\n' we're leaving a carriage return in the string. For example: 'foo\r\nbar'.split('\n') --> ['foo\r', 'bar'] Then when we're joining with the line separator we're getting: ['foo\r', 'bar'].join('<br>') --> 'foo\r<br>bar'

rmcginty commented 3 years ago

Hmm. I am using alwaysWrapLineHighlights: false and lineSeparator: '<br>' and still see the issue. I was able to get the unit tests running. Let me see if I can get a couple of new tests that reproduce the issue.

bennypowers commented 2 years ago

Those waiting on the above-linked PR can apply this patch (which contains a bunch of irrelevant auto-formatting changes, sorry):

patches/@11ty+eleventy-plugin-syntaxhighlight+4.0.0.patch ```diff diff --git a/node_modules/@11ty/eleventy-plugin-syntaxhighlight/src/HighlightPairedShortcode.js b/node_modules/@11ty/eleventy-plugin-syntaxhighlight/src/HighlightPairedShortcode.js index 5dbd602..bc5765c 100644 --- a/node_modules/@11ty/eleventy-plugin-syntaxhighlight/src/HighlightPairedShortcode.js +++ b/node_modules/@11ty/eleventy-plugin-syntaxhighlight/src/HighlightPairedShortcode.js @@ -3,31 +3,32 @@ const PrismLoader = require("./PrismLoader"); const HighlightLinesGroup = require("./HighlightLinesGroup"); const getAttributes = require("./getAttributes"); -module.exports = function (content, language, highlightNumbers, options = {}) { +module.exports = function(content, language, highlightNumbers, options = {}) { const preAttributes = getAttributes(options.preAttributes); const codeAttributes = getAttributes(options.codeAttributes); // default to on - if(options.trim === undefined || options.trim === true) { + if (options.trim === undefined || options.trim === true) { content = content.trim(); } let highlightedContent; - if( language === "text" ) { + if ( language === "text" ) { highlightedContent = content; } else { highlightedContent = Prism.highlight(content, PrismLoader(language), language); } - let group = new HighlightLinesGroup(highlightNumbers); - let lines = highlightedContent.split("\n"); + const group = new HighlightLinesGroup(highlightNumbers); + // see https://github.com/11ty/eleventy-plugin-syntaxhighlight/pull/60/files + let lines = highlightedContent.split(/\r?\n/); lines = lines.map(function(line, j) { - if(options.alwaysWrapLineHighlights || highlightNumbers) { - let lineContent = group.getLineMarkup(j, line); + if (options.alwaysWrapLineHighlights || highlightNumbers) { + const lineContent = group.getLineMarkup(j, line); return lineContent; } return line; }); - return `
` + lines.join(options.lineSeparator || "
") + "
"; + return `
${lines.join(options.lineSeparator || "
")}
`; }; ```
zachleat commented 2 years ago

Shipping with 4.1.0!