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

guard against minimal environments #24

Merged
merged 1 commit into from
Sep 18, 2016

Conversation

buzzdecafe
Copy link
Contributor

ran into this in gitbash on an OS that must not be named

xyz
reset=$(tput sgr0)
else
bold=""
reset=""
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think the three lines above are necessary. Won't bold and reset default to empty string if not defined?

Copy link
Contributor Author

@buzzdecafe buzzdecafe Sep 9, 2016

Choose a reason for hiding this comment

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

Won't bold and reset default to empty string if not defined?

Yes, however, when running xyz in gitbash, the script fails like so:

$ ./xyz --dry-run
./xyz: line 122: tput: command not found

With the guard above included, it works just fine:

$ ./xyz --dry-run
Current version is 0.5.0. Press [enter] to publish [email protected].
npm prune
npm test
node -e 'var o = require("./package.json"); o.version = "0.5.1"; require("fs").writeFileSync("./package.json", JSON.stringify(o, null, 2) + "\n");'
git add 'package.json'
git commit --message 'Version 0.5.1'
git tag --annotate 'v0.5.1' --message 'Version 0.5.1'
git push 'origin' 'refs/heads/master' 'refs/tags/v0.5.1'
npm publish

But you don't get the nice bolding of the version numbers.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not suggesting removing the guard. I'm suggesting that rather than…

if ... ; then
  bold=$(tput bold)
  reset=$(tput sgr0)
else
  bold=""
  reset=""
fi

we have…

if ... ; then
  bold=$(tput bold)
  reset=$(tput sgr0)
fi

@buzzdecafe
Copy link
Contributor Author

((:bell:))

@davidchambers
Copy link
Owner

This looks great. I love how minimal the fix ended up being.

One minor point: Your avatar is not being displayed because the email address you used is not associated with your GitHub account. Would you mind linking it? I'd like to see the dinosaur in the commit history. :)

@buzzdecafe
Copy link
Contributor Author

curious. is the avatar there now?

@davidchambers
Copy link
Owner

is the avatar there now?

Yes. Thanks. :)

One more thing: Would you mind rebasing on master?

@davidchambers
Copy link
Owner

🌳

@davidchambers davidchambers merged commit 76e7beb into davidchambers:master Sep 18, 2016
@davidchambers
Copy link
Owner

Released in v1.0.1

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.

2 participants