Re: [PATCH v3 2/2] submodule: drop the top-level requirement

From: John Keeping <john@keeping.me.uk>
Date: Apr 19, 2013, 07:46 UTC
Message-ID: <20130419074632.GC2278@serenity.lan>
In-Reply-To: <7vfvyn4g46.fsf@alter.siamese.dyndns.org>

On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:

Show 24 quoted lines

> John Keeping <john@keeping.me.uk> writes:
> 
> > +relative_path ()
> > +{
> > +	local target curdir result
> > +	target=$1
> > +	curdir=${2-$wt_prefix}
> > +	curdir=${curdir%/}
> > +	result=
> > +
> > +	while test -n "$curdir"
> > +	do
> > +		case "$target" in
> > +		"$curdir/"*)
> > +			target=${target#$curdir/}
> > +			break
> > +			;;
> > +		esac
> 
> Could $curdir have glob wildcard to throw this part of the logic
> off?  It is OK to have limitations like "you cannot have a glob
> characters in your path to submodule working tree" (at least until
> we start rewriting these in C or Perl or Python), but we need to be
> aware of them.

I think the use of "#" instead of "##" saves us here because even with a wildcard in $curdir the case statement matches literally, so we know that "$target" starts with "$curdir/", so "${target#$curdir/}" can't match anything longer than the literal "$curdir" prefix.

Show 20 quoted lines

> >  module_list()
> >  {
> > +	eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"
> 
> An efficient reuse of "--" ;-)
> 
> > +test_expect_success 'run summary from subdir' '
> > +	mkdir sub &&
> > +	(
> > +		cd sub &&
> > +		git submodule summary >../actual
> > +	) &&
> > +	cat >expected <<-EOF &&
> > +* ../sm1 0000000...$head1 (2):
> > +  > Add foo2
> > +
> > +EOF
> 
> It somewhat looks strange to start with "<<-EOF" and then not to
> indent the body nor EOF.

Yes, but I copied this from the preceding test. I'd rather leave this as it is for consistency and let later style fixes change all of the tests in this file.