On Fri, Oct 10, 2014 at 02:13:55AM -0400, Jeff King wrote: > Having finally figured out how to drop the "set +x" from the output, I > have noticed that I kind of liked the "test_eval_ret=$?" part of the > trace (which is now gone, too), because it pretty explicitly tells you > that the last traced command failed. But now that it has been silenced, > there's no reason we couldn't add back in our own output to make it more > clear. Like: diff --git a/t/test-lib.sh b/t/test-lib.sh index a60ec75..81ceb23 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -237,7 +237,11 @@ do shift ;; -x) test_eval_start_='set -x' - test_eval_end_='set +x' + test_eval_end_=' + set +x + test "$test_eval_ret_" = 0 || + say_color error >&4 "last command exited with \$?=$?" + ' verbose=t shift ;; *) @@ -543,7 +547,7 @@ test_eval_ () { { test_eval_inner_ "$@" </dev/null >&3 2>&4 test_eval_ret_=$? - $test_eval_end_ + eval "$test_eval_end_" } 2>/dev/null return $test_eval_ret_ } I think we can probably do away with this excessive use of eval, and just keep a boolean flag for "is -x in effect" and check it inside test_eval_. Originally I was trying to keep the number of executed commands down, because everything until the "set +x" ran (including checks for an "is -x in effect" flag) was shown to the user. But since that is no longer the case, we can be less stingy with the conditionals. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html