Skip to content

console: add color support #19372

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

Closed
addaleax wants to merge 5 commits into from
Closed

console: add color support #19372

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
75cc568
util: introduce `formatWithOptions()`
addaleax Mar 15, 2018
565d8c3
doc: document `Console(…, ignoreErrors)` option
addaleax Mar 15, 2018
ec7741f
console: allow `options` object as constructor arg
addaleax Mar 17, 2018
313f29c
console: add color support
addaleax Mar 15, 2018
0e52d4b
console: auto-detect color support by default
addaleax Mar 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
doc: document Console(…, ignoreErrors) option
  • Loading branch information
@addaleax
addaleax committed Apr 12, 2018
commit 565d8c391e7132884c51388b736e899e0fc52833
14 changes: 12 additions & 2 deletions doc/api/console.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,10 @@ changes:
- version: v8.0.0
pr-url: https://github.com/nodejs/node/pull/9744
description: Errors that occur while writing to the underlying streams
will now be ignored.
will now be ignored by default.
-->


<!--type=class-->

The `Console` class can be used to create a simple logger with configurable
Expand All @@ -77,9 +78,18 @@ const { Console } = require('console');
const { Console } = console;
```

### new Console(stdout[, stderr])
### new Console(stdout[, stderr][, ignoreErrors])
Copy link
Copy Markdown
Member Author

@addaleax addaleax Apr 9, 2018

Choose a reason for hiding this comment

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

@vsemozhetbyt The doctool doesn’t seem to be able to handle this anymore, since 9a6dd07?

Copy link
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt Apr 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Do you mean two consecutive headings? I cannot say why it should not. We have many of them in the bufer.md and they seem to be handled properly. Do you encounter issues with tests or doc building?

Copy link
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt Apr 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, sorry, now I've tested this. It seems the consecutive headings we already have contain similar parameters and they do not differ from the list parameters below. Before 9a6dd07, these differences were checked, but they did not throw, just reported to stderr. It seems we should either split the section or change the parser strictness, which may not be good for detecting differences between signatures and parameter lists.

Copy link
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt Apr 9, 2018

Choose a reason for hiding this comment

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

BTW, consecutive headings are parsed (and were parsed) as clones: the section bodies are copied and checked identically: see here and here. This is not changed in 9a6dd07, I've just make the differences throw.

Copy link
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt Apr 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Compare How we usually split sections with different signatures:

https://nodejs.org/api/assert.html#assert_assert_fail_message
https://nodejs.org/api/assert.html#assert_assert_fail_actual_expected_message_operator_stackstartfunction

https://nodejs.org/api/buffer.html#buffer_class_method_buffer_from_array
https://nodejs.org/api/buffer.html#buffer_class_method_buffer_from_arraybuffer_byteoffset_length

https://nodejs.org/api/fs.html#fs_fs_write_fd_buffer_offset_length_position_callback
https://nodejs.org/api/fs.html#fs_fs_write_fd_string_position_encoding_callback

But with similar signatures:

https://nodejs.org/api/buffer.html#buffer_buf_readdoublebe_offset_noassert
https://nodejs.org/api/buffer.html#buffer_buf_readdoublele_offset_noassert

Copy link
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt Apr 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

So two headings with a similar name but different signatures do not confuse the JSON parser as far as the second signature is in sync with the parameter list below. However, it seems this is still the only case in our docs for now. And it can be a bit confusing: the first signature parameters are not openly listed (they are similar to object properties/options fields, but this is a bit non-formal way). Maybe we can split them, add an own minimal parameter list to the first signature and a link to the common description in the second section? It is a vague thing for me and it is not a strong opinion, so feel free to ignore) We still have a valid JSON (just a bit overflooded with parameter list clones) and the doc is still pretty readable in this way.

<!-- YAML
changes:
- version: v8.0.0
pr-url: https://github.com/nodejs/node/pull/9744
description: The `ignoreErrors` option was introduced.
-->

* `stdout` {stream.Writable}
* `stderr` {stream.Writable}
* `ignoreErrors` {boolean} Ignore errors when writing to the underlying streams.
Defaults to `true`.

Creates a new `Console` with one or two writable stream instances. `stdout` is a
writable stream to print log or info output. `stderr` is used for warning or
Expand Down