Re: [PATCH] stash: prevent warning about null bytes in input

[Kevin Daudt <me@ikke.info>]

On Mon, Aug 14, 2017 at 12:51:26PM -0700, Junio C Hamano wrote:
> Kevin Daudt <me@ikke.info> writes:
> 
> > The no_changes function calls the untracked_files function through
> > command substitution. untracked_files will return null bytes because it
> > runs ls-files with the '-z' option.
> >
> > Bash since version 4.4 warns about these null bytes. As they are not
> > required for the test that is being done, remove null bytes from the
> > input.
> 
> That's an interesting one ;-)
> 
> I wonder if you considered giving an option to untracked_files
> helper function, though.  After all, it has only two callers,
> and it feels a bit suboptimal to ask the command to do a special
> thing (i.e. "-z") only to clean it up with a pipe.

As a matter of fact, I did not consider that option. I do agree that's a
much better approach.

> 
> IOW, something along the lines of (totally untested)...
> 

How should I proceed with this? Resubmit it after testing with the
appropriate attribution?


>  git-stash.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index 9b6c2da7b4..5f09a47f0a 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -43,9 +43,16 @@ no_changes () {
>  }
>  
>  untracked_files () {
> +	if test "$1" = "-z"
> +	then
> +		shift
> +		z=-z
> +	else
> +		z=
> +	fi
>  	excl_opt=--exclude-standard
>  	test "$untracked" = "all" && excl_opt=
> -	git ls-files -o -z $excl_opt -- "$@"
> +	git ls-files -o $z $excl_opt -- "$@"
>  }
>  
>  clear_stash () {
> @@ -114,7 +121,7 @@ create_stash () {
>  		# Untracked files are stored by themselves in a parentless commit, for
>  		# ease of unpacking later.
>  		u_commit=$(
> -			untracked_files "$@" | (
> +			untracked_files -z "$@" | (
>  				GIT_INDEX_FILE="$TMPindex" &&
>  				export GIT_INDEX_FILE &&
>  				rm -f "$TMPindex" &&
> 
>