Re: [PATCH 3/3] stash: support filename argument

Thomas Gummerer [off-list ref] writes:

Does $1 need to be quoted to prevent it from split at $IFS?

Hmph.  What is this "no-op" option about?  Did you mean to say
something like this instead?

	case "$1" in
	...
	--file)
		case $# in
		1)
			die "--file needs a pathspec" ;;
		*)
			shift
			files="$files$1 " ;;
		esac
		;;


Another thing I noticed.  We won't support filenames with embedded
$IFS characters at all?

I somehow had an impression that the script was carefully done
(e.g. by using -z option where appropriate) to add such a
limitation.

Perhaps we have broken it over time and it no longer matters
(i.e. there already may be existing breakages), but this troubles
me somehow.

By the way, in addition to "push" thing that corrects the argument
convention by requiring "-m" before the message, we need to correct
create_stash that is used internally from "stash push" somehow?

On 01/23, Junio C Hamano wrote:

Thomas Gummerer [off-list ref] writes:

quoted

diff --git a/git-stash.sh b/git-stash.sh
index d6b4ae3290..7dcce629bd 100755
--- a/git-stash.sh
+++ b/git-stash.sh

@@ -41,7 +41,7 @@ no_changes () {
 untracked_files () {
 	excl_opt=--exclude-standard
 	test "$untracked" = "all" && excl_opt=
-	git ls-files -o -z $excl_opt
+	git ls-files -o -z $excl_opt -- $1

Does $1 need to be quoted to prevent it from split at $IFS?

Not sure, I'll check and add a test for the re-roll.

quoted

@@ -56,6 +56,23 @@ clear_stash () {
 }
 
 create_stash () {
+	files=
+	while test $# != 0
+	do
+		case "$1" in
+		--)
+			shift
+			break
+			;;
+		--files)
+			;;
+		*)
+			files="$1 $files"
+			;;

Hmph.  What is this "no-op" option about?  Did you mean to say
something like this instead?

	case "$1" in
	...
	--file)
		case $# in
		1)
			die "--file needs a pathspec" ;;
		*)
			shift
			files="$files$1 " ;;
		esac
		;;

Hmm that would require multiple --file arguments to create_stash,
which I wanted to avoid.  But probably the correct solution is to
introduce a new format for create_stash, which allows a -m before the
message, and uses -- to disambiguate before the file name arguments.

This would be similar to what Johannes suggested in [1], deprecating
the old syntax in git stash create.  While this didn't work in git
stash save, it would work in git stash create, as "--" isn't used to
disambiguate anything there yet.

Another thing I noticed.  We won't support filenames with embedded
$IFS characters at all?

I somehow had an impression that the script was carefully done
(e.g. by using -z option where appropriate) to add such a
limitation.

Perhaps we have broken it over time and it no longer matters
(i.e. there already may be existing breakages), but this troubles
me somehow.

Good point, I didn't think about $IFS characters.  Fill fix in the
next round.

By the way, in addition to "push" thing that corrects the argument
convention by requiring "-m" before the message, we need to correct
create_stash that is used internally from "stash push" somehow?

Yeah, I think that would make sense, see above.

[1]: http://public-inbox.org/git/alpine.DEB.2.20.1701241148300.3469@virtualbox/