Re: [PATCHv2 2/2] submodule: only preserve flags across recursive status/update invocations

From: Kevin Ballard <kevin@sb.org>
Date: Nov 3, 2010, 05:45 UTC
Message-ID: <E0BA6B5C-AB98-4D72-8E8A-B4EA4B7A17CC@sb.org>
In-Reply-To: <20101103053827.GD10631@burratino>

On Nov 2, 2010, at 10:38 PM, Jonathan Nieder wrote:

Show 23 quoted lines

> Kevin Ballard wrote:
> 
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -374,41 +374,35 @@ cmd_init()
> [...]
>> 		--reference)
>> 			case "$2" in '') usage ;; esac
>> 			reference="--reference=$2"
>> -			shift 2
>> +			orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")"
>> +			shift
> 
> Hmm.  Maybe a helper would make it clearer.
> 
> 	save_arg () {
> 		orig_flags="$orig_flags $(git rev-parse --sq-quote "$1")"
> 	}
> 
> making this
> 
> +			save_arg --reference
> +			shift

I considered that, but I already dislike the fact that orig_flags is a global. I left it as such for the sake of not fixing what ain't broke, but the use of a helper function would imply that the global nature of the variable is intentional.

Show 9 quoted lines

>> --- a/t/t7407-submodule-foreach.sh
>> +++ b/t/t7407-submodule-foreach.sh
>> @@ -238,4 +238,23 @@ test_expect_success 'use "git clone --recursive" to checkout all submodules' '
>> 	test -d clone4/nested1/nested2/nested3/submodule/.git
>> '
>> 
>> +test_expect_success 'use "update --recursive nested1" to checkout all submodules rooted in nested1' '
> 
> Maybe a submodule status --cached --recursive -- <files> test, too?

Good idea.

-Kevin Ballard