[pacman-dev] [PATCH] contrib/pacdiff : big rework and cleanup

[Dan McGee]

On Tue, Feb 17, 2009 at 4:44 PM, Xavier Chantry <shiningxc at gmail.com> wrote:
> I initially only wanted to add a -l/--locate option to use locate instead of
> find, which should have been easy.
>
> Then I thought I would try to support filename with whitespaces while I was
> at it, and this was a bit more complex.
>
> Then I asked for some help on #bash, and I received a lot of criticism on
> the script :) I tried to address them all, so hopefully that script is in a
> better state now.

I think some of the developers on our side are going to think
differently than them. :)

> Signed-off-by: Xavier Chantry <shiningxc at gmail.com>
> ---
>  contrib/pacdiff |   64 +++++++++++++++++++++++++++++++++---------------------
>  1 files changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/contrib/pacdiff b/contrib/pacdiff
> index 6493649..f9c29fb 100755
> --- a/contrib/pacdiff
> +++ b/contrib/pacdiff
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash
>  #   pacdiff : a simple pacnew/pacorig/pacsave updater for /etc/
>  #
>  #   Copyright (c) 2007 Aaron Griffin <aaronmgriffin at gmail.com>
> @@ -17,29 +17,43 @@
>  #   along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  #
>
> -# Original http://phraktured.net/config/bin/pacdiff
> -
>  diffprog=${DIFFPROG:-vimdiff}
> -for x in $(find /etc/ -name "*.pacnew" -o -name "*.pacorig" -o -name "*.pacsave")
> -do
> -    echo "File: ${x%.pac*}"
> -    chk="$(cmp $x ${x%.pac*})"
> -    if [ -z "${chk}" ]; then
> -        echo "  Files are identical, removing..."
> -        rm $x
> -    else
> -        echo -n "  File differences found. (V)iew, (S)kip, (R)emove: [v/s/r] "
> -        read c
> -        c="$(echo $c| tr A-Z a-z)" #tolower
> -        if [ "$c" = "r" ]; then
> -            rm $x
> -        elif [ "$c" = "s" ]; then
> -            continue
> -        else
> -            [ "$c" = "n" -o "$c" = "N" ] || $diffprog $x ${x%.pac*}
> -            echo -n "  Remove file? [Y/n] "
> -            read c
> -            [ "$c" = "n" -o "$c" = "N" ] || rm $x
> -        fi
> -    fi
> +
> +case $1 in
> +       -l|--locate)
> +       cmd() { locate -0 -e -b \*.pacnew \*.pacorig \*.pacsave; }
> +       ;;
> +       *)
> +       cmd() { find /etc/ -name \*.pacnew -o -name \*.pacorig -o -name \*.pacsave -print0; }
> +       ;;
> +esac
Isn't this over-complex? It just seems like overkill to me. I'd rather
have the argument searching be in one place, and then cmd() does an
if/else based on the result of that (similar to makepkg, although they
really seemed to hate on that in #bash).

> +
> +exec 3< <(cmd)
This is a bit...sick. I agree with Aaron here.

> +while IFS= read -r -d '' pacfile <&3; do
> +       file="${pacfile%.pac*}"
> +       echo "File: $file"
> +       if [ ! -f "$file" ]; then
> +               echo "  $file does not exist"
> +               rm -i "$pacfile"
> +               continue
> +       fi
> +       check="$(cmp "$pacfile" "$file")"
> +       if [ -z "${check}" ]; then
> +               echo "  Files are identical, removing..."
> +               rm "$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 ;;
> +                               v|V)
> +                               $diffprog "$pacfile" "$file"
> +                               rm -i "$pacfile"; break ;;
> +                               s|S) break ;;
> +                               *) echo -n "  Invalid answer. Try again: [v/s/r] "; continue ;;
> +                       esac
> +               done
> +       fi
>  done
> +exec 3>&-
> +exit 0
> --

The rest seems fine.

-Dan