On Wed, Jun 17, 2015 at 03:06:26PM -0400, Richard Hansen wrote:
> If tput needs ~/.terminfo for the current $TERM, then tput will
> succeed before HOME is changed to $TRASH_DIRECTORY (causing color to
> be set to 't') but fail afterward.
>
> One possible way to fix this is to treat HOME like TERM: back up the
> original value and temporarily restore it before say_color() runs
> tput.
>
> Instead, pre-compute and save the color control sequences before
> changing either TERM or HOME. Use the saved control sequences in
> say_color() rather than call tput each time. This avoids the need to
> back up and restore the TERM and HOME variables, and it avoids the
> overhead of a subshell and two invocations of tput per call to
> say_color().
>
> Signed-off-by: Richard Hansen <rhansen@xxxxxxx>
Nice, I like it.
> + # Save the color control sequences now rather than run tput
> + # each time say_color() is called. This is done for two
> + # reasons:
> + # * TERM will be changed to dumb
> + # * HOME will be changed to a temporary directory and tput
> + # might need to read ~/.terminfo from the original HOME
> + # directory to get the control sequences
> + # Note: This approach assumes the control sequences don't end
> + # in a newline for any terminal of interest (command
> + # substitutions strip trailing newlines). Given that most
> + # (all?) terminals in common use are related to ECMA-48, this
> + # shouldn't be a problem.
Yeah, that was my first thought, but I agree it probably isn't going to
be a big deal in practice.
> + say_color_error=$(tput bold; tput setaf 1) # bold red
> + say_color_skip=$(tput setaf 4) # blue
> + say_color_warn=$(tput setaf 3) # brown/yellow
> + say_color_pass=$(tput setaf 2) # green
> + say_color_info=$(tput setaf 6) # cyan
> + say_color_sgr0=$(tput sgr0)
> [...]
> + error|skip|warn|pass|info)
> + eval "say_color_color=\$say_color_$1";;
> *)
> test -n "$quiet" && return;;
I think you could dispense with this case statement entirely and do:
eval "say_color_color=\$say_color_$1"
if test -z "$say_color_color"; then
test -n "$quiet" && return
fi
I guess that is making the assumption that all colors have non-zero
sizes, but that seems reasonable. I do not mind it so much as you have
it, but it does mean adding a new field needs to update two spots.
-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