-
Notifications
You must be signed in to change notification settings - Fork 50
Migrate from ContT to Aff #208
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
Changes from all commits
d918289
b6f9d51
552acb7
3d26261
e6efb9a
f0f87b9
7c260e6
2d4d25a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,17 +13,19 @@ module Try.API | |
|
||
import Prelude | ||
|
||
import Affjax (URL, printError) | ||
import Affjax as AX | ||
import Affjax.RequestBody as AXRB | ||
import Affjax.ResponseFormat as AXRF | ||
import Affjax.StatusCode (StatusCode(..)) | ||
import Control.Alt ((<|>)) | ||
import Control.Monad.Cont.Trans (ContT(ContT)) | ||
import Control.Monad.Except (runExcept) | ||
import Control.Monad.Except.Trans (ExceptT(ExceptT)) | ||
import Control.Monad.Except (ExceptT(..), runExcept) | ||
import Data.Either (Either(..)) | ||
import Data.Generic.Rep (class Generic) | ||
import Data.List.NonEmpty (NonEmptyList) | ||
import Data.Maybe (Maybe) | ||
import Effect (Effect) | ||
import Effect.Uncurried (EffectFn1, EffectFn3, EffectFn4, mkEffectFn1, runEffectFn3, runEffectFn4) | ||
import Foreign (Foreign, ForeignError) | ||
import Data.Maybe (Maybe(..)) | ||
import Effect.Aff (Aff) | ||
import Foreign (ForeignError, unsafeToForeign) | ||
import Foreign.Class (class Decode, decode) | ||
import Foreign.Generic (defaultOptions, genericDecode) | ||
import Foreign.Generic.Class (Options, SumEncoding(..)) | ||
|
@@ -123,31 +125,23 @@ instance decodeCompileResult :: Decode CompileResult where | |
CompileSuccess <$> genericDecode decodingOptions f | ||
<|> CompileFailed <$> genericDecode decodingOptions f | ||
|
||
foreign import get_ | ||
:: EffectFn3 | ||
String | ||
(EffectFn1 String Unit) | ||
(EffectFn1 String Unit) | ||
Unit | ||
|
||
-- | A wrapper for `get` which uses `ContT`. | ||
get :: String -> ExceptT String (ContT Unit Effect) String | ||
get uri = ExceptT (ContT \k -> runEffectFn3 get_ uri (mkEffectFn1 (k <<< Right)) (mkEffectFn1 (k <<< Left))) | ||
|
||
-- | POST the specified code to the Try PureScript API, and wait for | ||
-- | a response. | ||
foreign import compile_ | ||
:: EffectFn4 | ||
String | ||
String | ||
(EffectFn1 Foreign Unit) | ||
(EffectFn1 String Unit) | ||
Unit | ||
|
||
-- | A wrapper for `compileApi` which uses `ContT`. | ||
compile | ||
:: String | ||
-> String | ||
-> ExceptT String (ContT Unit Effect) | ||
(Either (NonEmptyList ForeignError) CompileResult) | ||
compile endpoint code = ExceptT (ContT \k -> runEffectFn4 compile_ endpoint code (mkEffectFn1 (k <<< Right <<< runExcept <<< decode)) (mkEffectFn1 (k <<< Left))) | ||
get :: URL -> ExceptT String Aff String | ||
get url = ExceptT $ AX.get AXRF.string url >>= case _ of | ||
Left e -> | ||
pure $ Left $ printError e | ||
Right { status } | status >= StatusCode 400 -> | ||
pure $ Left $ "Received error status code: " <> show status | ||
Right { body } -> | ||
pure $ Right body | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe the second check (which guards against status codes over 400) is technically unnecessary, but I couldn't verify that at a quick breeze through the Affjax documentation so I've included it here. I can remove these checks if they're redundant, though. This doesn't really need to be in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you check and see what happens if you get a 404 status code? I haven't been able to verify from a quick glance through the documentation either but I actually suspect it's not redundant. Either way, I think we should know the answer before merging. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’ll verify this check is necessary. Do you have any opinion on the code as-is if the check does turn out to be necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The code seems fine to me as-is if the check is necessary. If it's not necessary, I think we should remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @hdgarrood Yep, it turns out this check is necessary. |
||
|
||
-- | POST the specified code to the Try PureScript API, and wait for a response. | ||
compile :: String -> String -> ExceptT String Aff (Either (NonEmptyList ForeignError) CompileResult) | ||
compile endpoint code = ExceptT $ AX.post AXRF.json (endpoint <> "/compile") (Just requestBody) >>= case _ of | ||
Left e -> | ||
pure $ Left $ printError e | ||
Right { status } | status >= StatusCode 400 -> | ||
pure $ Left $ "Received error status code: " <> show status | ||
Right { body } -> | ||
pure $ Right $ runExcept (decode (unsafeToForeign body)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This |
||
where | ||
requestBody = AXRB.String code |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,48 +1,9 @@ | ||
"use strict"; | ||
|
||
exports.getGistById_ = function(id, done, fail) { | ||
$.ajax({ | ||
url: 'https://api.github.com/gists/' + id, | ||
dataType: 'json' | ||
}).done(done).fail(function(err) { | ||
fail("Unable to load Gist metadata"); | ||
}); | ||
} | ||
|
||
exports.tryLoadFileFromGist_ = function(gistInfo, filename, done, fail) { | ||
exports.rawUrl_ = function (gistInfo, filename) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This small function, exports.tryLoadFileFromGist_ = function(gistInfo, filename, done, fail) {
if (gistInfo.files && gistInfo.files.hasOwnProperty(filename)) {
var url = gistInfo.files[filename].raw_url; I opted to keep this code in the FFI rather than implement new data types and decoding in PureScript. That's once again to keep the diff minimal. In the future, though, it would be nice to switch to something like |
||
if (gistInfo.files && gistInfo.files.hasOwnProperty(filename)) { | ||
var url = gistInfo.files[filename].raw_url; | ||
|
||
return $.ajax({ | ||
url: url, | ||
dataType: 'text' | ||
}).done(done).fail(function(err) { | ||
fail(err.statusText); | ||
}); | ||
return gistInfo.files[filename].raw_url; | ||
} else { | ||
fail("Gist does not contain a file named " + filename); | ||
return null; | ||
} | ||
}; | ||
|
||
exports.uploadGist_ = function(content, done, fail) { | ||
var data = { | ||
"description": "Published with try.purescript.org", | ||
"public": false, | ||
"files": { | ||
"Main.purs": { | ||
"content": content | ||
} | ||
} | ||
}; | ||
|
||
$.ajax({ | ||
url: 'https://api.github.com/gists', | ||
type: 'POST', | ||
dataType: 'json', | ||
data: JSON.stringify(data) | ||
}).success(function(e) { | ||
done(e.id); | ||
}).error(function(e) { | ||
fail(e); | ||
}); | ||
}; |
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.
These changes are largely indentation related; the only real difference is the introduction of
liftEffect
and switchingrunContT
for binds inAff
.