purescript-node / purescript-node-fs

Node.js file I/O for purescript
MIT License
33 stars 34 forks source link

additional Int53 api using Number #37 #38

Closed grmble closed 6 years ago

grmble commented 6 years ago

This would be one suggestion.

hdgarrood commented 6 years ago

Yeah, as it happens I’ve never liked the “int53” name, as to me that name ought to imply proper twos-complement ints with 53 bits, not floats which happen to behave like integers as long as you stay below 2^53. I think Number is indeed the right choice here.

I think the type synonym is also a bit misleading though, it would be better to just use Number.

A “large” suffix to differentiate these from the normal versions sounds sensible to me.

hdgarrood commented 6 years ago

Rather than having two copies of the implementation for each function, though, I’d rather just import each once and implement the old ones in terms of the new ones.

grmble commented 6 years ago

How about this version?

I made all the arguments Number this time. If you use this API, you will probably do some math with positions and byte counts. It's easier to use if everything is the same type.

I just noticed Atom reformatted all the changed import lines. Will fix that in the next round

grmble commented 6 years ago

Thinking about this some more, it would probably be better to leave everything except the file position an Int. You might have to add the byte count to your position, but Int -> Number is easy. Number -> Int is the troublesome part, and if you need to calculate buffer offsets you need an Int.

grmble commented 6 years ago

No idea what the travis problem is. I just tried the exact commands from the travis config

grmble commented 6 years ago

Ah! Travis is having issues. https://www.traviscistatus.com/

I'll trigger a rebuild once Travis service continues