Add multi argument callback helpers. #77
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit does several things. * It makes sure that line widths don't exceed 80 in GHCJS/Foreign/Callback.hs * It adds jsbits/callback.js which contains h$makeCallbackMulti. * It adds syncCallbackMulti, syncCallbackMulti', and asyncCallbackMulti to GHCJS/Foreign/Callback.hs. These functions utilize h$makeCallbackMulti. * It bumps up the version to 0.2.1.0 * It documents `OnBlocked` and `syncCallback'` more clearly on README. * It documents syncCallbackMulti, syncCallbackMulti', and asyncCallbackMulti on README. I tested syncCallbackMulti, syncCallbackMulti', and asyncCallbackMulti in the NodeJS example I included in README.
It interchanges a section on a callback example with a section on multi-argument callbacks
README now shows an example of using `syncCallbackMulti'` instead of `asyncCallbackMulti`.
hamishmack
reviewed
Oct 29, 2016
| asyncCallback1 :: (JSVal -> IO ()) -- ^ the function that the callback calls | ||
| -> IO (Callback (JSVal -> IO ())) -- ^ the calback | ||
| asyncCallback1 :: (JSVal -> IO ()) -- the function that the callback calls | ||
| -> IO (Callback (JSVal -> IO ())) -- the calback |
There was a problem hiding this comment.
oops. I don't think you meant to remove the haddock markup cues
There was a problem hiding this comment.
Sorry, I haven't learned haddock, yet. I'll restore haddock markup cues. I thought they were mere visual cues.
|
LGTM |
I made a mistake of removing haddock markup cues because I still don't know how to use haddock.
|
@hamishmack Would it be ok to replace {- | Make a callback (JavaScript function) that runs the supplied IO function
in a synchronous thread when called. The callback takes an arbitrary
number of arguments that it passes as an array of JSVal values to the
Haskell function.
Call 'releaseCallback' on the callback when done with the callback,
freeing data referenced by the function.
-}
syncCallbackMulti :: OnBlocked -- ^ what to do when the thread blocks
-> ([JSVal] -> IO ()) -- ^ the Haskell action
-> IO (Callback ([JSVal] -> IO ())) -- ^ the callback
syncCallbackMulti onBlocked f = do
js_syncCallbackMulti (onBlocked == ContinueAsync) $ unsafeCoerce $ \args ->
Array.toListIO (unsafeCoerce args) >>= fUpdate 1. the above function is now gone. The functions are now replaced with polyvariadic variants. |
|
I think it would be fine. |
Replace JavaScript.Array.toListIO with GHCJS.Prim.fromJSArray in GHCJS/Foreign/Callback.hs
Previously, they were cumbersome to use. Now, they are a lot easier to use.
|
@hamishmack @luite I think the pull request is ready for merge. I request that a tag, |
ToArrayCallback was difficult to understand and use, and it used `UndecidableInstances` extension. I replaced ToArrayCallback with VariadicCallback and VariadicCallbackReturn. Both VariadicCallback and VariadicCallbackReturn are much easier to understand and use. The functions that use VariadicCallback(Return) have simpler types than those that use ToArrayCallback.
It is no longer necessary after replacing ToArrayCallback with VariadicCallback(Return)
After IsJSVal was replaced with PFromJSVal, the comments on variadic callback generators weren't updated.
With UndecidableInstances, I can remove helper functions. Do not fear UndecidableInstances. UndecidableInstances just makes programmers responsible for making instance resolution terminate in a finite amount of time.
array copy is eliminated, and variadic callback generators work directly on JSArray.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request does several things.
h$makeCallbackMulti.GHCJS/Foreign/Callback/Variadic.hswhich enables polyvariadic callback functions.syncCallbackMulti,syncCallbackMulti', andasyncCallbackMultitoGHCJS/Foreign/Callback.hs. These functions utilize
h$makeCallbackMultiandVariadicCallbackfor convenience of users. Refer to README for details.OnBlockedandsyncCallback'more clearly on README.syncCallbackMulti,syncCallbackMulti', andasyncCallbackMultion README.I tested
syncCallbackMulti,syncCallbackMulti', andasyncCallbackMultiagainst the NodeJS example I included in README, and the functions worked well in my example.
This closes #76
It doesn't break existing functions.
Thus, I want this pull request to be merged as soon as possible if maintainers don't have major objections.