Re: [PATCH] Add new git-rm command with documentation

From: Junio C Hamano <junkio@cox.net>
Date: Feb 22, 2006, 08:19 UTC
Message-ID: <7vaccjst3x.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <87r75ws48c.wl%cworth@cworth.org>

Carl Worth <cworth@cworth.org> writes:

Show 10 quoted lines

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