repl,util: insert carriage returns in output

[JungMinu]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

repl, util

Description of change

\n is not enough for Linux with some custom stream
add carriage returns to ensure that the output is displayed correctly
using \r\n should not be a problem, even on non-Windows platforms.

Fixes: #7954

[JungMinu]

CI: https://ci.nodejs.org/job/node-test-pull-request/3586/
/cc @mscdex @jasnell @Fishrock123 @addaleax @julianduque

[addaleax]

I guess this looks okay (and semver-major-y?) but tbh I didn’t really understand the problem it is supposed to solve… usually TTYs should have their ONLCR flag set by default, translating \n to \r\n?

[mscdex]

@addaleax I'm not sure why it was happening that way, but that is the problem I was running into with a module I wrote.

[JungMinu]

@mscdex PTAL, LGTY?

[mscdex]

There are still various places that still only use \n, especially anything that is written to the outputStream.

[JungMinu]

@mscdex Done, PTAL again :)

[JungMinu]

@mscdex If LGTY, I will run a new CI

[Fishrock123]

Would it bet better to use os.EOL for this?

[addaleax]

See #7954 (comment) … so, apparently no, but I’d still really like to understand what the real problem here is that this is trying to solve.

[JungMinu]

gentle ping @mscdex

[mscdex]

I think that should do it. LGTM if CI is ok with it.

[JungMinu]

new CI: https://ci.nodejs.org/job/node-test-pull-request/3652/

[JungMinu]

CI is green :)

[JungMinu]

Landed in fce4b98

[evanlucas]

I feel like this should have been marked as a breaking change.

[yorkie]

+1 on marking as breaking change.