util: add sorted option #22788

BridgeAR · 2018-09-10T07:48:14Z

The first commit adds the ~~ordered~~ sorted option to util.inspect(). It makes sure object inspection output is sorted and stable, no matter of the property insertion order or the order in the set / map.

The second commit improves the assertion diff output by using the ~~ordered~~ sorted option. If requested, I'll move this PR out in a second one that relies on this.

Refs: #22763

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2018-09-10T07:48:21Z

@BridgeAR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/849/pipeline

targos · 2018-09-10T07:57:21Z

I'd call it "sorted"

mscdex · 2018-09-10T13:07:42Z

What does 'reliable' mean here exactly? To me that sounds like 'stable' which we don't have yet for sorting in V8 (but will soon as this behavior has been merged into V8 recently FWIW).

BridgeAR · 2018-09-11T11:50:58Z

I have no strong opinion about either ordered, sorted or stable as name.

@mscdex I used stable in the commit description. If the sort is truly stable (as in the new V8 version) does not matter for this use case since all values are strings.

BridgeAR · 2018-09-11T18:15:59Z

Small poll @nodejs/util for finding the best name

👍 ordered
🎉 sorted
❤️ stable

jdalton · 2018-09-12T15:10:24Z

When I saw the issue title and began to review the PR I thought this was an option to preserve object property order (I thought maybe the existing implementation may have messed that up and this was a back compat way to fix it) and not something dealing with sorting. I was surprised when I got to the code bit and it was for sorting properties.

BridgeAR · 2018-09-12T22:15:58Z

I just renamed it to sorted.

CI https://ci.nodejs.org/job/node-test-pull-request/17160/

jdalton · 2018-09-12T22:33:16Z

lib/util.js

+    if (ctx.sorted === true) {
+      output = output.sort();
+      return reduceToSingleString(ctx, output.sort(), base, braces);
+    }


☝️ This is sorting things twice. I believe the first one, output = output.sort(), isn't needed.

jdalton · 2018-09-12T22:38:19Z

lib/util.js

+      return reduceToSingleString(ctx, output.sort(), base, braces);
+    }
+    return reduceToSingleString(ctx, output.sort(ctx.sorted), base, braces);
+  }
  return reduceToSingleString(ctx, output, base, braces);


👆 To dry out the condition above it could be something like:

const comparator = ctx.sorted === true ? undefined : ctx.sorted return reduceToSingleString(ctx, output.sort(comparator), base, braces)

BridgeAR · 2018-09-12T22:42:22Z

Comments addressed :)

BridgeAR · 2018-09-12T22:43:28Z

CI https://ci.nodejs.org/job/node-test-pull-request/17161/

BridgeAR · 2018-09-13T09:21:41Z

Rebased due to conflicts.

New CI https://ci.nodejs.org/job/node-test-pull-request/17168/ (:negative_squared_cross_mark:)

addaleax · 2018-09-14T16:42:13Z

doc/api/util.md

+  * `sorted` {boolean|Function} If specified, the output for all object types
+    besides arrays and typed arrays is going to be stable. If set to `true` the
+    default sort is going to be used. If set to a function, it is used as
+    [compare function][].


as a [compare function][]?

addaleax · 2018-09-14T16:42:54Z

doc/api/util.md

@@ -426,6 +429,10 @@ changes:
    objects the same as arrays. Note that no text will be reduced below 16
    characters, no matter the `breakLength` size. For more information, see the
    example below. **Default:** `true`.
+  * `sorted` {boolean|Function} If specified, the output for all object types
+    besides arrays and typed arrays is going to be stable. If set to `true` the


I’m not sure it’s obvious what stable means? Maybe just explain what happens, i.e. that entries will be sorted before being printed?

The order option can be used to sort the inspected values in case they do not rely on their order as arrays. That way the output is stable no matter of the object property inspection order.

BridgeAR · 2018-09-17T08:40:35Z

PTAL. I just rebased due to conflicts and addressed the comments. I also added an example and made sure that non-index properties on arrays and typed arrays will also be sorted. The changed part is in the fixup: util commit.

CI https://ci.nodejs.org/job/node-test-pull-request/17225/

vsemozhetbyt · 2018-09-17T21:04:14Z

doc/api/util.md

