Skip to content

Various improvements from the NoRedInk/ branch#2

Merged
supermario merged 20 commits intosupermario:masterfrom
NoRedInk:master
Dec 23, 2017
Merged

Various improvements from the NoRedInk/ branch#2
supermario merged 20 commits intosupermario:masterfrom
NoRedInk:master

Conversation

@jwoudenberg
Copy link

As discussed previously @supermario, these are some changes we added on our fork of elm-export. We'd love to combine these all in one place.

  • Fixes for decoder/encoder generation of newtype and unit types.
  • ElmType instance for servant Headers.

ehamberg and others added 20 commits June 29, 2017 21:44
Like all the cool kids are doing these days. :-)
It's unfortunate that we have to add this instance here,
but there are like three projects at play.

It doesn't make sense to push this into `servant`,
it almost surely wouldn't be accepted.

It doesn't make sense to push this into `servant-elm`.
That would be the ideal place for it,
but it would still be an orphan there.
If we're going to end up with an orhpan, why bother pushing it there?
The generated decoders for what were originally Haskell new types were
invalid Elm code. This commit fixes that. The newly produced Elm code
might not be the most idiomatic but it gets the job done.

Further improvements are definitely possible!
The previous commit fixed newtypes, but broke unit types:

data Unit = Unit

This adds a test case to demonstrate the fact. Writing a fix will be the
next step.
The useless encoder test is currently failing. Encoding of this case is
not compatible with Aeson's behaviour.
The Wrapper type is a simple Int wrapped in a newtype. Of the tests
added, the encoder case is failing because it generates Elm code that
does not compile.
@supermario
Copy link
Owner

Nice, thanks @jwoudenberg! I'll work through this week. I see you've also done a conversion to hpack 👍

@jwoudenberg
Copy link
Author

The conversion of hpack comes from Kris' commits (from the devel branch).

@supermario
Copy link
Owner

Ah. 😅

@supermario
Copy link
Owner

supermario commented Dec 23, 2017

Ok - I'm going to merge this in and then work on updating the changelog and reconciling a few things, including the removal of cb1966c due to issues documented in krisajenkins#48.

@supermario supermario merged commit 5a4ed59 into supermario:master Dec 23, 2017
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.

5 participants