Output window handles ANSI newline escape sequence incorrectly

[rmunn]

• VSCode Version: 1.5.3 (commit 5be4091)
• OS Version: Ubuntu Linux 14.04

Steps to Reproduce:

1. Write some code that prints the characters xA to the output window.
2. They are incorrectly turned into a newline.

F# Code:

printfn "Test - axAb should not print a newline between a and b"

Output:

Test - a
b should not print a newline between a and b

This has already been reported as #12108, but that bug was incorrectly closed as "working as designed" because it was assumed that the code in removeAnsiEscapeCodes was working as designed. In fact, that code is buggy, and has actually been buggy since the first commit to VS Code's public repo (I couldn't trace it further back than that). The character sequence xA is not the ANSI escape for a linefeed/newline; in fact, there is no ANSI escape character for a linefeed/newline, since ANSI just uses the single character U+000A for that purpose. It's pretty clear, looking at the code, that that's what was the LF regex was intended to be set to in the lines just above the removeAnsiEscapeCodes: it is set to /\xA/g. However, \xA is actually an invalid Javascript hex escape. The ECMAScript spec specifies that \x must be followed by two hex digits, because otherwise the sequences \b (U+0007 BELL) and \f (U+000C FORM FEED) would be ambiguous. So \xA is an invalid sequence, and is instead interpreted by Javascript as the character x followed by the character A (the backslash is swallowed).

So the correct fix to bug #12108 is actually to replace that /\xA/g regex with /\x0A/g.

Then one could notice that the code immediately below is replacing all occurrences of 0x0A with \n, which is a no-op since \n is 0x0A. So the LF regex and the str = str.replace(LF, '\n') line serve no purpose but to slow things down by spending O(N) time doing a no-op regex search. They can safely be removed without affecting any other code, and this would actually speed things up a fraction.

[isidorn]

fyi @bpasero

[bpasero]

Yup, we should look into this and fix 👍

[isidorn]

@rmunn might you be interested into doing a PR that fixes this since you have already done this great analysis?
If not, we can follow your analysis to do the changes you proposed.

Thanks either way!

[rmunn]

Sure, why not? Here's the PR. Do you need me to sign a code licensing agreement of some kind before it can be merged in? If so, let me know how. UPDATE: Oh, I see that MSBOT replied on the PR's comment thread. I'm not surprised that no CLA is required for a commit that removed two lines and added none.