Jeff King <peff@xxxxxxxx> writes: > On Wed, Jun 17, 2015 at 05:11:21PM -0400, Richard Hansen wrote: > >> + test -z "$1" && test -n "$quiet" && return >> + eval "say_color_color=\$say_color_$1" > > Thanks, this looks much simpler. > > In the non-quiet case, you will eval $say_color_, even though we know it > to be bogus. Yeah, but there is this gem in this patch: + ... + say_color_info=$(tput setaf 6) # cyan + say_color_reset=$(tput sgr0) + say_color_="" # no formatting for normal text In other words, the patch handles these two in the same mechanism: say_color error "this is my error message" say_color "" "ok this is just a regular message" and treating an empy string just one of the supported "colors", i.e. "error", "skip", "warn", "pass", "info" "reset" and "" are the colors. > I guess we need to make sure say_color_color is blank, > though. The alternative would be: > > if test -z "$1"; then > test -n "$quiet" && return > say_color_color= > else > eval "say_color_color=\$say_color_$1" > fi > > I dunno if that makes the intent more clear or not. I am OK with it > either way. I am OK with it either way, too. -- 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