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

[Junio C Hamano <gitster@pobox.com>]

Kevin Daudt <me@ikke.info> writes:

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

Sure.  

An appropriate attribution would be a "Helped-by: me" at most, but I
do not think for something this small it may not even bee needed.

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