-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rest-gen refactorings and extensions #133
base: master
Are you sure you want to change the base?
Conversation
rest-core/tests/Runner.hs
Outdated
@@ -206,5 +206,5 @@ allMethods = [GET, PUT, POST, DELETE] | |||
|
|||
testAcceptHeaders :: Assertion | |||
testAcceptHeaders = | |||
do fmt <- runRestM_ RestM.emptyInput { RestM.headers = H.singleton "Accept" "text/json" } accept | |||
do fmt <- runRestM_ RestM.emptyInput { RestM.headers = H.singleton "Accept" "application/json" } accept |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should test both here? We want both to work, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup
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. |
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 |
Hmm, I see. That use case makes sense. I'd like it more if the normal thing was |
…o mkHsApi and writeDocs so they can be used standalone easier
…ich returns the result of generation but doesn't exit the program
…/json accept header. Conflicts: rest-gen/src/Rest/Gen.hs
I ended up cleaning some more (and fixing rest-happstack and rest-example). I consider myself done now. Mind reviewing again @hesselink? |
import Rest.Gen.Base | ||
import Rest.Gen.Docs | ||
|
||
-- | Web interface for documentation | ||
apiDocsHandler :: (ServerMonad m, MonadPlus m, FilterMonad Response m, MonadIO m) => String -> String -> Api a -> m Response | ||
apiDocsHandler rootURL tmpls api = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did anything change here? It's hard to see with the change from let .. in
to where
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I had to change this and then refactored it back to the original...
Looks good! I'm still not super happy with the change to the signature of Have you thought about the possibility of providing all the building blocks instead so you can do something like |
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 |
Ooh, multiple outputs complicate things. What do you think? Do you prefer the current code with the callback, or this 'combinator' approach? |
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. |
Yeah I think it makes sense to decouple it, a
since we want the unminimized version to be available and used during development. |
... 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. |
application/json
instead oftext/json
everywhererunGenerate
function that doesn't call exitFailure/exitSuccess so generate can be used as part of other executablesRest.Gen
for library usage(FileType -> String -> IO String)
argument that can be used for postprocessing output. e.g. to prettify haskell sources or minimize javascript.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