Skip to content
This repository has been archived by the owner on Nov 24, 2022. It is now read-only.

Fix getArgs support for node #645

Merged
merged 7 commits into from
May 15, 2020
Merged

Fix getArgs support for node #645

merged 7 commits into from
May 15, 2020

Conversation

TerrorJack
Copy link
Member

Closes #634.

This PR fixes getArgs for node. Say we compile test.hs and run node test.mjs extra flags, getArgs will properly return ["extra", "flags"].

The current implementation is just enough to support #640. Limitations compared to native getArgs:

@TerrorJack TerrorJack added P1 critical: next release type: bug labels May 13, 2020
@TerrorJack TerrorJack requested a review from gkaracha May 13, 2020 14:25
@TerrorJack TerrorJack self-assigned this May 13, 2020
Copy link
Member

@gkaracha gkaracha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comment suggestions to document the more intricate parts of the PR.

Optionally, I think I'd also add a simple test:

import Asterius.Types
import Data.Coerce
import System.Environment

foreign import javascript "console.log($1)" js_print :: JSVal -> IO ()

js_print_string :: String -> IO ()
js_print_string s = js_print (coerce (toJSString s))

main :: IO ()
main = getArgs >>= print

(expected output: ["extra","flags"])

@TerrorJack
Copy link
Member Author

@gkaracha Go ahead with the test then; remember to pull the weeds in the code snippet you pasted.

@gkaracha
Copy link
Member

@gkaracha Go ahead with the test then; remember to pull the weeds in the code snippet you pasted.

1546735 should do it 🙂

Copy link
Member

@gkaracha gkaracha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test indeed prints ["extra","flags"] as expected (see the very end of https://buildkite.com/tweag-1/asterius/builds/566#3daf7c98-89ea-4c20-8f8f-61f15b099545). This one is good to go.

@TerrorJack TerrorJack merged commit 2768e10 into master May 15, 2020
@TerrorJack TerrorJack deleted the wip-getargs branch May 15, 2020 01:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P1 critical: next release type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement support for getArgs
2 participants