add sum encoding and handling for sum types of unary constructors #26

justinwoo · 2017-06-24T17:28:06Z

fixes #25

uses some flags to determine casing and then uses these flags in encoding/decoding

updated excerpt:

testOption (makeCasingOptions toUpper) "\"FrIkAnDeL\"" Frikandel
testOption (makeCasingOptions toLower) "\"FrIkAnDeL\"" Frikandel

justinwoo · 2017-06-26T19:06:51Z

Alternatively, I wrote this up where I've defined a function to provide instances specifically for sum types of unary constructors: justinwoo@179831c. Should I squash this here instead of using the conditional version in this branch?

I'd really like to also provide some Fail constrained instances for warnings and explanations, but I don't know how to do that without having overlapped instances warnings in usage sites.

paf31 · 2017-06-27T03:28:13Z

What do you think about making a separate generic decoding/encoding function for enums like this? That way we could actually enforce using the type system that they only accept "enum types".

justinwoo · 2017-06-27T15:47:02Z

I squashed my separate genericDecodeUnaryConstructors and genericEncodeUnaryConstructors functions into this one. Like my comment before though, while it doesn't match any instances for reps that aren't Sum or Constructor NoArg, I can't add Fail constraints without having overlapping instance warnings, so it could be a little confusing to use. But that fail constraint can wait until something with instance chains gets figured out.

paf31 · 2017-06-27T16:10:53Z

src/Data/Foreign/Generic/SumEncoding.purs

@@ -0,0 +1,62 @@
+module SumEncoding where


Module name is incomplete here.

paf31 · 2017-06-27T16:12:15Z

src/Data/Foreign/Generic/SumEncoding.purs

+  => GenericDecodeUnarySum (Constructor name NoArguments) where
+  decodeSum {constructorTagTransform} f = do
+    tag <- constructorTagTransform <$> readString f
+    unless (constructorTagTransform tag == ctorName) $


You apply constructorTagTransform twice.

paf31 · 2017-06-27T16:12:30Z

src/Data/Foreign/Generic/SumEncoding.purs

+      fail (ForeignError ("Expected " <> show ctorName <> " tag for unary constructor literal " <> ctorName))
+    pure $ Constructor NoArguments
+    where
+      ctorName = constructorTagTransform $ reflectSymbol (SProxy :: SProxy name)


paf31 · 2017-06-27T16:15:32Z

Just add a Fail instance for Product and Rec?

Also, can we use the term "enum"? I think it's pretty common for this type of data.

justinwoo · 2017-06-27T16:52:07Z

Updated. I added the instances, though, I haven't been able to come up with ways to reach those without a constructor constraint.

paf31 · 2017-06-27T16:59:00Z

Yes, you would need to write several instances for Constructor, but they won't overlap.

paf31 · 2017-06-27T17:00:15Z

src/Data/Foreign/Generic/Enum.purs

+  encodeEnum _ _ = unsafeCrashWith "unreachable encodeEnum was reached."
+
+instance recGenericEncodeEnum
+  :: Fail """


Maybe pull the error messages out into a single common type synonym?

But I wouldn't be able to change the wording right? I do want to put what kind of type they were trying to plug in here for a more relevant error message

Why not just say "doesn't work with constructors with arguments" though? Records and products are both types of arguments. Do you not think it's clear enough?

I guess that's fine. I'll switch them over then

paf31 · 2017-06-27T17:00:35Z

Another option is to use TypeEquals.

paf31 · 2017-06-27T17:15:35Z

src/Data/Foreign/Generic/Types.purs

  { sumEncoding :: SumEncoding
  , unwrapSingleConstructors :: Boolean
  , unwrapSingleArguments :: Boolean
+  , constructorTagTransform :: String -> String


Can this go into TaggedObject?

paf31 · 2017-06-27T17:15:47Z

src/Data/Foreign/Generic/Enum.purs

+  decodeEnum opts f = Inl <$> decodeEnum opts f <|> Inr <$> decodeEnum opts f
+
+instance ctorNoArgsGenericDecodeEnum
+  :: (IsSymbol name)


Parens are redundant here.

justinwoo · 2017-06-27T18:22:32Z

Updated this. Like we found on Slack, I couldn't find out how to make a synonym actually work at the usage site of the Fail instances, and then I moved the tag transform into SumEncoding.

paf31 · 2017-06-27T18:32:51Z

src/Data/Foreign/Generic/Enum.purs

+
+instance ctorArgumentGenericEncodeEnum
+  :: Fail """
+    genericEncodeEnum cannot be used with constructors with arguments, as it is meant to work with string literal "Enums", Sum types of no-argument constructors.


I would still trim this down a bit and put the stuff about enums into the documentation for the classes.

justinwoo · 2017-06-27T19:20:34Z

Wrote up some docstrings with a small code example in each top level method, with just short descriptions of the typeclasses and the instance errors.

paf31 · 2017-07-06T01:45:03Z

src/Data/Foreign/Generic/Enum.purs

