TypedArray::create API accept length or slice

simon-whitehead commented 7 years ago

Issue: https://github.com/servo/rust-mozjs/issues/336

Changes TypedArray::create signature to accept a new CreateWith argument in place of the length and data arguments.

Includes similar tests to what was already there using the new enum.

Some notes:

My first Servo PR. Yay!
Rustfmt seems to have made some changes. Not sure if thats a problem. The license /* */ block comments are now // single line comments. And some other multi-line code has been changed to single line code. If this is an issue let me know and I'll disable rustfmt and revert those changes back.
I didn't bump the crate version. I am not sure about the process around that and looking at other PRs I've noticed none of those bumped the version number either.
I don't yet understand how this applies to Servo. From what I can see everything uses the macro and not TypedArray::create directly. I may have missed something though.

Feedback welcome.

@jdm - tagging you since you opened the issue (I hope thats okay?)

This change is

simon-whitehead commented 7 years ago

Also one other potential noteworthy change.

The original tests allowed different length and data length inputs. For example, create with a length of 10 but a slice with length 5 would result in a 0 padded slice with a length of 10.

The issue itself seems to suggest this shouldn't be possible and so after the change was made, this situation wasn't possible and I have updated the existing tests to reflect that.

jdm commented 7 years ago

There are uses of ArrayBuffer::create and Uint8ClampedArray::Create and in Servo right now, and https://github.com/servo/servo/pull/15427 adds more typed array creation.

jdm commented 7 years ago

This looks great! Thank you for working on this! @bors-servo: r+

bors-servo commented 7 years ago

:pushpin: Commit 86f06e4 has been approved by jdm

bors-servo commented 7 years ago

:hourglass: Testing commit 86f06e4 with merge 901b1b5...

bors-servo commented 7 years ago

:sunny: Test successful - status-appveyor, status-travis Approved by: jdm Pushing 901b1b548d07170ecc7aa36da6ddc0f14b8cbac7 to master...

simon-whitehead commented 7 years ago

No problem! Thanks for merging so quickly! :smiley:

(Ps: this was such a painless process. I will look out for more issues I can help with :+1:)

servo / rust-mozjs

TypedArray::create API accept length or slice #337