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

From: John Keeping <[email-masked]>
Date: Fri, 19 Apr 2013 08:46:32 +0100

On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:
> John Keeping <[email-masked]> 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.

> >  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.