purescript-node / purescript-node-fs

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

asynchronous fdOpen, fdRead, etc #21

Closed timbod7 closed 8 years ago

timbod7 commented 8 years ago

In order to implement this: https://github.com/purescript-node/purescript-node-fs-aff/issues/7, I think these functions:

foreign import fdOpen :: forall eff.
   FilePath -> FileFlags -> Maybe FileMode -> Callback eff FileDescriptor -> Eff (fs :: FS | eff) Unit

foreign import fdRead :: forall eff.
   FileDescriptor -> Buffer -> BufferOffset -> BufferLength -> Maybe FilePosition -> Callback eff ByteCount -> Eff (fs :: FS | eff) Unit

foreign import fdNext :: forall eff.
   FileDescriptor -> Buffer -> Callback eff ByteCount -> Eff (fs :: FS | eff) Unit

foreign import fdWrite :: forall eff.
   FileDescriptor -> Buffer -> BufferOffset -> BufferLength -> Callback eff ByteCount -> Eff (fs :: FS | eff) Unit

foreign import fdAppend :: forall eff.
   FileDescriptor -> Buffer -> Callback eff ByteCount -> Eff (fs :: FS | eff) Unit

foreign import fdClose :: forall opts eff.
   FileDescriptor -> Callback eff Unit -> Eff (fs :: FS | eff) Unit

need to be added to Node.FS.Async. Does that sound correct? Do the above types look correct?

hdgarrood commented 8 years ago

Yep, sounds correct. I guess the relevant data and type declarations would need to move out of Node.FS.Sync as well, possibly into Node.FS?

timbod7 commented 8 years ago

Yes - I thought that too. So a pull request would be ok?

On Mon, Nov 9, 2015 at 10:40 PM, Harry Garrood notifications@github.com wrote:

Yep, sounds correct. I guess the relevant data and type declarations would need to move out of Node.FS.Sync as well, possibly into Node.FS?

— Reply to this email directly or view it on GitHub https://github.com/purescript-node/purescript-node-fs/issues/21#issuecomment-155040061 .

hdgarrood commented 8 years ago

Yes :) If you're planning to use these functions in some application, what would be really helpful is if you tried using these functions in this application before sending it (bower link might be helpful for this). Nothing fancy - I'm just thinking something like calling them once and checking nothing obviously wrong happens.

timbod7 commented 8 years ago

What's the relationship between FS:

https://github.com/purescript-node/purescript-node-fs/blob/master/src/Node/FS.purs#L8

and BufferWrite

https://github.com/purescript-node/purescript-node-buffer/blob/master/src/Node/Buffer.purs#L46

? It seems like the functions in this package mutate buffers even though they don't declare a BufferWrite effect. (eg fdRead).

hdgarrood commented 8 years ago

Um, don't know. It does seem like perhaps fdRead should have a BufferWrite effect in addition to the ones it already has, yes.

hdgarrood commented 8 years ago

Ok I was just playing around with purescript-node-buffer and it seems like it has been broken for quite a long time. The functions in Node.Buffer which are typed as returning Eff actions — that is, write, writeString, and fill — don't actually return Eff actions, and I don't think the current implementation can work correctly under any circumstances. See this code, for example.

hdgarrood commented 8 years ago

I also think the API of node-buffer is broken. As far as I can tell, there should be an effect for reading or writing to buffers. Consider this program:

module Main where

import Prelude
import qualified Node.Buffer as B
import Control.Monad.Eff.Console (log)

main = do
  let buf = B.fromArray [0]
  let x = B.read B.UInt8 0 buf
  B.write B.UInt8 1 0 buf
  let y = B.read B.UInt8 0 buf

  log ("x = " ++ show x)
  log ("y = " ++ show y)
hdgarrood commented 8 years ago

Basically what I'm saying is wait until the next release of node-buffer, which will probably be very soon and will probably fix all of these issues.

timbod7 commented 8 years ago

OK. This all seems to touch more modules and be a little harder that I was hoping. I'm really after two functions to support incremental file processing in the Aff monad, something like:

type Reader eff = {
   read :: Integer -> Aff eff Buffer,
   close :: Aff eff Unit
}

type Writer eff = {
   write :: Buffer -> Aff eff Unit,
   close :: Aff eff Unit
}

readFile :: forall eff. FilePath -> Aff (fs :: F.FS | eff) (Reader (fs :: F.FS | eff))
writeFile :: forall eff. FilePath -> Aff (fs :: F.FS | eff) (Writer (fs :: F.FS | eff))
hdgarrood commented 8 years ago

In retrospect, the things I mentioned aren't really relevant to this issue, I should probably have just left it at "node-buffer is currently broken to the point where it's not really possible to do anything useful with buffers, so we need to fix it first". Or were the other modules you are referring to a different set?

timbod7 commented 8 years ago

"node-buffer is currently broken to the point where it's not really possible to do anything useful with buffers

Does that mean the the existing fdRead function is not useful?

hdgarrood commented 8 years ago

Yes, but only until node-buffer is updated, and I do have a PR open now which fixes all the issues I've identified.

timbod7 commented 8 years ago

Opened a PR: https://github.com/purescript-node/purescript-node-fs/pull/22

hdgarrood commented 8 years ago

Resolved by #22