Support CRLF when splitting code lines for display

[silverwind]

This removes the whitspace between <li> elements and adds them to the tag content, where it was originally removed. display:block because with the previous display:inline-block it would all end up on one line.

Fixes: #1857

[ethantkoenig]

Could we instead do

output.WriteString(fmt.Sprintf(`<li class="L%d" rel="L%d">%s\n</li>`, index+1, index+1, gotemplate.HTMLEscapeString(line)))

[silverwind]

Possibly, but I think I'd rather avoid newlines in HTML entirely.

[silverwind]

I'll have to revise this, I think it should be possible to avoid newlines in HTML (line termination should be done through block-level elements). Notably this issue only affects files without syntax highlighting.

[silverwind]

Updated. The actual issue with CRLF copying was that lines were always split only on \n which left a lone \r on the line, which browsers interpreted as a separate newline (it wasn't visible as such because of CSS, but it became visible on copy/paste).

I've now updated the line splitting to detect files containing \r\n and split them on this sequence and join them back after rendering the HTML. I'd have loved to eliminate line terminators from HTML completely, but the syntax highlighting broke when I did, so these will need to stay for now.

For files that contain both \n and \r\n, it will render the whole file as \n.

[ethantkoenig]

@silverwind The CI build failed, run make fmt

[lafriks]

To deal with mixed newlines it would be better to split using:
lines := regexp.MustCompile("\\r?\\n").Split(fileContent, -1)

[silverwind]

Yeah, but how do I know whether to join the lines with LF or CRLF afterwards in the loop? I'd like to preserve CRLF if it's there.

[lafriks]

Leave detection code, just replace strings split with regexp split

[silverwind]

Hmm, actually I'll try your suggestion. I can keep the detection code above to know.

[ethantkoenig]

By using a single terminator, we aren't able to correctly render files that sometime use \r\n and in other places use \n? Is there a reason we can't just do

lines := strings.Split(fileContent, "\n")
for index, line := range lines {
	if index < len(lines) - 1 {
		line += "\n"
	}
	output.WriteString(fmt.Sprintf(`<li class="L%d" rel="L%d">%s</li>`, index+1, index+1, line))
}

Since both \r\n and \n end in \n, it seems to me this would work, and it would handle the case where a file mixes \r\n and \n

[silverwind]

Actually, yes, this looks to work. My first approach was similar to this one, but for some reason I stopped pursuing it. Anyways, this copies fine, preserves newlines in mixed files and also highlights fine, looking good!

[ethantkoenig]

LGTM

[lunny]

LGTM

[lunny]

make L-G-T-M work