From: Junio C Hamano <junkio@cox.net>
Date: Wed, 22 Feb 2006 08:19:46 GMT
Subject: Re: [PATCH] Add new git-rm command with documentation
Message-ID: <7vaccjst3x.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <87r75ws48c.wl%cworth@cworth.org>

Carl Worth <cworth@cworth.org> writes:

> +files=$(
> +    if test -f "$GIT_DIR/info/exclude" ; then
> +	git-ls-files \
> +	    --exclude-from="$GIT_DIR/info/exclude" \
> +	    --exclude-per-directory=.gitignore -- "$@"
> +    else
> +	git-ls-files \
> +	--exclude-per-directory=.gitignore -- "$@"
> +    fi | sort | uniq
> +)

Note you are not using -z, which means we will c-quote the funny
characters in the output...

> +case "$show_only" in
> +true)
> +	echo $files
> +	;;

And here $files lack surrounding double quote.  For human
consumption it might be OK, but I somehow care about a bit of
details like this.

> +*)
> +	[[ "$remove_files" = "true" ]] && rm -- $files

Same here. What happens to filenames with IFS letters in them?
"git-add" does not use -z and xargs -0 without a good reason.

> +	git-update-index $index_remove_option $verbose $files
> +	;;
> +esac

Even if rm -- $files were quoted correctly, and tried to remove
the right files, if some of the files failed to disappear for
whatever reason, what happens?