Skip to content

doc: add some info on tty#setRawMode()#10147

Merged
Fishrock123 merged 1 commit intonodejs:masterfrom
Fishrock123:tty-rawmode-docs
Dec 8, 2016
Merged

doc: add some info on tty#setRawMode()#10147
Fishrock123 merged 1 commit intonodejs:masterfrom
Fishrock123:tty-rawmode-docs

Conversation

@Fishrock123
Copy link
Contributor

@Fishrock123 Fishrock123 commented Dec 6, 2016

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

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

@cjihrig cjihrig Dec 6, 2016

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.

addaleax
addaleax reviewed Dec 6, 2016
View reviewed changes
doc/api/tty.md Outdated
Copy link
Member

@addaleax addaleax Dec 6, 2016

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

@cjihrig cjihrig Dec 6, 2016

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

@addaleax addaleax Dec 6, 2016

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

@cjihrig cjihrig Dec 6, 2016

Choose a reason for hiding this comment

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

mean -> means

Copy link
Contributor Author

@Fishrock123 Fishrock123 Dec 6, 2016

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

@addaleax addaleax Dec 6, 2016

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

cjihrig
cjihrig reviewed Dec 6, 2016
View reviewed changes
doc/api/tty.md Outdated
Copy link
Contributor

@cjihrig cjihrig Dec 6, 2016

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.

addaleax
addaleax approved these changes Dec 7, 2016
View reviewed changes
Copy link
Member

@addaleax addaleax 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
Contributor Author

Fishrock123 commented Dec 8, 2016

@addaleax LGTY?

addaleax
addaleax approved these changes Dec 8, 2016
View reviewed changes
Copy link
Member

@addaleax addaleax 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!

@Fishrock123
doc: add some info on tty#setRawMode()
a0a6ff2 
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 <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Fishrock123 Fishrock123 merged commit a0a6ff2 into nodejs:master Dec 8, 2016
Fishrock123 added a commit that referenced this pull request Dec 13, 2016
@Fishrock123
doc: add some info on tty#setRawMode()
20fdf3a 
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 <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@italoacasas italoacasas mentioned this pull request Dec 15, 2016
Merged
MylesBorins pushed a commit that referenced this pull request Jan 16, 2017
@Fishrock123 @MylesBorins
doc: add some info on tty#setRawMode()
13a9d40 
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 <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@Fishrock123 @MylesBorins
doc: add some info on tty#setRawMode()
3d3263c 
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 <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
@Fishrock123 @MylesBorins
doc: add some info on tty#setRawMode()
7815efa 
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 <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@addaleax addaleax addaleax approved these changes

@cjihrig cjihrig cjihrig approved these changes

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. tty Issues and PRs related to the tty subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

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