@@ -535,6 +542,34 @@ console.log(inspect(weakSet, { showHidden: true }));
 // WeakSet { { a: 1 }, { b: 2 } }
 ```

+The `sorted` option makes sure the output is stable, no matter of the properties


It seems the word stable is a bit confusing here. Compare this sentence with the wiki definition (referenced from the MDN Array.prototype.sort() page):

Stable sort algorithms sort identical elements in the same order that they appear in the input.

I.e. the traditional meaning of the stable sorting is opposite to no matter of the properties insertion order.

True, I think I found a better wording.

The output is now a tiny bit improved by sorting object properties when inspecting the values that are compared with each other. That reduces the overall diff for identical objects with a different property insertion order.

The order option can be used to sort the inspected values in case they do not rely on their order as arrays. That way the output is stable no matter of the object property inspection order. Backport-PR-URL: #23039 PR-URL: #22788 Refs: #22763 Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

The output is now a tiny bit improved by sorting object properties when inspecting the values that are compared with each other. That reduces the overall diff for identical objects with a different property insertion order. PR-URL: nodejs#22788 Refs: nodejs#22763 Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

The output is now a tiny bit improved by sorting object properties when inspecting the values that are compared with each other. That reduces the overall diff for identical objects with a different property insertion order. Backport-PR-URL: #23226 PR-URL: #22788 Refs: #22763 Reviewed-By: John-David Dalton <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]>

Notable changes: * assert * The diff output is now a tiny bit improved by sorting object properties when inspecting the values that are compared with each other. #22788 * cli * The options parser now normalizes `_` to `-` in all multi-word command-line flags, e.g. `--no_warnings` has the same effect as `--no-warnings`. #23020 * Added bash completion for the `node` binary. To generate a bash completion script, run `node --completion-bash`. The output can be saved to a file which can be sourced to enable completion. #20713 * crypto * Added support for PEM-level encryption. #23151 * Added an API asymmetric key pair generation. The new methods `crypto.generateKeyPair` and `crypto.generateKeyPairSync` can be used to generate public and private key pairs. The API supports RSA, DSA and EC and a variety of key encodings (both PEM and DER). #22660 * fs * Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If this option is set to true, non-existing parent folders will be automatically created. #21875 * http2 * Added a `'ping'` event to `Http2Session` that is emitted whenever a non-ack `PING` is received. #23009 * Added support for the `ORIGIN` frame. #22956 * module * Added `module.createRequireFromPath(filename)`. This new method can be used to create a custom require function that will resolve modules relative to the filename path. #19360 * process * Added a `'multipleResolves'` process event that is emitted whenever a `Promise` is attempted to be resolved multiple times, e.g. if the `resolve` and `reject` functions are both called in a `Promise` executor. #22218 * **url** * Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These methods can be used to correctly convert between file: URLs and absolute paths. #22506 * **util** * Added the `sorted` option to `util.inspect()`. If set to `true`, all properties of an object and Set and Map entries will be sorted in the returned string. If set to a function, it is used as a compare function. #22788 * The `util.instpect.custom` symbol is now defined in the global symbol registry as `Symbol.for('nodejs.util.inspect.custom')`. #20857 * **Windows** * The Windows msi installer now provides an option to automatically install the tools required to build native modules. #22645 * **Added new collaborators**: * digitalinfinity - Hitesh Kanwathirtha PR-URL: #23313

Notable changes: * assert * The diff output is now a tiny bit improved by sorting object properties when inspecting the values that are compared with each other. #22788 * cli * The options parser now normalizes `_` to `-` in all multi-word command-line flags, e.g. `--no_warnings` has the same effect as `--no-warnings`. #23020 * Added bash completion for the `node` binary. To generate a bash completion script, run `node --completion-bash`. The output can be saved to a file which can be sourced to enable completion. #20713 * crypto * Added support for PEM-level encryption. #23151 * Added an API asymmetric key pair generation. The new methods `crypto.generateKeyPair` and `crypto.generateKeyPairSync` can be used to generate public and private key pairs. The API supports RSA, DSA and EC and a variety of key encodings (both PEM and DER). #22660 * fs * Added a `recursive` option to `fs.mkdir` and `fs.mkdirSync`. If this option is set to true, non-existing parent folders will be automatically created. #21875 * http2 * Added a `'ping'` event to `Http2Session` that is emitted whenever a non-ack `PING` is received. #23009 * Added support for the `ORIGIN` frame. #22956 * Updated nghttp2 to 1.34.0. This adds RFC 8441 extended connect protocol support to allow use of WebSockets over HTTP/2. #23284 * module * Added `module.createRequireFromPath(filename)`. This new method can be used to create a custom require function that will resolve modules relative to the filename path. #19360 * process * Added a `'multipleResolves'` process event that is emitted whenever a `Promise` is attempted to be resolved multiple times, e.g. if the `resolve` and `reject` functions are both called in a `Promise` executor. #22218 * url * Added `url.fileURLToPath(url)` and `url.pathToFileURL(path)`. These methods can be used to correctly convert between file: URLs and absolute paths. #22506 * util * Added the `sorted` option to `util.inspect()`. If set to `true`, all properties of an object and Set and Map entries will be sorted in the returned string. If set to a function, it is used as a compare function. #22788 * The `util.instpect.custom` symbol is now defined in the global symbol registry as `Symbol.for('nodejs.util.inspect.custom')`. #20857 * Added support for `BigInt` numbers in `util.format()`. #22097 * V8 API * A number of V8 C++ APIs have been marked as deprecated since they have been removed in the upstream repository. Replacement APIs are added where necessary. #23159 * Windows * The Windows msi installer now provides an option to automatically install the tools required to build native modules. #22645 * Workers * Debugging support for Workers using the DevTools protocol has been implemented. #21364 * The public `inspector` module is now enabled in Workers. #22769 * Added new collaborators: * digitalinfinity - Hitesh Kanwathirtha PR-URL: #23313

BridgeAR added the semver-minor PRs that contain new features and should be released in the next minor version. label Sep 10, 2018

nodejs-github-bot added assert Issues and PRs related to the assert subsystem. util Issues and PRs related to the built-in util module. labels Sep 10, 2018

BridgeAR changed the title ~~util: add ordered option~~ util: add sorted option Sep 12, 2018

jdalton reviewed Sep 12, 2018

View reviewed changes

BridgeAR force-pushed the improve-assertion-error branch from 140bce8 to c979552 Compare September 12, 2018 22:41

jdalton approved these changes Sep 12, 2018

View reviewed changes

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 12, 2018

BridgeAR force-pushed the improve-assertion-error branch from c979552 to 314a7f8 Compare September 13, 2018 09:20

addaleax approved these changes Sep 14, 2018

View reviewed changes

addaleax removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 14, 2018

trivikr approved these changes Sep 15, 2018

View reviewed changes

BridgeAR added 2 commits September 16, 2018 22:45

util: add order option to .inspect()

28ffb3b

The order option can be used to sort the inspected values in case they do not rely on their order as arrays. That way the output is stable no matter of the object property inspection order.

fixup: util

a8f898c

BridgeAR force-pushed the improve-assertion-error branch from 314a7f8 to 036759e Compare September 17, 2018 08:34

vsemozhetbyt reviewed Sep 17, 2018

View reviewed changes

BridgeAR added 2 commits September 18, 2018 13:30

assert: improve diff output

0ade8a7

The output is now a tiny bit improved by sorting object properties when inspecting the values that are compared with each other. That reduces the overall diff for identical objects with a different property insertion order.

fixup: fix assert test

ea61a2f

targos added backported-to-v10.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v10.x labels Sep 24, 2018

oyyd mentioned this pull request Sep 26, 2018

[v10.x] Backport assert PRs #23095

Closed

2 tasks

targos mentioned this pull request Oct 7, 2018

Release proposal: v10.12.0 #23313

Merged

BridgeAR deleted the improve-assertion-error branch January 20, 2020 11:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

util: add sorted option #22788

util: add sorted option #22788

BridgeAR commented Sep 10, 2018 •

edited

Loading

nodejs-github-bot commented Sep 10, 2018

targos commented Sep 10, 2018

mscdex commented Sep 10, 2018

BridgeAR commented Sep 11, 2018

BridgeAR commented Sep 11, 2018

jdalton commented Sep 12, 2018

BridgeAR commented Sep 12, 2018

jdalton Sep 12, 2018 •

edited

Loading

jdalton Sep 12, 2018

BridgeAR commented Sep 12, 2018

BridgeAR commented Sep 12, 2018

BridgeAR commented Sep 13, 2018 •

edited by addaleax

Loading

addaleax Sep 14, 2018

addaleax Sep 14, 2018

BridgeAR commented Sep 17, 2018

vsemozhetbyt Sep 17, 2018

BridgeAR Sep 18, 2018

util: add sorted option #22788

util: add sorted option #22788

Conversation

BridgeAR commented Sep 10, 2018 • edited Loading

Checklist

nodejs-github-bot commented Sep 10, 2018

targos commented Sep 10, 2018

mscdex commented Sep 10, 2018

BridgeAR commented Sep 11, 2018

BridgeAR commented Sep 11, 2018

jdalton commented Sep 12, 2018

BridgeAR commented Sep 12, 2018

jdalton Sep 12, 2018 • edited Loading

Choose a reason for hiding this comment

jdalton Sep 12, 2018

Choose a reason for hiding this comment

BridgeAR commented Sep 12, 2018

BridgeAR commented Sep 12, 2018

BridgeAR commented Sep 13, 2018 • edited by addaleax Loading

addaleax Sep 14, 2018

Choose a reason for hiding this comment

addaleax Sep 14, 2018

Choose a reason for hiding this comment

BridgeAR commented Sep 17, 2018

vsemozhetbyt Sep 17, 2018

Choose a reason for hiding this comment

BridgeAR Sep 18, 2018

Choose a reason for hiding this comment

BridgeAR commented Sep 10, 2018 •

edited

Loading

jdalton Sep 12, 2018 •

edited

Loading

BridgeAR commented Sep 13, 2018 •

edited by addaleax

Loading