Skip to content

Conversation

@Fishrock123
Copy link
Contributor

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc, tty

Description of change

Partially taken from https://linux.die.net/man/3/cfmakeraw

A very simple test script:

if (process.argv[2] === 'raw') process.stdin.setRawMode(true) process.stdin.on('data', (chunk) =>{console.log(chunk) console.log(chunk.toString()) }) 

Related to #10037
Refs(?): http://docs.libuv.org/en/v1.x/tty.html#c.uv_tty_mode_t

@Fishrock123Fishrock123 added doc Issues and PRs related to the documentations. tty Issues and PRs related to the tty subsystem. labels Dec 6, 2016
@nodejs-github-botnodejs-github-bot added doc Issues and PRs related to the documentations. tty Issues and PRs related to the tty subsystem. labels Dec 6, 2016
doc/api/tty.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you wrap at 80 characters.

doc/api/tty.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I’d just call them “modifiers”, I don’t think they count as characters in any meaningful context

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could probably just use a comma instead of parentheses.

doc/api/tty.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why For `stdin` ? This applies to all tty.ReadStreams equally

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mean -> means

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well stdout doesn't usually receive this kind of input? Honestly I'm not really sure what this does to stdout...

doc/api/tty.md Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe expand “echoing” to “echoing input characters” for clarity

doc/api/tty.md Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the best way to improve it, but this sentence in parens looks off.

Copy link
Member

@addaleaxaddaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still wonder why stdin is being explicitly mentioned in the added text but other than that LGTM

@Fishrock123
Copy link
ContributorAuthor

@addaleax LGTY?

Copy link
Member

@addaleaxaddaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, LGTM!

Partially taken from https://linux.die.net/man/3/cfmakeraw A very simple test script: ``` if (process.argv[2] === 'raw') process.stdin.setRawMode(true) process.stdin.on('data', (chunk) =>{console.log(chunk) console.log(chunk.toString()) }) ``` Refs: nodejs#10037 PR-URL: nodejs#10147 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
@Fishrock123Fishrock123 merged commit a0a6ff2 into nodejs:masterDec 8, 2016
Fishrock123 added a commit that referenced this pull request Dec 13, 2016
Partially taken from https://linux.die.net/man/3/cfmakeraw A very simple test script: ``` if (process.argv[2] === 'raw') process.stdin.setRawMode(true) process.stdin.on('data', (chunk) =>{console.log(chunk) console.log(chunk.toString()) }) ``` Refs: #10037 PR-URL: #10147 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
@italoacasasitaloacasas mentioned this pull request Dec 15, 2016
MylesBorins pushed a commit that referenced this pull request Jan 16, 2017
Partially taken from https://linux.die.net/man/3/cfmakeraw A very simple test script: ``` if (process.argv[2] === 'raw') process.stdin.setRawMode(true) process.stdin.on('data', (chunk) =>{console.log(chunk) console.log(chunk.toString()) }) ``` Refs: #10037 PR-URL: #10147 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
Partially taken from https://linux.die.net/man/3/cfmakeraw A very simple test script: ``` if (process.argv[2] === 'raw') process.stdin.setRawMode(true) process.stdin.on('data', (chunk) =>{console.log(chunk) console.log(chunk.toString()) }) ``` Refs: #10037 PR-URL: #10147 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jan 24, 2017
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
Partially taken from https://linux.die.net/man/3/cfmakeraw A very simple test script: ``` if (process.argv[2] === 'raw') process.stdin.setRawMode(true) process.stdin.on('data', (chunk) =>{console.log(chunk) console.log(chunk.toString()) }) ``` Refs: #10037 PR-URL: #10147 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.ttyIssues and PRs related to the tty subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Fishrock123@jasnell@addaleax@cjihrig@MylesBorins@nodejs-github-bot