Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

readline: crlfDelay and fs streams #16077

Closed
vsemozhetbyt opened this issue Oct 8, 2017 · 2 comments
Closed

readline: crlfDelay and fs streams #16077

vsemozhetbyt opened this issue Oct 8, 2017 · 2 comments
Labels
readline Issues and PRs related to the built-in readline module.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Oct 8, 2017 •
edited
Loading

  • Version: from v6.6.0
  • Platform: mostly Windows
  • Subsystem: readline

Recently, I've got a complaint that some of my old scripts for a line by line processing of big files add unexpected new lines. The cause seems to be similar to the one described in this OP.

I suggest two variants of precaution:

  1. Add a small note to the doc + set crlfDelay to Infinity in our main file stream example. This change is milder, but it is also a bit confusing and makes the fs example look like a workaround or even an abuse of terminal API.

  2. Automatically set crlfDelay to Infinity when terminal is false or even completely remove this check from ._normalWrite(). Is there any reason for this option in fs use cases?

Refs: #8109, #13497
@vsemozhetbyt vsemozhetbyt added the readline Issues and PRs related to the built-in readline module. label Oct 8, 2017
@Fishrock123
Copy link
Contributor

I think 2. seems reasonable?

@vsemozhetbyt
Copy link
Contributor Author

So maybe I shall start with a doc change. If anybody is confident enough to implement the second change, then the doc change can be reverted,

@vsemozhetbyt vsemozhetbyt mentioned this issue Oct 17, 2017
Closed
2 tasks
MylesBorins pushed a commit that referenced this issue Oct 23, 2017
@vsemozhetbyt @MylesBorins
doc: clarify using crlfDelay with fs streams
497bfff 
PR-URL: #16259
Fixes: #16077
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this issue Oct 26, 2017
@vsemozhetbyt @addaleax
doc: clarify using crlfDelay with fs streams
14ddaa2 
PR-URL: nodejs/node#16259
Fixes: nodejs/node#16077
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this issue Dec 7, 2017
@vsemozhetbyt @addaleax
doc: clarify using crlfDelay with fs streams
1405eb7 
PR-URL: nodejs/node#16259
Fixes: nodejs/node#16077
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
readline Issues and PRs related to the built-in readline module.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Fishrock123 @vsemozhetbyt