buffer: add tests for Buffer.prototype.inspect() and refactor by Trott · Pull Request #11600 · nodejs/node · GitHub

First commit: lib/buffer.js defines Buffer.prototype.inspect() to override how buffers are presented by util.inspect(). Add basic tests for it. (This test completes code coverage for the function. Previously, the condition where the this.length > 0 was always true during test runs. This adds a test where it will be false.)

Second commit: Now that tests are in place, refactor. Replace toString().match().join() with toString().replace().trim(). This enables the elimination of a length check becuase replace() will return empty string if Buffer is empty whereas match() returns null. (To be clear, I don't think the .length check is a performance liability or anything like that. I just think that fewer logic branches usually makes for code that is easier to understand.)

Checklist

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

Affected core subsystem(s)

buffer test

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

nit: Template string here? ;)

lib/buffer.js defines Buffer.prototype.inspect() to override how buffers are presented by util.inspect(). Add basic tests for it.

Replace toString().match().join() with toString().replace().trim(). This enables the elimination of a length check becuase replace() will return empty string if Buffer is empty whereas match() returns null.

Nit addressed. New CI: https://ci.nodejs.org/job/node-test-pull-request/6649/

lib/buffer.js defines Buffer.prototype.inspect() to override how buffers are presented by util.inspect(). Add basic tests for it. PR-URL: #11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Replace toString().match().join() with toString().replace().trim(). This enables the elimination of a length check becuase replace() will return empty string if Buffer is empty whereas match() returns null. PR-URL: #11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Landed in bb5c84a...4c05d6a

lib/buffer.js defines Buffer.prototype.inspect() to override how buffers are presented by util.inspect(). Add basic tests for it. PR-URL: #11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Replace toString().match().join() with toString().replace().trim(). This enables the elimination of a length check becuase replace() will return empty string if Buffer is empty whereas match() returns null. PR-URL: #11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

lib/buffer.js defines Buffer.prototype.inspect() to override how buffers are presented by util.inspect(). Add basic tests for it. PR-URL: #11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Replace toString().match().join() with toString().replace().trim(). This enables the elimination of a length check becuase replace() will return empty string if Buffer is empty whereas match() returns null. PR-URL: #11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

lib/buffer.js defines Buffer.prototype.inspect() to override how buffers are presented by util.inspect(). Add basic tests for it. PR-URL: #11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Replace toString().match().join() with toString().replace().trim(). This enables the elimination of a length check becuase replace() will return empty string if Buffer is empty whereas match() returns null. PR-URL: #11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

lib/buffer.js defines Buffer.prototype.inspect() to override how buffers are presented by util.inspect(). Add basic tests for it. PR-URL: nodejs/node#11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Replace toString().match().join() with toString().replace().trim(). This enables the elimination of a length check becuase replace() will return empty string if Buffer is empty whereas match() returns null. PR-URL: nodejs/node#11600 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Trott added buffer Issues and PRs related to the buffer subsystem. test Issues and PRs related to the tests. labels Feb 28, 2017

nodejs-github-bot added the buffer Issues and PRs related to the buffer subsystem. label Feb 28, 2017

addaleax approved these changes Feb 28, 2017

View reviewed changes

jasnell approved these changes Feb 28, 2017

View reviewed changes

cjihrig approved these changes Mar 1, 2017

View reviewed changes

hiroppy approved these changes Mar 1, 2017

View reviewed changes

Trott added 2 commits March 1, 2017 19:11

test: add test-buffer-prototype-inspect

48583c4

lib/buffer.js defines Buffer.prototype.inspect() to override how buffers are presented by util.inspect(). Add basic tests for it.

buffer: refactor Buffer.prototype.inspect()

03b1a2d

Replace toString().match().join() with toString().replace().trim(). This enables the elimination of a length check becuase replace() will return empty string if Buffer is empty whereas match() returns null.

Trott force-pushed the buffer-inspect branch from a98b2dd to 03b1a2d Compare March 2, 2017 03:24

lpinca approved these changes Mar 2, 2017

View reviewed changes

jasnell closed this Mar 2, 2017

evanlucas mentioned this pull request Mar 8, 2017

v7.7.2 proposal #11745

Merged

MylesBorins added the land-on-v6.x label Apr 17, 2017

MylesBorins mentioned this pull request Apr 19, 2017

v6.10.3 proposal #12498

Merged

Trott deleted the buffer-inspect branch January 13, 2022 22:34

buffer: add tests for Buffer.prototype.inspect() and refactor by Trott · Pull Request #11600 · nodejs/node · GitHub

Uh oh!

Uh oh!

buffer: add tests for Buffer.prototype.inspect() and refactor - #11600

buffer: add tests for Buffer.prototype.inspect() and refactor#11600
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:buffer-inspect

Trott commented Feb 28, 2017

Uh oh!

Trott commented Feb 28, 2017

Uh oh!

addaleax Feb 28, 2017

Uh oh!

Trott commented Mar 2, 2017

Uh oh!

jasnell commented Mar 2, 2017 •
edited by MylesBorins
Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

buffer: add tests for Buffer.prototype.inspect() and refactor by Trott · Pull Request #11600 · nodejs/node · GitHub

Uh oh!

Uh oh!

buffer: add tests for Buffer.prototype.inspect() and refactor - #11600

buffer: add tests for Buffer.prototype.inspect() and refactor#11600Trott wants to merge 2 commits intonodejs:masternodejs/node:masterfrom Trott:buffer-inspectTrott/io.js:buffer-inspectCopy head branch name to clipboard

Conversation

Trott commented Feb 28, 2017

Checklist

Affected core subsystem(s)

Uh oh!

Trott commented Feb 28, 2017

Uh oh!

addaleax Feb 28, 2017

Choose a reason for hiding this comment

Uh oh!

Trott commented Mar 2, 2017

Uh oh!

jasnell commented Mar 2, 2017 • edited by MylesBorins Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

buffer: add tests for Buffer.prototype.inspect() and refactor#11600
Trott wants to merge 2 commits into
nodejs:masterfrom
Trott:buffer-inspect

jasnell commented Mar 2, 2017 •
edited by MylesBorins
Loading