[pacman-dev] [PATCH] makepkg: use tput for terminal-safe colored and bold text

[Cedric Staniewski]

Allan McRae wrote:
> Dan McGee wrote:
>> On Thu, Oct 22, 2009 at 7:41 PM, Allan McRae <allan at archlinux.org> wrote:
>>  
>>> Cedric Staniewski wrote:
>>>    
>>>> Suggested-by: Dan McGee <dan at archlinux.org>
>>>> Signed-off-by: Cedric Staniewski <cedric at gmx.ca>
>>>> ---
>>>>
>>>> I do not know if this patch is usefull at all, because I do not
>>>> notice any
>>>> change. Just grabbed this from Dan's TODO list and wanted to play a bit
>>>> with tput.
>>>>
>>>>       
>>> It looks fine to me...
>>> I just have no idea on the advantage we are achieving with this
>>> change apart
>>> from the apparent terminal safeness.  Was the old version not safe?
>>>
>>> Dan: comments?
>>>     
>>
>> First comment- that TODO list is still huge, wow. :)

Hehe. :)


>>
>> Anyway, this seems pretty reasonable to me, but not sure it is worth
>> it. At the least, we should capture these sequences once on script
>> startup, and then use the global variable in each function. And does
>> $(tput offbold) make more sense for the reset?
>>   
> 
> There does not appear to be such a thing as "tput offbold".
> 
> Anyway, I like the idea of setting these all at the start.
> 
> if [ $COLORMSG -eq 1 ]; then
>  BOLD_ON=$(tput bold)
>  BOLD_OFF=${tput srg0)
>  GREEN_ON...
>  ...
> fi
> 
> then the messages can just be
> 
> plain() {
>     local mesg=$1; shift
>         printf "${BOLD_ON}    ${mesg}${BOLD_OFF}\n" "$@" >&2
>     fi
> }
> 
> 
> with none of the tests needed.
> 
> Allan

That definitely makes sense.
If you think the attached patch is fine, I plan to change the order of these two ones and resubmit them.


>From 50901c551ddefb9a5eca500ac63ab0a0c7eef167 Mon Sep 17 00:00:00 2001
From: Cedric Staniewski <cedric at gmx.ca>
Date: Fri, 23 Oct 2009 12:27:54 +0200
-- 8< --
Subject: [PATCH] makepkg: define the required escape sequences globally

In doing so, it is possible to get rid of all the tests for colored
messages except for one global one.

Signed-off-by: Cedric Staniewski <cedric at gmx.ca>
---
 scripts/makepkg.sh.in |   42 +++++++++++++++---------------------------
 1 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index a64a867..b89acdb 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -72,7 +72,6 @@ IGNOREARCH=0
 HOLDVER=0
 PKGFUNC=0
 SPLITPKG=0
-COLORMSG=0
 
 # Forces the pkgver of the current PKGBUILD. Used by the fakeroot call
 # when dealing with svn/cvs/etc PKGBUILDs.
@@ -84,47 +83,27 @@ PACMAN_OPTS=
 
 plain() {
 	local mesg=$1; shift
-	if [ $COLORMSG -eq 1 ]; then
-		printf "$(tput bold)    ${mesg}$(tput sgr0)\n" "$@" >&2
-	else
-		printf "    ${mesg}\n" "$@" >&2
-	fi
+	printf "${BOLD}    ${mesg}${ALL_OFF}\n" "$@" >&2
 }
 
 msg() {
 	local mesg=$1; shift
-	if [ $COLORMSG -eq 1 ]; then
-		printf "$(echo -e "bold\nsetaf 2" | tput -S)==>$(echo -e "sgr0\nbold" | tput -S) ${mesg}$(tput sgr0)\n" "$@" >&2
-	else
-		printf "==> ${mesg}\n" "$@" >&2
-	fi
+	printf "${GREEN}==>${ALL_OFF_BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2
 }
 
 msg2() {
 	local mesg=$1; shift
-	if [ $COLORMSG -eq 1 ]; then
-		printf "$(echo -e "bold\nsetaf 4" | tput -S)  ->$(echo -e "sgr0\nbold" | tput -S) ${mesg}$(tput sgr0)\n" "$@" >&2
-	else
-		printf "  -> ${mesg}\n" "$@" >&2
-	fi
+	printf "${BLUE}  ->${ALL_OFF_BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2
 }
 
 warning() {
 	local mesg=$1; shift
-	if [ $COLORMSG -eq 1 ]; then
-		printf "$(echo -e "bold\nsetaf 3" | tput -S)==> $(gettext "WARNING:")$(echo -e "sgr0\nbold" | tput -S) ${mesg}$(tput sgr0)\n" "$@" >&2
-	else
-		printf "==> $(gettext "WARNING:") ${mesg}\n" "$@" >&2
-	fi
+	printf "${YELLOW}==> $(gettext "WARNING:")${ALL_OFF_BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2
 }
 
 error() {
 	local mesg=$1; shift
-	if [ $COLORMSG -eq 1 ]; then
-		printf "$(echo -e "bold\nsetaf 1" | tput -S)==> $(gettext "ERROR:")$(echo -e "sgr0\nbold" | tput -S) ${mesg}$(tput sgr0)\n" "$@" >&2
-	else
-		printf "==> $(gettext "ERROR:") ${mesg}\n" "$@" >&2
-	fi
+	printf "${RED}==> $(gettext "ERROR:")${ALL_OFF_BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2
 }
 
 
@@ -1576,7 +1555,16 @@ fi
 
 # check if messages are to be printed using color
 if [ -t 2 -a ! "$USE_COLOR" = "n" -a "$(check_buildenv color)" = "y" ]; then
-	COLORMSG=1
+	readonly ALL_OFF="$(tput sgr0)" \
+						BOLD="$(tput bold)"
+	readonly ALL_OFF_BOLD="${ALL_OFF}${BOLD}" \
+						RED="${BOLD}$(tput setaf 1)" \
+						BLUE="${BOLD}$(tput setaf 4)" \
+						GREEN="${BOLD}$(tput setaf 2)" \
+						YELLOW="${BOLD}$(tput setaf 3)"
+else
+	unset ALL_OFF ALL_OFF_BOLD BOLD RED BLUE GREEN YELLOW
+	readonly ALL_OFF ALL_OFF_BOLD BOLD RED BLUE GREEN YELLOW
 fi
 
 # override settings with an environment variable for batch processing
-- 
1.6.5.1