Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Use HugeInt for Integer#22

Merged
garyb merged 4 commits into
slamdata:masterfrom
garyb:hugeint
Jun 28, 2017
Merged

Use HugeInt for Integer#22
garyb merged 4 commits into
slamdata:masterfrom
garyb:hugeint

Conversation

@garyb
Copy link
Copy Markdown
Member

@garyb garyb commented Jun 28, 2017

Resolves #21

@garyb garyb requested a review from kritzcreek June 28, 2017 11:52
A.some parseDigit
<#> F.foldl (\a i → a * 10 + i) 0
⇒ Semiring n
⇒ n
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could use some documentation. I guess one of these arguments is the base?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added some explanation, how does it look now?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

Null → JS.jsonNull
Boolean b → encodeJson b
Integer i → encodeJson i
Integer i → encodeJson $ HN.toNumber $ HI.toHugeNum i -- TODO: bug in HI.toInt
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have a ticket for that bug?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We do now 😄 purescript-contrib/purescript-precise#10

My WIP stuff on -precise will probably fix it anyway, but you're right it's worth having the issue.

@garyb garyb merged commit d2d8bbd into slamdata:master Jun 28, 2017
@garyb garyb deleted the hugeint branch June 28, 2017 15:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants