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

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Oct 22, 2009, 08:31 UTC
Message-ID: <4AE0187C.4040608@viscovery.net>
In-Reply-To: <200910221005.11813.trast@student.ethz.ch>

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.

Show 18 quoted lines

> 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