Permalink
Browse files

Set `pipefail' and `lastpipe' for better debugging

`true <<< $(false)` would always be true, we'd never catch the failure in
the command substitution red handed. Instead, we use `foo | bar` with the
`pipefail' shell option to catch the failure anywhere in the pipeline.
Also, enable `lastpipe' so `bar` can run in the current shell instead of
in a subshell, so variable assignments in `bar` don't get lost after the
pipeline.

Another subtle issue with `bar <<< "$(foo)"` is the output of `foo` will
be altered, with trailing newlines stripped and one newline appended.
This behavior may be fine in most cases, some may even find it desirable
for the default `read` behavior which uses newline as line delimiter,
but sometimes `foo | bar` is preferable since it does not modify the
output of `foo` in any way.

`foo | bar` is nicer for the `read -n 1` while loop which takes a string
$region_select_action as input, since it does not append a newline so
we don't read in a null at the end.

But when piping to `read`, we must be careful not to trigger an error by
reaching EOF. e.g.,

  printf '%s' a | read         # PIPESTATUS 0 1
  printf '%s\n' a | read       # PIPESTATUS 0 0
  printf '%s\0' a | read -d '' # PIPESTATUS 0 0

For this reason, we add '\n' to the output of several functions.
  • Loading branch information...
1 parent 7ce2cc3 commit 7db94cdcfc396b0cd624c35c219d2687fa2c70b0 @lolilolicon committed Oct 23, 2011
Showing with 25 additions and 12 deletions.
  1. +25 −12 ffcast.bash
View
@@ -16,8 +16,8 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-set -e
-shopt -s extglob
+set -e +m -o pipefail
+shopt -s extglob lastpipe
readonly progname=ffcast progver='@VERSION@'
readonly cast_cmd_pattern='@(ffmpeg|byzanz-record|recordmydesktop)'
@@ -57,6 +57,14 @@ debug_run() {
debug_dryrun "$@" && "$@"
}
+debug_pipestatus() {
+ set -- "${PIPESTATUS[@]}"
+ (( verbosity >=2 )) || return 0
+ printf 'debug: pipestatus:'
+ (( $# )) && printf ' %d' "$@"
+ printf '\n'
+} >&2
+
debug() {
(( verbosity >= 2 )) || return 0
_msg 'debug: ' "$@"
@@ -152,7 +160,7 @@ parse_geospec_get_corners() {
return 1
;;
esac
- printf '%d,%d %d,%d' $_x $_y $x_ $y_
+ printf '%d,%d %d,%d\n' $_x $_y $x_ $y_
}
region_intersect_corners() {
@@ -169,7 +177,7 @@ region_intersect_corners() {
(( X_ = x_ > X_ ? x_ : X_ )) || :
(( Y_ = y_ > Y_ ? y_ : Y_ )) || :
done
- printf '%d,%d %d,%d' $_X $_Y $X_ $Y_
+ printf '%d,%d %d,%d\n' $_X $_Y $X_ $Y_
}
region_union_corners() {
@@ -186,7 +194,7 @@ region_union_corners() {
(( X_ = x_ < X_ ? x_ : X_ )) || :
(( Y_ = y_ < Y_ ? y_ : Y_ )) || :
done
- printf '%d,%d %d,%d' $_X $_Y $X_ $Y_
+ printf '%d,%d %d,%d\n' $_X $_Y $X_ $Y_
}
select_region_get_corners() {
@@ -202,7 +210,7 @@ select_window_get_corners() {
# stdout: x1,y1 x2,y2
xrectsel_get_corners() {
# Note: requires xrectsel 0.3
- xrectsel "%x,%y %X,%Y"
+ xrectsel "%x,%y %X,%Y"$'\n'
}
# stdin: xwininfo output (locale: C)
@@ -219,7 +227,7 @@ xwininfo_get_dimensions() {
continue
fi
if (( w && h )); then
- printf '%dx%d' $w $h
+ printf '%dx%d\n' $w $h
return
fi
done
@@ -256,7 +264,7 @@ xwininfo_get_corners() {
else
continue
fi
- printf '%d,%d %d,%d' $_x $_y $x_ $y_
+ printf '%d,%d %d,%d\n' $_x $_y $x_ $y_
return
done
return 1
@@ -341,11 +349,15 @@ shift $(( OPTIND -1 ))
# Process region geometry
declare rootw=0 rooth=0 _x=0 _y=0 x_=0 y_=0 w=0 h=0
-IFS=x read root{w,h} <<< "$(LC_ALL=C xwininfo -root | xwininfo_get_dimensions)"
+if ! LC_ALL=C xwininfo -root | xwininfo_get_dimensions |
+ IFS=x read rootw rooth; then
+ debug_pipestatus
+ error 'failed to get root window dimensions'
+ exit 1
+fi
# Note: this is safe because xwininfo_get_dimensions ensures that its output is
# either {int}x{int} or null, a random string like "rootw" is impossible.
if ! (( rootw && rooth )); then
- # %d is OK even if rootw or rooth is null, in which case 0 is printed
error 'invalid root window dimensions: %dx%d' "$rootw" "$rooth"
exit 1
fi
@@ -367,6 +379,7 @@ while (( $# )); do
shift
done
+printf %s "$region_select_action" |
while read -n 1; do
case $REPLY in
's')
@@ -379,10 +392,10 @@ while read -n 1; do
;;
'b')
borderless=0
- debug "windows: now including borders"
+ verbose "windows: now including borders"
;;
esac
-done <<< "$region_select_action"
+done
if (( i )); then
corners=$(region_union_corners "${corners_list[@]}")

0 comments on commit 7db94cd

Please sign in to comment.