[pacman-dev] [PATCH v2 1/5] pacdiff: color filename and mention what we found

Florian Pritz

20 Dec 2012 20 Dec '12

12:06 a.m.

Signed-off-by: Florian Pritz --- 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

Show replies by date

Florian Pritz

20 Dec 20 Dec

12:06 a.m.

New subject: [pacman-dev] [PATCH v2 2/5] pacdiff: be more verbose if we rename or remove

Doesn't hurt and reassures the user that we did the right thing. Signed-off-by: Florian Pritz --- 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

Florian Pritz

12:06 a.m.

New subject: [pacman-dev] [PATCH v2 3/5] pacdiff: check cmp's exit code rather than output

Signed-off-by: Florian Pritz --- 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

Florian Pritz

12:06 a.m.

New subject: [pacman-dev] [PATCH v2 4/5] pacdiff: Add option to overwrite, clarify remove option

Signed-off-by: Florian Pritz --- 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

Dave Reisner

12:19 a.m.

New subject: [pacman-dev] [PATCH v2 4/5] pacdiff: Add option to overwrite, clarify remove option

On Thu, Dec 20, 2012 at 01:06:36AM +0100, Florian Pritz wrote:

...

Signed-off-by: Florian Pritz --- 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

Allan McRae

10:31 a.m.

New subject: [pacman-dev] [PATCH v2 4/5] pacdiff: Add option to overwrite, clarify remove option

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 --- 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

Florian Pritz

28 Dec 28 Dec

3:18 p.m.

New subject: [pacman-dev] [PATCH v2 4/5] pacdiff: Add option to overwrite, clarify remove option

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 --- 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.

Allan McRae

2 Jan 2 Jan

1:30 a.m.

New subject: [pacman-dev] [PATCH v2 4/5] pacdiff: Add option to overwrite, clarify remove option

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 --- 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.

Florian Pritz

20 Dec 20 Dec

12:06 a.m.

New subject: [pacman-dev] [PATCH v2 5/5] pacdiff: update copyright notice

Signed-off-by: Florian Pritz --- 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 +# Copyright (c) 2012 Pacman Development Team # # 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 ' + echo 'Copyright (C) 2012 Pacman Development Team ' } cmd() { -- 1.8.0.2

Allan McRae

2 Jan 2 Jan

4:36 a.m.

New subject: [pacman-dev] [PATCH v2 5/5] pacdiff: update copyright notice

On 20/12/12 10:06, Florian Pritz wrote:

...

Signed-off-by: Florian Pritz ---

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 +# Copyright (c) 2012 Pacman Development Team # # 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 ' + echo 'Copyright (C) 2012 Pacman Development Team ' }

cmd() {

Allan McRae

4:31 a.m.

New subject: [pacman-dev] [PATCH v2 1/5] pacdiff: color filename and mention what we found

On 20/12/12 10:06, Florian Pritz wrote:

...

Signed-off-by: Florian Pritz --- 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"

Allan McRae

4:40 a.m.

New subject: [pacman-dev] [PATCH v2 1/5] pacdiff: color filename and mention what we found

On 20/12/12 10:06, Florian Pritz wrote:

...

Signed-off-by: Florian Pritz ---

<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"

4141

Age (days ago)

4154

Last active (days ago)

List overview

Download

11 comments

3 participants

participants (3)

Allan McRae
Dave Reisner
Florian Pritz

[pacman-dev] [PATCH v2 1/5] pacdiff: color filename and mention what we found

tags

participants (3)