-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add optional output
argument to mustache bin
#540
Conversation
output
argument to mustache binoutput
argument to mustache bin
Seems like a good feature 👍 Could you please add atleast one test for it aswell? |
Sorry, test added now. |
@@ -0,0 +1 @@ | |||
Howdy LeBron, CLI rox |
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'm thinking this file shouldn't be commited, it would even be better if it were deleted after the tests has run.
When this file exists in the repo, your changes in ./bin/mustache could actually be reverted, and the test below would still pass - right?
Nice! One little comment about the output file, otherwise it LGTM. |
Done 💪 |
Add optional `output` argument to mustache bin
Thanks! 👍 |
Btw @wizawu, what's the case in which |
When using ansible's shell module.
Apparently we don't want to append to output. |
@wizawu But does it raise an error, or? Have you checked ansible/ansible#9791 and/or http://docs.ansible.com/ansible/shell_module.html, specially:
|
It won't raise an error. It would exit normally, but without any output. I know it is not a bug of mustache. And yes, I've checked those docs. Thanks :) I just thought adding a third argument would be easier for me. I do agree that it is just an uncommon scene. There is another case in package.json
The second script won't work as expected, since I also agree that it is another uncommon scene. |
FYI if you'd like to send custom arguments to a npm script, like it seems you're trying to with Read more about it in npm run script docs. |
@phillipj Very useful! But after all, |
I am not sure whether this PR is welcome, since I didn't see much people had a requirement for using this argument.
Yet we have been using this feature in production (because
> output
is forbidden in certain cases).Feel free to close it if you find it confusing.
Thanks.