Skip to content

Conversation

paf31
Copy link
Owner

@paf31 paf31 commented Dec 15, 2016

Fixes #15

@chexxor Would you mind taking a look over this?

@paf31 paf31 changed the title use generics-rep Use generics-rep Dec 15, 2016
@chexxor
Copy link

chexxor commented Dec 15, 2016

Requires 0.10.3 compiler? I'm getting an error on pulp test for the usage genericShow:

No type class instance was found for
                                     
    Data.Symbol.IsSymbol "TupleArray"

update: yeah, looks like compiling with 0.10.3 instead of 0.10.2 fixed that issue.

@paf31
Copy link
Owner Author

paf31 commented Dec 15, 2016

Yes, that's right, it needs the symbol deriving feature to derive Show.

@chexxor
Copy link

chexxor commented Dec 15, 2016

@paf31 - I'm not sure what exactly you wanted me to check for, so I looked for the bit I'm most interested in seeing fixed in this rewrite, which is adding the missing field name to the error.

It seems to work well in the success case on my one simple test, which is parsing a record, but the failure case still doesn't include the missing field's name. See my notes and test case in my branch, if you're curious: chexxor@21ad569

I have a suggested fix, but I'm not familiar with the expected return value and errors of the genericDecodeFieldsField instance. Following is its description.

y, the Foreign value at the expected field name, is 'undefined'. read undefined throws error: (TypeMismatch "String" "Undefined"). I'd like to add name, the field's name, to this error. By doing this, would this function's error message still describe the correct error in all cases of this function args?

@chexxor
Copy link

chexxor commented Dec 15, 2016

Using your suggestion, I added a fix to include the field name. It's in this commit: chexxor@c2cf778#diff-f4f396b6bb00857e2d3c4a669f217589R172

The error message is given a field name like: (Left (NonEmptyList (NonEmpty (ErrorAtIndex 0 (ErrorAtProperty "name" (TypeMismatch "String" "Undefined"))) Nil)))

You can pull that fix into your current branch, if you'd like.

@paf31 paf31 merged commit 4016f01 into master Dec 16, 2016
@paf31 paf31 deleted the generics-rep branch December 16, 2016 02:15
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.

2 participants