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

The behavior of Buffer#compare doesn't match its documentation #17618

Closed
benblank opened this issue Dec 11, 2017 · 3 comments
Closed

The behavior of Buffer#compare doesn't match its documentation #17618

benblank opened this issue Dec 11, 2017 · 3 comments
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.

Comments

@benblank
Copy link

In the documentation for Buffer#compare, it's stated that the targetEnd, sourceStart, and sourceEnd parameters are each "ignored when targetStart is undefined". However, I do not observe that behavior on versions 6.12.2, 8.9.3, or 9.2.1. Instead, the following code fails the third assertion:

const assert = require('assert');

const foo = Buffer.from([0, 1, 2, 3]);
const bar = Buffer.from([1, 2, 3, 4]);

const LESS_THAN = -1;
const EQUAL = 0;

assert.equal(foo.compare(bar), LESS_THAN, 'Without indices, foo is less.');
assert.equal(foo.compare(bar, 0, 3, 1, 4), EQUAL, 'With indices which compare only the 1..3 from each buffer, foo and bar are equal.');
assert.equal(foo.compare(bar, undefined, 3, 1, 4), LESS_THAN, 'With targetStart undefined, all other indices should be ignored.');

Although both behaviors (ignoring or not ignoring subsequent parameters) seem reasonable, it looks like the discrepancy has always existed (see 473f086, which introduced the feature), so it probably makes more sense to change the docs.

@vsemozhetbyt vsemozhetbyt added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Dec 11, 2017
@BridgeAR
Copy link
Member

As far as I see it the "ignored" part is just interpreted differently. By setting the value to undefined, it is normal that it will fall back to the default value and not have any further impact on other arguments. So the behavior is correct as far as I see it. The wording might be improved though.

@BridgeAR BridgeAR added the good first issue Issues that are suitable for first-time contributors. label Dec 11, 2017
@Jedeu
Copy link

Jedeu commented Dec 12, 2017

Hey I'd like to try and tackle this one please! Will look into it after work

maclover7 referenced this issue Dec 31, 2017
PR-URL: #17796
Reviewed-By: Anatoli Papirovski <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Jon Moss <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
@maclover7
Copy link
Contributor

Fixed via #17796

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

No branches or pull requests

5 participants