Add an optional maximum length argument to Buffer#write #243

Closed
bzmeteorite opened this Issue Aug 18, 2010 · 13 comments

4 participants

@bzmeteorite

So here's the little reminder for you ryah.

An optional argument to Buffer#write that accepts the maximum length in bytes to write from the string. Especially useful for when you're writing UTF-8 and you want to cut the string off prematurely, but don't want to break any multibyte characters in half, but should be implemented no matter the encoding (Buffer#write has this functionality if the buffer isn't large enough).

@bnoordhuis
Node.js Foundation member

I think this works as expected now?

> b = Buffer(3); b.fill(0); b.write('ëë', 0, 'utf-8'); b;
<Buffer c3 ab 00>

If that isn't what you mean, can you post a test case and the expected output?

@koichik

Probably, @bzmeteorite's request is to add length argument to Buffer.write().
Like this:

> b = Buffer(4); b.fill(0); b.write('ëë', 0, 3, 'utf-8'); b;
<Buffer c3 ab 00 00>
@bzmeteorite

It's been awhile and I forget the exact details (I wish I had written a test case describing expected functionality, but this issue was mostly a reminder for ryah who said to post an issue, but he never got around to). But I believe what @koichik describes is most likely what I had meant. I believe I found a workaround that implemented this same functionality as well.

I can likely find more info later this week and post it here if anyone is still interested.

@sixtus

See #1361 for a use case: Writing strings that do not use the 0x0 termination pattern.

@koichik koichik added a commit to koichik/node that referenced this issue Jul 21, 2011
@koichik koichik Add an optional maxLength argument to Buffer.write
Fixes #243.
Fixes #1361.
f2ee5aa
@koichik

Please review.

@bnoordhuis
Node.js Foundation member

@koichik - I hacked test-buffer.js into an ersatz benchmark (diff) and there's a huge drop in performance.

Before:

real    0m15.674s
user    0m15.610s
sys     0m0.120s

After:

real    0m24.308s
user    0m24.010s
sys     0m0.190s
@sixtus

Ouch, that's a lot. I guess I can work around it, but please do add a big fat warning to the API doc.

@koichik

Oops! This is slower than Utf8Length()!! orz

master:

real    0m9.768s
user    0m10.440s
sys     0m0.420s

f2ee5aa:

real    0m13.870s
user    0m14.680s
sys     0m0.660s

tuned:

real    0m9.987s
user    0m10.630s
sys     0m0.460s
@koichik koichik added a commit to koichik/node that referenced this issue Jul 22, 2011
koichik Add an optional length argument to Buffer.write()
Fixes #243.
Fixes #1361.
bc42d04
@koichik

Please review (2).

@bnoordhuis
Node.js Foundation member

@koichik bc42d04 fails to apply on master.

$ curl -s https://github.com/koichik/node/commit/bc42d04.patch | git am --whitespace=fix
Applying: Add an optional length argument to Buffer.write()
error: patch failed: lib/buffer.js:92
error: lib/buffer.js: patch does not apply
Patch failed at 0001 Add an optional length argument to Buffer.write()
@koichik

I hadn't squashed commits, please apply f2ee5aa first.

@bnoordhuis
Node.js Foundation member

Thanks, much better. Still 2% slower but I suppose that's acceptable if it solves a real problem.

@koichik

Thanks!

@koichik koichik closed this in 50e147b Jul 23, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment