silkapp / rest

Packages for defining APIs, running them, generating client code and documentation.
http://silkapp.github.io/rest
390 stars 52 forks source link

rest-gen refactorings and extensions #133

Open bergmark opened 9 years ago

bergmark commented 9 years ago

This code was pretty messy and I tried to clean it up a bit while I was at it, comments on the API welcome.

@octopuscabbage does this work the way you need it to?

Related: #116, #126, #122

hesselink commented 9 years ago

Why is the post processing function necessary? I'd rather have a function that doesn't write to a file so you can do the post processing externally. This seems the wrong way around.

bergmark commented 9 years ago

I wanted to make sure we can still run the main function as a complete executable. With this you can let rest-gen handle the output location and write the files instead of users having to duplicate the logic for everything after the post processing step (e.g. write the file to the correct location, print status messages, call exitSuccess).

hesselink commented 9 years ago

Hmm, I see. That use case makes sense. I'd like it more if the normal thing was writeToFile . generateCode and we exposed also those two pieces, but if that's not possible easily, this is probably fine.

bergmark commented 9 years ago

I ended up cleaning some more (and fixing rest-happstack and rest-example). I consider myself done now. Mind reviewing again @hesselink?

hesselink commented 9 years ago

Looks good! I'm still not super happy with the change to the signature of generate. We could of course keep it backwards compatible by adding a new function generateWithPostProcess or something. Then we wouldn't break as much.

Have you thought about the possibility of providing all the building blocks instead so you can do something like writeToFileAsNormal . transform . generateToString instead of passing in the transformation?

bergmark commented 9 years ago

It is possible... something like this pseudo code? Note that we need several outputs for the haskell client and documentation.

type M e = ExceptT GenerateError (ReaderT Config IO)

type Output = [(FilePath, FileType, String)]

generateToString :: M GenerateError Output

runTransform :: (Output -> IO String) -> M a Output

writeToFile :: M GenerateError Output -> IO ExitCode

writeToStdout :: M GenerateError Output -> IO ExitCode

executable :: ExitCode -> IO ()
executable = exitWith
hesselink commented 9 years ago

Ooh, multiple outputs complicate things. What do you think? Do you prefer the current code with the callback, or this 'combinator' approach?

octopuscabbage commented 9 years ago

I'll have to take a look when I get a free moment, but it looks like it could be usable. I could drop in the closure compiler as a proof of concept, or write a tiny library for adding JS compilers/minifiers to this project if you'd rather it be decoupled like that.

bergmark commented 9 years ago

Yeah I think it makes sense to decouple it, a closure-compiler library could just export a String -> IO String function that could be plugged in here, that way rest doesn't have to worry about closure and others are free to use a separate minimizer if they want. At Silk we'd probably want this to be something like

min s = do
  closure s >>= writeFile "api.min.js"
  return s

since we want the unminimized version to be available and used during development.

bergmark commented 9 years ago

... which does bring up another point, we'd then want to get the path where rest will save the file as an argument to this function. I'm going to fiddle more with the combinator approach, I think that would give us a less ad-hoc way of getting this information.