Cedric Staniewski wrote:
Allan McRae wrote:
Dan McGee wrote:
On Thu, Oct 22, 2009 at 7:41 PM, Allan McRae <[email protected]> wrote:
Cedric Staniewski wrote:
Suggested-by: Dan McGee <[email protected]>
Signed-off-by: Cedric Staniewski <[email protected]>
---
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 <[email protected]>
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 <[email protected]>
---
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
That looks good to me. Feel free to tidy it as you see fit and forward
the patch.
Allan
