[pacman-dev] [PATCH v2 1/5] pacdiff: color filename and mention what we found
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- Now uses tput to get the color codes. contrib/pacdiff.sh.in | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index bfafda2..4c6277d 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -24,6 +24,22 @@ diffprog=${DIFFPROG:-vimdiff} diffsearchpath=${DIFFSEARCHPATH:-/etc} locate=0 +if tput setaf 0 &>/dev/null; then + ALL_OFF="$(tput sgr0)" + BOLD="$(tput bold)" + BLUE="${BOLD}$(tput setaf 4)" + GREEN="${BOLD}$(tput setaf 2)" + YELLOW="${BOLD}$(tput setaf 3)" + PURPLE="${BOLD}$(tput setaf 5)" +else + ALL_OFF="\e[1;0m" + BOLD="\e[1;1m" + BLUE="${BOLD}\e[1;34m" + GREEN="${BOLD}\e[1;32m" + YELLOW="${BOLD}\e[1;33m" + PURPLE="${BOLD}\e[1;35m" +fi + usage() { echo "$myname : a simple pacnew/pacorig/pacsave updater" echo "Usage : $myname [-l]" @@ -62,7 +78,15 @@ fi # see http://mywiki.wooledge.org/BashFAQ/020 while IFS= read -u 3 -r -d '' pacfile; do file="${pacfile%.pac*}" - echo "File: $file" + file_type="pac${pacfile##*.pac}" + + case $file_type in + pacnew) printf "$GREEN%s$ALL_OFF" "$file_type";; + pacorig) printf "$YELLOW%s$ALL_OFF" "$file_type";; + pacsave) printf "$BLUE%s$ALL_OFF" "$file_type";; + esac + + printf " found for $PURPLE%s$ALL_OFF\n" "$file" if [ ! -f "$file" ]; then echo " $file does not exist" rm -i "$pacfile" -- 1.8.0.2
Doesn't hurt and reassures the user that we did the right thing. Signed-off-by: Florian Pritz <bluewind@xinu.at> --- same patch contrib/pacdiff.sh.in | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 4c6277d..05531c6 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -89,21 +89,21 @@ while IFS= read -u 3 -r -d '' pacfile; do printf " found for $PURPLE%s$ALL_OFF\n" "$file" if [ ! -f "$file" ]; then echo " $file does not exist" - rm -i "$pacfile" + rm -iv "$pacfile" continue fi check="$(cmp "$pacfile" "$file")" if [ -z "${check}" ]; then echo " Files are identical, removing..." - rm "$pacfile" + rm -v "$pacfile" else echo -n " File differences found. (V)iew, (S)kip, (R)emove: [v/s/r] " while read c; do case $c in - r|R) rm "$pacfile"; break ;; + r|R) rm -v "$pacfile"; break ;; v|V) $diffprog "$pacfile" "$file" - rm -i "$pacfile"; break ;; + rm -iv "$pacfile"; break ;; s|S) break ;; *) echo -n " Invalid answer. Try again: [v/s/r] "; continue ;; esac -- 1.8.0.2
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- same patch contrib/pacdiff.sh.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 05531c6..86dc20e 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -92,8 +92,8 @@ while IFS= read -u 3 -r -d '' pacfile; do rm -iv "$pacfile" continue fi - check="$(cmp "$pacfile" "$file")" - if [ -z "${check}" ]; then + + if cmp -s "$pacfile" "$file"; then echo " Files are identical, removing..." rm -v "$pacfile" else -- 1.8.0.2
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- Now uses printf instead of echo -n. contrib/pacdiff.sh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 86dc20e..3461627 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -97,15 +97,16 @@ while IFS= read -u 3 -r -d '' pacfile; do echo " Files are identical, removing..." rm -v "$pacfile" else - echo -n " File differences found. (V)iew, (S)kip, (R)emove: [v/s/r] " + printf " (V)iew, (S)kip, (R)emove $file_type, (O)verwrite with $file_type: [v/s/r/o] " while read c; do case $c in r|R) rm -v "$pacfile"; break ;; + o|O) mv -v "$pacfile" "$file"; break ;; v|V) $diffprog "$pacfile" "$file" rm -iv "$pacfile"; break ;; s|S) break ;; - *) echo -n " Invalid answer. Try again: [v/s/r] "; continue ;; + *) printf " Invalid answer. Try again: [v/s/r/o] "; continue ;; esac done fi -- 1.8.0.2
On Thu, Dec 20, 2012 at 01:06:36AM +0100, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- Now uses printf instead of echo -n.
contrib/pacdiff.sh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 86dc20e..3461627 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -97,15 +97,16 @@ while IFS= read -u 3 -r -d '' pacfile; do echo " Files are identical, removing..." rm -v "$pacfile" else - echo -n " File differences found. (V)iew, (S)kip, (R)emove: [v/s/r] " + printf " (V)iew, (S)kip, (R)emove $file_type, (O)verwrite with $file_type: [v/s/r/o] "
You're using the prompt as a format string. It's obvious what the possible values for $file_type are, but I'd rather these be inserted as tokens replacements rather than embedded in the format string.
while read c; do case $c in r|R) rm -v "$pacfile"; break ;; + o|O) mv -v "$pacfile" "$file"; break ;; v|V) $diffprog "$pacfile" "$file" rm -iv "$pacfile"; break ;; s|S) break ;; - *) echo -n " Invalid answer. Try again: [v/s/r] "; continue ;; + *) printf " Invalid answer. Try again: [v/s/r/o] "; continue ;;
Here too.
esac done fi -- 1.8.0.2
On 20/12/12 10:19, Dave Reisner wrote:
On Thu, Dec 20, 2012 at 01:06:36AM +0100, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- Now uses printf instead of echo -n.
contrib/pacdiff.sh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 86dc20e..3461627 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -97,15 +97,16 @@ while IFS= read -u 3 -r -d '' pacfile; do echo " Files are identical, removing..." rm -v "$pacfile" else - echo -n " File differences found. (V)iew, (S)kip, (R)emove: [v/s/r] " + printf " (V)iew, (S)kip, (R)emove $file_type, (O)verwrite with $file_type: [v/s/r/o] "
You're using the prompt as a format string. It's obvious what the possible values for $file_type are, but I'd rather these be inserted as tokens replacements rather than embedded in the format string.
I am more concerned that the prompt line will almost always go over 80 characters. How about" printf " File differences found in %s\n" $file_type printf " (V)iew, (S)kip, (R)emove, (O)verwrite: [v/s/r/o] "
while read c; do case $c in r|R) rm -v "$pacfile"; break ;; + o|O) mv -v "$pacfile" "$file"; break ;; v|V) $diffprog "$pacfile" "$file" rm -iv "$pacfile"; break ;; s|S) break ;; - *) echo -n " Invalid answer. Try again: [v/s/r] "; continue ;; + *) printf " Invalid answer. Try again: [v/s/r/o] "; continue ;;
Here too.
esac done fi -- 1.8.0.2
On 20.12.2012 11:31, Allan McRae wrote:
On 20/12/12 10:19, Dave Reisner wrote:
On Thu, Dec 20, 2012 at 01:06:36AM +0100, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- Now uses printf instead of echo -n.
contrib/pacdiff.sh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 86dc20e..3461627 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -97,15 +97,16 @@ while IFS= read -u 3 -r -d '' pacfile; do echo " Files are identical, removing..." rm -v "$pacfile" else - echo -n " File differences found. (V)iew, (S)kip, (R)emove: [v/s/r] " + printf " (V)iew, (S)kip, (R)emove $file_type, (O)verwrite with $file_type: [v/s/r/o] "
You're using the prompt as a format string. It's obvious what the possible values for $file_type are, but I'd rather these be inserted as tokens replacements rather than embedded in the format string.
@Dave: I've fixed that in my repo.
I am more concerned that the prompt line will almost always go over 80 characters.
@Allan: The longest prompt line possible (file_type = pacorig or pacsave) is 72 chars including all spaces: " (V)iew, (S)kip, (R)emove pacorig, (O)verwrite with pacorig: [v/s/r/o] " Make that 73 if you want to count the char the user has to enter.
How about"
printf " File differences found in %s\n" $file_type printf " (V)iew, (S)kip, (R)emove, (O)verwrite: [v/s/r/o] "
That doesn't mention what file will be removed (or overwritten) which I found pretty disturbing in the original script and which cause me to write all those patches. If you think a little bit about it the only sane case is to remove the pacsave, pacnew, pacorig, but I still think being ambiguous here is a bad idea because we are dealing mostly with important files and users should be reassured that we are doing the right thing and they don't have to worry.
On 29/12/12 01:18, Florian Pritz wrote:
On 20.12.2012 11:31, Allan McRae wrote:
On 20/12/12 10:19, Dave Reisner wrote:
On Thu, Dec 20, 2012 at 01:06:36AM +0100, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- Now uses printf instead of echo -n.
contrib/pacdiff.sh.in | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 86dc20e..3461627 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -97,15 +97,16 @@ while IFS= read -u 3 -r -d '' pacfile; do echo " Files are identical, removing..." rm -v "$pacfile" else - echo -n " File differences found. (V)iew, (S)kip, (R)emove: [v/s/r] " + printf " (V)iew, (S)kip, (R)emove $file_type, (O)verwrite with $file_type: [v/s/r/o] "
You're using the prompt as a format string. It's obvious what the possible values for $file_type are, but I'd rather these be inserted as tokens replacements rather than embedded in the format string.
@Dave: I've fixed that in my repo.
I am more concerned that the prompt line will almost always go over 80 characters.
@Allan: The longest prompt line possible (file_type = pacorig or pacsave) is 72 chars including all spaces: " (V)iew, (S)kip, (R)emove pacorig, (O)verwrite with pacorig: [v/s/r/o] "
Make that 73 if you want to count the char the user has to enter.
How about"
printf " File differences found in %s\n" $file_type printf " (V)iew, (S)kip, (R)emove, (O)verwrite: [v/s/r/o] "
That doesn't mention what file will be removed (or overwritten) which I found pretty disturbing in the original script and which cause me to write all those patches.
If you think a little bit about it the only sane case is to remove the pacsave, pacnew, pacorig, but I still think being ambiguous here is a bad idea because we are dealing mostly with important files and users should be reassured that we are doing the right thing and they don't have to worry.
ah - it is just the extension printed. I thought it was the whole filename. (I will assume the whole filename is printed above this somewhere... too lazy to check!) Patchset looks fine to me.
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- contrib/pacdiff.sh.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 3461627..8480455 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -2,6 +2,7 @@ # pacdiff : a simple pacnew/pacorig/pacsave updater # # Copyright (c) 2007 Aaron Griffin <aaronmgriffin@gmail.com> +# Copyright (c) 2012 Pacman Development Team <pacman-dev@archlinux.org> # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -52,6 +53,7 @@ usage() { version() { printf "%s %s\n" "$myname" "$myver" echo 'Copyright (C) 2007 Aaron Griffin <aaronmgriffin@gmail.com>' + echo 'Copyright (C) 2012 Pacman Development Team <pacman-dev@archlinux.org>' } cmd() { -- 1.8.0.2
On 20/12/12 10:06, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> ---
Changed to 2013 given it is only getting applied now.
contrib/pacdiff.sh.in | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index 3461627..8480455 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -2,6 +2,7 @@ # pacdiff : a simple pacnew/pacorig/pacsave updater # # Copyright (c) 2007 Aaron Griffin <aaronmgriffin@gmail.com> +# Copyright (c) 2012 Pacman Development Team <pacman-dev@archlinux.org> # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -52,6 +53,7 @@ usage() { version() { printf "%s %s\n" "$myname" "$myver" echo 'Copyright (C) 2007 Aaron Griffin <aaronmgriffin@gmail.com>' + echo 'Copyright (C) 2012 Pacman Development Team <pacman-dev@archlinux.org>' }
cmd() {
On 20/12/12 10:06, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> --- Now uses tput to get the color codes.
Ack. Someone could pull this out into the scripts library given it is share with makepkg. Could be annoying with whatever m4 will do with all those backslashes etc...
contrib/pacdiff.sh.in | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/contrib/pacdiff.sh.in b/contrib/pacdiff.sh.in index bfafda2..4c6277d 100644 --- a/contrib/pacdiff.sh.in +++ b/contrib/pacdiff.sh.in @@ -24,6 +24,22 @@ diffprog=${DIFFPROG:-vimdiff} diffsearchpath=${DIFFSEARCHPATH:-/etc} locate=0
+if tput setaf 0 &>/dev/null; then + ALL_OFF="$(tput sgr0)" + BOLD="$(tput bold)" + BLUE="${BOLD}$(tput setaf 4)" + GREEN="${BOLD}$(tput setaf 2)" + YELLOW="${BOLD}$(tput setaf 3)" + PURPLE="${BOLD}$(tput setaf 5)" +else + ALL_OFF="\e[1;0m" + BOLD="\e[1;1m" + BLUE="${BOLD}\e[1;34m" + GREEN="${BOLD}\e[1;32m" + YELLOW="${BOLD}\e[1;33m" + PURPLE="${BOLD}\e[1;35m" +fi + usage() { echo "$myname : a simple pacnew/pacorig/pacsave updater" echo "Usage : $myname [-l]" @@ -62,7 +78,15 @@ fi # see http://mywiki.wooledge.org/BashFAQ/020 while IFS= read -u 3 -r -d '' pacfile; do file="${pacfile%.pac*}" - echo "File: $file" + file_type="pac${pacfile##*.pac}" + + case $file_type in + pacnew) printf "$GREEN%s$ALL_OFF" "$file_type";; + pacorig) printf "$YELLOW%s$ALL_OFF" "$file_type";; + pacsave) printf "$BLUE%s$ALL_OFF" "$file_type";; + esac + + printf " found for $PURPLE%s$ALL_OFF\n" "$file" if [ ! -f "$file" ]; then echo " $file does not exist" rm -i "$pacfile"
On 20/12/12 10:06, Florian Pritz wrote:
Signed-off-by: Florian Pritz <bluewind@xinu.at> ---
<snip>
@@ -62,7 +78,15 @@ fi # see http://mywiki.wooledge.org/BashFAQ/020 while IFS= read -u 3 -r -d '' pacfile; do file="${pacfile%.pac*}" - echo "File: $file" + file_type="pac${pacfile##*.pac}" + + case $file_type in + pacnew) printf "$GREEN%s$ALL_OFF" "$file_type";; + pacorig) printf "$YELLOW%s$ALL_OFF" "$file_type";; + pacsave) printf "$BLUE%s$ALL_OFF" "$file_type";; + esac + + printf " found for $PURPLE%s$ALL_OFF\n" "$file"
would this be better: printf " file found for $PURPLE%s$ALL_OFF\n" "$file" pacnew found for /etc/passwd vs pacnew file found for /etc/passwd
if [ ! -f "$file" ]; then echo " $file does not exist" rm -i "$pacfile"
participants (3)
-
Allan McRae
-
Dave Reisner
-
Florian Pritz