Skip to content
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

Fix exit-code pass-thru with nave use and add -o option to enable overriding default cURL flags #38

Closed
wants to merge 5 commits into from

Conversation

niallo
Copy link

@niallo niallo commented Jun 25, 2012

E.g. get silent output with

nave -o'-Lfs' install 0.8.0

Niall O'Higgins added 4 commits June 25, 2012 10:06
e.g without patch:

<nialljohiggins@mbp:nave>$ nave use 0.8.0 exit 1
Already installed: 0.8.0
using 0.8.0
<nialljohiggins@mbp:nave>$ echo $?
0

with patch:

<nialljohiggins@mbp:nave>$ sh nave.sh use 0.8.0 exit 1
Already installed: 0.8.0
using 0.8.0
failed somehow
<nialljohiggins@mbp:nave>$ echo $?
1
@niallo
Copy link
Author

niallo commented Jun 27, 2012

Added another commit to fix bug with nave use:

without patch:

<nialljohiggins@mbp:nave>$ nave use 0.8.0 exit 1
Already installed: 0.8.0
using 0.8.0
<nialljohiggins@mbp:nave>$ echo $?
0

with patch:

<nialljohiggins@mbp:nave>$ sh nave.sh use 0.8.0 exit 1
Already installed: 0.8.0
using 0.8.0
failed somehow
<nialljohiggins@mbp:nave>$ echo $?
1

@isaacs
Copy link
Owner

isaacs commented Aug 7, 2012

Took the return value hiding, and changed to not do --debug.

Parsing options and overriding the curl opts seems weird and unnecessary to me.

In general, it's best to only have one feature per pull request. You can push different features to different branches to keep it more clear.

@isaacs isaacs closed this Aug 7, 2012
@niallo
Copy link
Author

niallo commented Aug 7, 2012

Thanks. Sorry about the multiple features in that pull request, will do branches next time.

Re: cURL opts. My use case is saving nave output. cURL's progress bar (-# option) does not gracefully handle non-TTY output fd and essentially spews garbage. To see what I mean, just redirect nave output to a file. Hopefully that convinces you :-)

Perhaps a better alternative to parsing options is using an environment variable? I'm not wedded to the getopt approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants