debugger: display array contents in repl #6448

cjihrig · 2016-04-28T17:03:35Z

Checklist

tests and code linting passes
a test and/or benchmark is included
the commit message follows commit guidelines

Affected core subsystem(s)

debugger

Description of change

This commit allows all array properties to be printed except for length. Previously, this filter was applied by checking the type of each property. However, something changed in V8, and array elements started coming through as numeric strings, which stopped them from being displayed.

Fixes: #6444

addaleax · 2016-04-28T17:13:35Z

LGTM. CI: https://ci.nodejs.org/job/node-test-commit/3080/

This fixes some bad labelling by ensuring that *every* file affected in the PR actually matches an exclusive label when we're checking for exclusive labels. Refs nodejs/node#6448 nodejs/node#6432 Closes #33

jasnell · 2016-04-28T20:04:39Z

LGTM if CI is green (when it finally finished)

cjihrig · 2016-04-28T20:04:48Z

@indutny what do you think?

jasnell · 2016-04-28T20:07:06Z

test/debugger/test-debugger-repl.js

@@ -75,3 +75,7 @@ addTest('for (var i in process.env) delete process.env[i]', []);
 addTest('process.env', [
  /\{\}/
 ]);
+
+addTest('arr = [{foo: "bar"}]', [
+  /\[ \{ foo: 'bar' \} \]/


nit: while it's not likely to change, it might be worthwhile making the whitespace in the regex optional, just in case. Something like /\[\s*\{\s*foo\:\s*'bar'\s*\}\s*\]/

indutny · 2016-04-28T20:16:11Z

I'd love to know what changed, would it be possible to bisect deps/v8 to figure it out?

addaleax · 2016-04-28T20:17:47Z

@indutny Already did that in this repo, if that’s what you mean: #6444 (comment)

cjihrig · 2016-04-28T20:18:52Z

Yep, @addaleax was on it. In this case, array index properties are now strings instead of numbers.

indutny · 2016-04-28T21:03:02Z

Alright, LGTM then. Thank you!

jasnell · 2016-04-29T03:51:46Z

CI is good.

This commit allows all array properties to be printed except for "length". Previously, this filter was applied by checking the type of each property. However, something changed in V8, and array elements started coming through as numeric strings, which stopped them from being displayed. Fixes: nodejs#6444 PR-URL: nodejs#6448 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>

This commit allows all array properties to be printed except for "length". Previously, this filter was applied by checking the type of each property. However, something changed in V8, and array elements started coming through as numeric strings, which stopped them from being displayed. Fixes: #6444 PR-URL: #6448 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>

This commit allows all array properties to be printed except for "length". Previously, this filter was applied by checking the type of each property. However, something changed in V8, and array elements started coming through as numeric strings, which stopped them from being displayed. Fixes: nodejs#6444 PR-URL: nodejs#6448 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>

* assert: `deep{Strict}Equal()` now works correctly with circular references. (Rich Trott) #6432 * debugger: Arrays are now formatted correctly in the debugger repl. (cjihrig) #6448 * deps: Upgrade OpenSSL sources to 1.0.2h (Shigeki Ohtsu) #6550 * net: Introduced a `Socket#connecting` property. (Fedor Indutny) #6404 - Previously this information was only available as the undocumented, internal `_connecting` property. * process: Introduced `process.cpuUsage()`. (Patrick Mueller) #6157 * stream: `Writable#setDefaultEncoding()` now returns `this`. (Alexander Makarenko) #5040 * util: Two new additions to `util.inspect()`: - Added a `maxArrayLength` option to truncate the formatting of Arrays. (James M Snell) #6334 - This is set to `100` by default. - Added a `showProxy` option for formatting proxy intercepting handlers. (James M Snell) #6465 - Inspecting proxies is non-trivial and as such this is off by default. PR-URL: #6557

* assert: `deep{Strict}Equal()` now works correctly with circular references. (Rich Trott) #6432 * debugger: Arrays are now formatted correctly in the debugger repl. (cjihrig) #6448 * deps: Upgrade OpenSSL sources to 1.0.2h (Shigeki Ohtsu) #6550 - Please see our blog post for more info on the security contents of this release: - https://nodejs.org/en/blog/vulnerability/openssl-may-2016/ * net: Introduced a `Socket#connecting` property. (Fedor Indutny) #6404 - Previously this information was only available as the undocumented, internal `_connecting` property. * process: Introduced `process.cpuUsage()`. (Patrick Mueller) #6157 * stream: `Writable#setDefaultEncoding()` now returns `this`. (Alexander Makarenko) #5040 * util: Two new additions to `util.inspect()`: - Added a `maxArrayLength` option to truncate the formatting of Arrays. (James M Snell) #6334 - This is set to `100` by default. - Added a `showProxy` option for formatting proxy intercepting handlers. (James M Snell) #6465 - Inspecting proxies is non-trivial and as such this is off by default. PR-URL: #6557

This commit allows all array properties to be printed except for "length". Previously, this filter was applied by checking the type of each property. However, something changed in V8, and array elements started coming through as numeric strings, which stopped them from being displayed. Fixes: #6444 PR-URL: #6448 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>

This LTS release comes with 89 commits. This includes 46 commits that are docs related, 11 commits that are test related, 8 commits that are build related, and 4 commits that are benchmark related. Notable Changes: - debugger: - All properties of an array (aside from length) can now be printed in the repl (cjihrig) #6448 - npm: - Upgrade npm to 2.15.8 (Rebecca Turner) #7412 - stream: - Fix for a bug that became more prevalent with the stream changes that landed in v4.4.5. (Anna Henningsen) #7160 - V8: - Fix for a bug in crankshaft that was causing crashes on arm64 (Myles Borins) #7442 - Add missing classes to postmortem info such as JSMap and JSSet (evan.lucas) #3792

nodejs-github-bot added the test Issues and PRs related to the tests. label Apr 28, 2016

mscdex added debugger and removed test Issues and PRs related to the tests. labels Apr 28, 2016

mscdex mentioned this pull request Apr 28, 2016

bad tagging for test nodejs/github-bot#33

Closed

phillipj mentioned this pull request Apr 28, 2016

Fix for bad node core PR exclusive labels. nodejs/github-bot#34

Merged

jasnell reviewed Apr 28, 2016
View reviewed changes

cjihrig force-pushed the debugger branch from a225230 to d2eb935 Compare May 2, 2016 14:21

cjihrig merged commit d2eb935 into nodejs:master May 2, 2016

cjihrig deleted the debugger branch May 2, 2016 14:21

This was referenced May 4, 2016

Propose v6.1.0 wrong base please ignore #6556

Closed

Propose v6.1.0 (Openssl update) #6557

Merged

addaleax mentioned this pull request May 23, 2016

Array representation buggy #6926

Closed

MylesBorins added the land-on-v4.x label Jun 2, 2016

MylesBorins mentioned this pull request Jun 24, 2016

v4.4.7 proposal #7395

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

debugger: display array contents in repl #6448

debugger: display array contents in repl #6448

cjihrig commented Apr 28, 2016

addaleax commented Apr 28, 2016

jasnell commented Apr 28, 2016

cjihrig commented Apr 28, 2016

jasnell Apr 28, 2016

indutny commented Apr 28, 2016

addaleax commented Apr 28, 2016

cjihrig commented Apr 28, 2016

indutny commented Apr 28, 2016

jasnell commented Apr 29, 2016

debugger: display array contents in repl #6448

debugger: display array contents in repl #6448

Conversation

cjihrig commented Apr 28, 2016

Checklist

Affected core subsystem(s)

Description of change

addaleax commented Apr 28, 2016

jasnell commented Apr 28, 2016

cjihrig commented Apr 28, 2016

jasnell Apr 28, 2016

Choose a reason for hiding this comment

indutny commented Apr 28, 2016

addaleax commented Apr 28, 2016

cjihrig commented Apr 28, 2016

indutny commented Apr 28, 2016

jasnell commented Apr 29, 2016