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.

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

doc: extra clarification of historySize option #6397

Closed
wants to merge 2 commits into from
Closed

doc: extra clarification of historySize option #6397

wants to merge 2 commits into from

Conversation

vsemozhetbyt
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt commented Apr 26, 2016

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

doc

Description of change

History caching in the readline io is active only for terminal interaction. Appropriate variables are initialized and relevant _addHistory() function is called only if exposed terminal option of readline.createInterface() is set to true by user or internal output check.

This clarification is useful to assure users there will be now wasted overhead connected with history caching if readline is used not for terminal interaction (e.g. for reading files line by line).

Particularly this fix is helpful after #6352 landing.

doc: extra clarification of historySize option

History caching in the `readline` io is active only for terminal
interaction. Appropriate variables are initialized and relevant
`_addHistory()` function is called only if exposed `terminal` option
of `readline.createInterface()` is set `true` by user or internal
output check.

This clarification is useful to assure users there will be now wasted
overhead connected with history caching if `readline` is used not
for terminal interaction (e.g. for reading files line by line).

Particularly this fix is helpful after #6352 landing.
@addaleax addaleax added doc Issues and PRs related to the documentations. readline Issues and PRs related to the built-in readline module. labels Apr 26, 2016
@@ -301,7 +301,9 @@ the following values:
Defaults to checking `isTTY` on the `output` stream upon instantiation.

- `historySize` - maximum number of history lines retained. To disable the
history set this value to `0`. Defaults to `30`.
history set this value to `0`. Defaults to `30`. This option makes sense
only if `terminal` is set `true` by user or by internal `output` check,
Copy link
Member

Choose a reason for hiding this comment

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

is set to `true`

set `true` -> set to `true`
@jasnell
Copy link
Member

jasnell commented Apr 27, 2016

LGTM

1 similar comment
@estliberitas
Copy link
Contributor

LGTM

@jasnell
Copy link
Member

jasnell commented Apr 29, 2016

@nodejs/documentation

@benjamingr
Copy link
Member

LGTM

jasnell pushed a commit that referenced this pull request May 1, 2016
History caching in the `readline` io is active only for terminal
interaction. Appropriate variables are initialized and relevant
`_addHistory()` function is called only if exposed `terminal` option
of `readline.createInterface()` is set `true` by user or internal
output check.

This clarification is useful to assure users there will be now wasted
overhead connected with history caching if `readline` is used not
for terminal interaction (e.g. for reading files line by line).

Particularly this fix is helpful after #6352 landing.

PR-URL: #6397
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexander Makarenko <estliberitas@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
@jasnell
Copy link
Member

jasnell commented May 1, 2016

Landed in 563c9f8. Made a couple of minor fixups in the process. Thank you for the contribution!

@jasnell jasnell closed this May 1, 2016
@vsemozhetbyt vsemozhetbyt deleted the patch-1 branch May 1, 2016 21:50
Fishrock123 pushed a commit that referenced this pull request May 4, 2016
History caching in the `readline` io is active only for terminal
interaction. Appropriate variables are initialized and relevant
`_addHistory()` function is called only if exposed `terminal` option
of `readline.createInterface()` is set `true` by user or internal
output check.

This clarification is useful to assure users there will be now wasted
overhead connected with history caching if `readline` is used not
for terminal interaction (e.g. for reading files line by line).

Particularly this fix is helpful after #6352 landing.

PR-URL: #6397
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexander Makarenko <estliberitas@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
joelostrowski pushed a commit to joelostrowski/node that referenced this pull request May 4, 2016
History caching in the `readline` io is active only for terminal
interaction. Appropriate variables are initialized and relevant
`_addHistory()` function is called only if exposed `terminal` option
of `readline.createInterface()` is set `true` by user or internal
output check.

This clarification is useful to assure users there will be now wasted
overhead connected with history caching if `readline` is used not
for terminal interaction (e.g. for reading files line by line).

Particularly this fix is helpful after nodejs#6352 landing.

PR-URL: nodejs#6397
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Alexander Makarenko <estliberitas@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. readline Issues and PRs related to the built-in readline module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants