Re: [PATCH 1/6] submodule foreach: correct path display in recursive submodules

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 29, 2016, 23:49 UTC
Message-ID: <xmqqshz8vo8k.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <1459292558-5840-2-git-send-email-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

> The new test replicates the previous test with the difference of executing
> from a sub directory. By executing from a sub directory all we would
> expect all displayed paths to be prefixed by '../'.

-ECANTPARSE on the last sentence.  Remove the first "all" perhaps?

As this patch is no longer about a new test but is about a fix of a problem, for which a new test serves as a typical example, it sounds somewhat funny to start the log message with description of the test.

Show 7 quoted lines

> Prior to this patch the test would report
>     Entering 'nested1/nested2/../nested3'
> instead of the expected
>     Entering '../nested1/nested2/nested3'
>
> because the prefix is put unconditionally in front and after that a
> computed display path with is affected by `wt_prefix`.

-ECANTPARSE even though I can guess what you want to say.

Show 10 quoted lines

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 43c68de..2838069 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -417,10 +417,11 @@ cmd_foreach()
>  			say "$(eval_gettext "Entering '\$prefix\$displaypath'")"
>  			name=$(git submodule--helper name "$sm_path")
>  			(
> -				prefix="$prefix$sm_path/"
> +				prefix="$(relative_path $prefix$sm_path)/"

What happens when prefix or sm_path has $IFS whitespace? Imitate the correct quoting you do three lines below, i.e.

	prefix=$(relative_path "$prefix$sm_path")/

Show 7 quoted lines

>  				clear_local_git_env
>  				cd "$sm_path" &&
>  				sm_path=$(relative_path "$sm_path") &&
> +				wt_prefix=
>  				# we make $path available to scripts ...
>  				path=$sm_path &&
>  				if test $# -eq 1