Re: [PATCH v2 1/2] filter-branch: stop special-casing $filter_subdir argument

[Johannes Sixt <j.sixt@viscovery.net>]

Thomas Rast schrieb:
> Well, I just observed while writing the patch that you cannot say
> 
>   git filter-branch --subdirectory-filter subdir -- --all -- subdir/file

Please include this in the commit message.

> Johannes Sixt wrote:
>> Thomas Rast schrieb:
>>> @@ -257,15 +257,29 @@ git read-tree || die "Could not seed the index"
>>>  # map old->new commit ids for rewriting parents
>>>  mkdir ../map || die "Could not create map/ directory"
>>>  
>>> +non_ref_args=$(git rev-parse --no-revs --sq "$@")
>>> +dashdash=--
>>> +for arg in "$non_ref_args"; do
>> At this point $non_ref_args might contain one or more IFS-separatable
>> words, but if you say "$non_ref_args" here, this loop will be entered
>> exactly once. But even if you drop the dquotes, the --sq quoting that you
>> requested from rev-parse bought you nothing.
> 
> Hrm.  Ok, so the ".." were clearly in mistake, but why could I remove
> the --sq?  Doesn't the shell expand the arguments provided by
> $non_ref_args if I use it without quotes nor --sq, so that it might
> accidentally expand paths or such?

When the shell expands $variable (outside quotes), it does not apply
quotes anymore, but only word-splits using $IFS. In your code, the words
would contain literal single-quotes, and paths with spaces would still be
split into words.

Wouldn't it be sufficient to just check whether any non-rev arguments are
present, and to suppress '--' if there are, like:

dashdash=
test -z "$(git rev-parse --no-revs "$@")" && dashdash=--

OK, this still leaves you with the problem that you want to separate
non-rev arguments from rev arguments. Right?

For this I suggest that you extract revs into a regular variable (because
the SHA1s can be word-split in a predictable way), and that you leave the
non-rev arguments in $@:

revs=$(git rev-parse --revs "$@")	# don't know if this works
eval set -- "$(git rev-parse --no-revs --sq "$@")"	# dquotes?

or so... This way you 'eval' should not be needed in later code.

-- Hannes