Skip to content

Newlines missing carriage returns in REPL, util.inspect() output #7954

Closed
#8028
Closed
Newlines missing carriage returns in REPL, util.inspect() output#7954
#8028
Assignees
JungMinu
Labels
good first issueIssues that are suitable for first-time contributors.replIssues and PRs related to the REPL subsystem.utilIssues and PRs related to the built-in util module.

Description

@mscdex
mscdex
opened on Aug 3, 2016
  • Version: v6.3.1
  • Platform: Linux
  • Subsystem: repl, util

I noticed that the repl module and util.inspect() only write \n and not \r\n in their output. readline however consistently writes \r\n to its output.

This seemingly causes output problems with custom REPLs with custom output streams. I'm not exactly sure why this is a problem though, since the output streams eventually reach a (Linux) terminal. The current workaround I have is to create a newline conversion stream (that inserts carriage returns) that then pipes to my actual output stream. However, I'd prefer not to have to use this workaround forever.

This should just be a matter of changing the relevant instances of \n to \r\n in lib/repl.js and lib/util.js.

Activity

  1. mscdex
    added
    utilIssues and PRs related to the built-in util module.
    replIssues and PRs related to the REPL subsystem.
    good first issueIssues that are suitable for first-time contributors.
    on Aug 3, 2016

  2. imyller commented on Aug 3, 2016

    @imyller
    imyller
    on Aug 3, 2016
    Member

    Userland is encouraged to use os.EOL instead of hardcoded \n or \r\n for improved portability.

    Why not use that in Node.js core libraries? console, repl, util etc.

  3. mscdex commented on Aug 3, 2016

    @mscdex
    mscdex
    on Aug 3, 2016
    ContributorAuthor

    Well as I said, for some reason \n is not enough for Linux with my custom stream (so os.EOL would not help), I'm not sure why and I haven't really dug into the issue to find out why. Local REPLs work just fine though.

    AFAIK using \r\n everywhere should not be a problem, even on non-Windows platforms, and it ensures that the output is displayed correctly.

  4. JungMinu
    self-assigned this
    on Aug 3, 2016

  5. JungMinu commented on Aug 3, 2016

    @JungMinu
    JungMinu
    on Aug 3, 2016
    Member

    I will resolve this issue :)

  6. imyller commented on Aug 3, 2016

    @imyller
    imyller
    on Aug 3, 2016
    Member

    I'm not sure why and I haven't really dug into the issue to find out why.

    So, changing core lib to use \r\n masks the real issue. Maybe test case could be created?

    AFAIK using \r\n everywhere should not be a problem, even on non-Windows platforms, and it ensures that the output is displayed correctly.

    I agree with that. I use \r\n universally just to be sure.

  7. mscdex commented on Aug 3, 2016

    @mscdex
    mscdex
    on Aug 3, 2016
    ContributorAuthor

    I'm not sure how you could create a test case to check for visual layout. I think about the best you could do is just to have a test that checks that only \r\n exists in the output and not just \n.

  8. JungMinu commented on Aug 4, 2016

    @JungMinu
    JungMinu
    on Aug 4, 2016
    Member

    Yep, I will create a pull request ASAP 😄

  9. JungMinu
    closed this as completedin #8028on Aug 13, 2016

  10. JungMinu commented on Aug 13, 2016

    @JungMinu
    JungMinu
    on Aug 13, 2016
    Member

    #8028 has now landed in fce4b98

  11. cjihrig
    added a commit that references this issue on Aug 15, 2016

    repl,util: insert carriage returns in output

    08e6eee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

good first issueIssues that are suitable for first-time contributors.replIssues and PRs related to the REPL subsystem.utilIssues and PRs related to the built-in util module.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions