-
Notifications
You must be signed in to change notification settings - Fork 11
add --shell option #17
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
Conversation
@obfusk, I apologise for losing track of this. Bumped up on the queue, I'll try to get to it soon. |
And again. My apologies. @obfusk this seems great, two birds with one stone. I have only some UI quibbles:
Instead, how would you feel about:
I would almost not use up the valuable -s flag for this, since --shell is quite mnemonic and still fairly short. Your call. |
I made the changes you suggested and added a test case. |
Thank you! |
Also, would you have any time to look at #13 ? I think those files need to be added to the cabal file, and so does your new bash test. |
|
||
testexamples: build | ||
$(SHELLTEST) tests/examples | ||
|
||
testbash: build | ||
! $(SHELLTEST) tests/bash |
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.
@obfusk, what is this syntax ? (!)
Although make testbash
passes its test, make then exits with an error, which we don't want.
What's the purpose of tests/bash again ? This is a place to put tests which test bash-specific things, and in theory should be run only with bash I guess.
What is dollar-quote.test for.. is it #15 ? Perhaps worth adding the issue number and a one line description of what's being tested, to the test file.
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.
From man bash
: "If the reserved word ! precedes a pipeline, the exit status of that pipeline is the logical negation of the exit status".
The dollar-quote.test
is indeed for #15. I'm running it both without bash (expecting it to fail) and with bash (expecting it to succeed).
The test that's expected to fail indeed shows an error in the output, but because its exit status is negated, the make test
command as a whole succeeds.
fixes #6
fixes #15