Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: >> > git archive --format=zip >with_file_with_content.zip \ >> > + --add-file-with-content=\"$QUOTED\": \ >> > --add-file-with-content=hello:world $EMPTY_TREE && >> > test_when_finished "rm -rf tmp-unpack" && >> > mkdir tmp-unpack && ( >> > cd tmp-unpack && >> > "$GIT_UNZIP" ../with_file_with_content.zip && >> > test_path_is_file hello && >> > + test_path_is_file $QUOTED && >> >> Looks OK, even though it probably is a good idea to have dq around >> $QUOTED, so that future developers can easily insert SP into its >> value to use a bit more common but still a bit more problematic >> pathnames in the test. > > I actually decided against this because reading > > "$QUOTED" > > would mislead future me to think that the double quotes that enclose > $QUOTED are the quotes that the variable's name talks about. But the > quotes are actually the escaped ones that are passed to `git archive` > above. > > So, to help future Dscho should they read this code six months from now or > even later, I wanted to specifically only add quotes to the `git archive` > call to make the intention abundantly clear. If you find "$QUOTED" misleads any reader to think QUOTED may have some quote characters in there, you could rename it, of course, to signal what the value is (e.g. $PATHNAME) better. But I think you misunderstood my comment completely. What I meant was to write these lines like: --add-file-with-content=\""$QUOTED"\": test_path_is_file "$QUOTED" Because the value in QUOTED can have $IFS whitespaces in it (after all, allowing random letters like colon, quotes and whitespaces is why we are adding this unquote_c_style() call), and without the extra double quotes to protect the parameter expansion of $QUOTED, the command line is broken. So, don't decide against it; the reasoning behind that decision is simply wrong. Thanks.
