Skip to content

Conversation

@garyb
Copy link
Member

@garyb garyb commented Mar 25, 2017

No description provided.

@sudhirvkumar
Copy link

@garyb this PR can also be merged?

@garyb
Copy link
Member Author

garyb commented Mar 28, 2017

Not quite, there's some changes to come yet.

@garyb garyb force-pushed the ps-0.11 branch 3 times, most recently from c44fe5d to bae0da0 Compare March 29, 2017 00:03
@garyb
Copy link
Member Author

garyb commented Mar 29, 2017

@paf31 Ok, just pushed a pretty big update...

  • Dropped IsForeign as discussed variously
  • Removed the newtypes for null / undefined / etc in favour of just providing functions that can be specific about what they read instead - the newtypes only existed to guide the chosen IsForeign implementation before.
  • Removed parseJSON since that's highly misleading about what this library is for (it exists in altered from in support code for the examples though, as it makes them easier to read rather than having the example values being parsed in a separate JS file in each case)
  • Added a new !! operator that allows chaining of property reads, so you can do value ! prop1 !! prop2
  • Updated all of the examples accordingly

ixKleisli :: forall i. Index i => F Foreign -> i -> F Foreign
ixKleisli f i = f >>= (_ ! i)

infixl 9 ixKleisli as !!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thought: we could overload ! using a type class here.

Copy link
Contributor

@paf31 paf31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I will probably continue to maintain some of the JSON features in the more opinionated foreign-generics but I think these are the right choices for this library.

@garyb garyb merged commit 47fb251 into master Mar 29, 2017
@garyb garyb deleted the ps-0.11 branch March 29, 2017 00:39
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 this pull request may close these issues.

4 participants