+  => GenericDecodeEnum (Constructor name NoArguments) where
+  decodeEnum {sumEncoding: TaggedObject {constructorTagTransform}} f = do
+    tag <- readString f
+    unless (constructorTagTransform tag == ctorName) $


Wrong way around here, surely?

ah, yeah. the ctorname is already transformed, but i guess transforming the tag itself is kinda wonky...

paf31 · 2017-07-06T01:45:49Z

src/Data/Foreign/Generic/Enum.purs

+      ctorName = constructorTagTransform $ reflectSymbol (SProxy :: SProxy name)
+
+instance ctorArgumentGenericDecodeEnum
+  :: Fail """


Please put these Fail constraints on one line and just use a single error message in a type declaration.

paf31 · 2017-07-06T01:46:31Z

src/Data/Foreign/Generic/Enum.purs

+  :: forall a rep
+   . Generic a rep
+  => GenericDecodeEnum rep
+  => Options


If we're only using constructorTagTransform here, let's only require a record with that one field.

We can use row polymorphism if we want to support more fields.

paf31 · 2017-07-06T01:47:41Z

Sorry for the delay, I have a few more comments.

justinwoo · 2017-07-06T22:11:57Z

I updated this, but I still cannot get the type alias string to properly work on my machine for the Fail constraint. I will get "No type class instance was found for Fail GenericEncodeDecodeFailMessage"...

paf31 · 2017-07-06T23:04:15Z

Ok, I'd prefer one of the following then:

No Fail instances for now
Just repeating the same error message in the code, but on single lines.
Adding a helper class with a single instance so that we can put the error in a single place.

Adds constructor tag transform so that users can customize the tag as needed

justinwoo · 2017-07-06T23:06:45Z

Pushed an update to do no 2. I'll try to remember to come back to this when we get purescript/purescript#2972 fixed.

paf31 · 2017-07-06T23:08:37Z

Great, thanks for all the work on this!

paf31 · 2017-07-06T23:09:44Z

bower.json

-    "purescript-symbols": "^3.0.0"
+    "purescript-proxy": "^2.1.0",
+    "purescript-symbols": "^3.0.0",
+    "purescript-strings": "^3.2.1"


It doesn't look like you're using this any more.

justinwoo · 2017-07-06T23:11:47Z

Thanks!!! Now I need to remember to update my howto guides and start making some noise about enum sum type support 😄

justinwoo force-pushed the unary-constructor-literal branch 3 times, most recently from db4dee6 to 8a41b83 Compare June 26, 2017 17:24

justinwoo force-pushed the unary-constructor-literal branch from 8a41b83 to 73155eb Compare June 27, 2017 15:44

paf31 reviewed Jun 27, 2017

View reviewed changes

justinwoo force-pushed the unary-constructor-literal branch from 73155eb to 67c61e5 Compare June 27, 2017 16:49

justinwoo force-pushed the unary-constructor-literal branch from 67c61e5 to 321dd09 Compare June 27, 2017 16:53

paf31 reviewed Jun 27, 2017

View reviewed changes

justinwoo force-pushed the unary-constructor-literal branch 2 times, most recently from 8bd5849 to 78bf86d Compare June 27, 2017 17:07

paf31 reviewed Jun 27, 2017

View reviewed changes

justinwoo force-pushed the unary-constructor-literal branch 2 times, most recently from f8c910b to eef91d7 Compare June 27, 2017 17:41

paf31 reviewed Jun 27, 2017

View reviewed changes

justinwoo force-pushed the unary-constructor-literal branch from 07e68b7 to 0dc8f05 Compare June 27, 2017 18:49

paf31 reviewed Jul 6, 2017

View reviewed changes

justinwoo force-pushed the unary-constructor-literal branch from 0dc8f05 to b69b255 Compare July 6, 2017 22:10

Provide enum/unary constructor sum type functions for use in instances

e42b38a

Adds constructor tag transform so that users can customize the tag as needed

justinwoo force-pushed the unary-constructor-literal branch from b69b255 to e42b38a Compare July 6, 2017 23:05

paf31 merged commit 34332d8 into paf31:master Jul 6, 2017

paf31 reviewed Jul 6, 2017

View reviewed changes

add sum encoding and handling for sum types of unary constructors #26

add sum encoding and handling for sum types of unary constructors #26

Uh oh!

Conversation

justinwoo commented Jun 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinwoo commented Jun 26, 2017

Uh oh!

paf31 commented Jun 27, 2017

Uh oh!

justinwoo commented Jun 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paf31 commented Jun 27, 2017

Uh oh!

justinwoo commented Jun 27, 2017

Uh oh!

paf31 commented Jun 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paf31 commented Jun 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinwoo commented Jun 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinwoo commented Jun 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paf31 commented Jul 6, 2017

Uh oh!

justinwoo commented Jul 6, 2017

Uh oh!

paf31 commented Jul 6, 2017

Uh oh!

justinwoo commented Jul 6, 2017

Uh oh!

paf31 commented Jul 6, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinwoo commented Jul 6, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

justinwoo commented Jun 24, 2017 •

edited

Loading