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

Allow arrays to pass through ports #597

Closed
mgold opened this issue Apr 25, 2014 · 12 comments · Fixed by #606
Closed

Allow arrays to pass through ports #597

mgold opened this issue Apr 25, 2014 · 12 comments · Fixed by #606

Comments

@mgold
Copy link
Contributor

mgold commented Apr 25, 2014

The groundwork seems to have been laid in #585, but it doesn't appear to be allowed yet. Also update the error message in ExtraChecks.hs.

@mgold
Copy link
Contributor Author

mgold commented Apr 30, 2014

If you point me in the right direction I can try to implement this myself.

@evancz
Copy link
Member

evancz commented Apr 30, 2014

This is how it was done for Json.Value. Check out those locations and find how it's done for _List and that should be pretty close.

Looking back on that code, it'd be cool if we could make it so Native.Json and Native.Array get loaded conditionally rather than every time.

@mgold
Copy link
Contributor Author

mgold commented Apr 30, 2014

Okay, thanks.

@xashili Why did you take out the toJSArray and fromJSArray functions? Without them I have to convert to a list first.

@mgold
Copy link
Contributor Author

mgold commented Apr 30, 2014

Here is what I've figured out. If we put back the toJSArray and fromJSArray functions, then I can just copy the code generation from _List. I tried to figure out how to convert Array->List->JSArray and it looked like a rabbit hole.

@xash
Copy link
Contributor

xash commented Apr 30, 2014

I opened a pull request with the functions, #603.

@evancz
Copy link
Member

evancz commented Apr 30, 2014

#603 is merged. My plan is to have this come with 0.13. I don't want to sit on 0.12.1 any longer!

@mgold
Copy link
Contributor Author

mgold commented May 1, 2014

If you like, but I think I've almost got it.

@evancz
Copy link
Member

evancz commented May 1, 2014

As long as I can publish tomorrow with confidence :)

mgold added a commit to mgold/Elm that referenced this issue May 1, 2014
@mgold
Copy link
Contributor Author

mgold commented May 1, 2014

Well... that's your call to make. I've tested incoming and outgoing and it seems to work, but I haven't run it through a proper test battery. The port code I just pushed is actually fairly simple. @xashili how much testing did you do for to/from JSArray?

@mgold
Copy link
Contributor Author

mgold commented May 1, 2014

Answer: enough.

Unless something jumps out at you I think we're good to merge.

@evancz
Copy link
Member

evancz commented May 1, 2014

Send the PR!

@mgold
Copy link
Contributor Author

mgold commented May 1, 2014

Oh, you can't merge from issues? Okay.

@mgold mgold mentioned this issue May 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants