-
Notifications
You must be signed in to change notification settings - Fork 6
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 error handling #3
Conversation
|
||
date=$(date +"%Y-%m-%dT%H:%M:%SZ") | ||
fpath=$(mktemp "/tmp/paste.$date.XXXXXX.txt") || | ||
die "could not 'ktemp /tmp/paste.$date.XXXXXX.txt'" |
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.
maybe indent die ...
as it's a continuation from the previous line?
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.
Git is very inconsistent about this.
Sometimes the die
is indented and sometimes not (see https://github.com/git/git/blob/master/git-rebase--interactive.sh for example).
By the way, I use Emacs, and in the shell-script mode it doesn't indent lines following || or &&. I didn't find a simple way to change that, and I think it is not a bug. So I could change it, but it would mean I would either have to fight Emacs or spend a lot of time finding a workaround.
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.
ok sounds good, no worries. (i do think it is confusing for the reader, the non-indentation makes it look like it's in the same scope, not within a conditional. but no use fighting everyone's sh indentation scripts
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.
Yeah, I understand that it can be confusing for the reader. I found it confusing myself when I started working on Git.
I asked on the Git mailing list:
http://thread.gmane.org/gmane.comp.version-control.git/274665
this makes me wonder if we should have tests. it may be nice to have a "sharnessify" script that sets up all the sharness scaffolding to test a project. (like makes the makefile, etc). that way it's super fast for us to add nice tests to these scripts. |
this all LGTM. i'll test it on osx and merge in the next few days |
Ok, I will have a look at a sharnessify script. |
- `set -o pipefail` is not portable as dash doesn't know about it, and - `set -e` is not very useful if you have a die() function, and [has some problems too](http://mywiki.wooledge.org/BashFAQ/105).
fa0e23d
to
3376138
Compare
I just force pushed the series with only a small typo fix in an error message. |
@chriscool i find myself copy-pasting a lot of shell code. have you considered the virtues of making modules installable with ipfs? i mean modules like libraries in other programs. Something like: #!/bin/sh
source "$SPM" # source in functions like "require"
require "clipboard" # expands to source "clipboard" from a well-known install location
clipboard_copy "hello" # cross-platform clipboard copy we could publish the scripts directly with IPFS and have a command like:
which would fetch clipboard from an index at a well-known IPNS (or HTTP or DNS) path. (/musing. this may be more trouble than it's worth.) |
LGTM thanks |
About a sharnessify script, there is now: https://github.com/chriscool/sharnessify It does not a lot of things but I plan to improve it. Maybe have it add a Makefile, a .gitignore, ... |
About shell modules installable using IPFS, yeah that's interesting! I will think about it. |
Use
die()
instead ofset -e
andset -o pipefail
for error handling.Also improve calling
curl
. And tryxclip
Linux.