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

Kevin Daudt [off-list ref] writes:

On Mon, Aug 14, 2017 at 12:51:26PM -0700, Junio C Hamano wrote:

quoted

Kevin Daudt [off-list ref] writes:

quoted

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.

quoted

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.

quoted

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