search

servo / rust-mozjs

DEPRECATED - moved to servo/mozjs instead.
Mozilla Public License 2.0
293 stars 117 forks source link

Usable DataView typed array bindings #453

Open aditj opened 5 years ago

aditj commented 5 years ago

This PR tries to solve issue #406 The changes use the traits of a typed array but the APIs for Dataview object from mozjs are used. The changes have not been tested , any suggested tests/changes would be appreciated. :)

This change is Reviewable

jdm commented 5 years ago

Can you add to the existing tests in https://github.com/servo/rust-mozjs/blob/master/tests/typedarray.rs? You can use the example at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DataView as inspiration.

aditj commented 5 years ago

We can't use the TypedArrayElement and TypedArrayElementCreator traits since the functions of length_and_data and create_new would be different in case of a DataView.

aditj commented 5 years ago

We can't use the TypedArrayElement and TypedArrayElementCreator traits since the functions of length_and_data and create_new would be different in case of a DataView.

@jdm Help :face_with_head_bandage:
Is there a way I could overload functions of a trait (The same way I would overload functions in an OOP Language), or is there any other alternative?

jdm commented 5 years ago

Could you explain the problem that you are having? What is the code you are trying to write that is not working?

aditj commented 5 years ago

Could you explain the problem that you are having? What is the code you are trying to write that is not working?

See this comment

jdm commented 5 years ago

I think we will have to create a new DataView type similar to TypedArray which directly wraps these APIs and then add a macro like typedarray! which creates a rooted instance.

aditj commented 5 years ago

I think we will have to create a new DataView type similar to TypedArray which directly wraps these APIs and then add a macro like typedarray! which creates a rooted instance.

I'll try doing that :+1:

bors-servo commented 3 years ago

:umbrella: The latest upstream changes (presumably #541) made this pull request unmergeable. Please resolve the merge conflicts.