add backend args flag #710
Conversation
src/Spago/CLI.hs
Outdated
| ) | ||
|
|
||
| firstNonEmpty x y = if null x then y else x | ||
| execArgs = firstNonEmpty <$> backendArgs <*> nodeArgs |
There was a problem hiding this comment.
Why do we have to pick one? I'd expect we could just merge them?
There was a problem hiding this comment.
I have no strong preference. I had it as merge first, but thought that would more surprising than preferring one or the other. For instance, if one script written by one person uses node-args, and another adds backend-args on top, you might expect it to override the first one, since you'd be treating them as equivalent. But, if the semantics of repeating node-args is to combine them, then having both would imply combining for the same reason.
| ( "test" | ||
| , "Test the project with some module, default Test.Main" | ||
| , Test <$> mainModule <*> buildOptions <*> nodeArgs | ||
| , Test <$> mainModule <*> buildOptions <*> execArgs |
There was a problem hiding this comment.
I'm now wondering if we should now call the new flag --exec-args or --run-args rather than --backend-args? 🤔
There was a problem hiding this comment.
Yea, I like exec-args better. I guess my variable names betray that.
|
Should I go forward with those two changes - merge instead of override and "exec-args"? |
@joprice sorry for delay, I was travelling yesterday - yes let's go forward with these! |
|
Updated. Let me know if I missed anything |
2 participants
Description of the change
This allows the more general "backend-args" to be used instead of "node-args", since there are now multiple backends and the flag is not actually specific to node. It addresses #709.
Checklist:
README