From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 18 Apr 2013 22:40:41 GMT
Subject: Re: [PATCH v3 2/2] submodule: drop the top-level requirement
Message-ID: <7vfvyn4g46.fsf@alter.siamese.dyndns.org>
In-Reply-To: <6e4122f3eedec3f520028b5598b78e0d59e5d12b.1366314439.git.john@keeping.me.uk>

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.

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