On Thu, Mar 24, 2016 at 4:34 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
> The usual early machinery of Git is to change the directory to
> the top level of the working tree and pass the actual path inside
> the working tree as `prefix` to the command being run.
> This is the case both for commands written in C (where the
> prefix is passed into the command in a function parameter) as
> well as in git-submodule.sh where the setup code runs
>
> wt_prefix=$(git rev-parse show-prefix)
> cd_to_top_level
>
> So the prefix passed into the `submodule--helper list` is actually
> the relative path inside the working tree, but we were not using
> the standard way of passing it through.
>
> Adhere to Gits standard of passing the relative path inside the
> working tree by passing it via -C.
>
> We do not need to pass it for `submodule foreach` as that command
> doesn't take further arguments ('$@') to operate on a subset of
> submodules, such that it is irrelevant for listing the submodules.
> The computation of the displaypath ('Entering <path>') is done
> separately there.
>
> Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> ---
It is nice to see the format for doing this standardized, and reduce
extra code in the submodule--helper. I had wondered why we used
--prefix before.
Reviewed-by: Jacob Keller <jacob.e.keller@xxxxxxxxx>
> builtin/submodule--helper.c | 5 +----
> git-submodule.sh | 12 ++++++------
> 2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index ed764c9..2983783 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -68,14 +68,11 @@ static int module_list(int argc, const char **argv, const char *prefix)
> struct module_list list = MODULE_LIST_INIT;
>
> struct option module_list_options[] = {
> - OPT_STRING(0, "prefix", &prefix,
> - N_("path"),
> - N_("alternative anchor for relative paths")),
> OPT_END()
> };
>
> const char *const git_submodule_helper_usage[] = {
> - N_("git submodule--helper list [--prefix=<path>] [<path>...]"),
> + N_("git submodule--helper list [<path>...]"),
> NULL
> };
>
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 6b18a03..1f7ad6e 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -407,7 +407,7 @@ cmd_foreach()
> # command in the subshell (and a recursive call to this function)
> exec 3<&0
>
> - git submodule--helper list --prefix "$wt_prefix"|
> + git submodule--helper list |
> while read mode sha1 stage sm_path
> do
> die_if_unmatched "$mode"
> @@ -467,7 +467,7 @@ cmd_init()
> shift
> done
>
> - git submodule--helper list --prefix "$wt_prefix" "$@" |
> + git -C "$wt_prefix" submodule--helper list "$@" |
> while read mode sha1 stage sm_path
> do
> die_if_unmatched "$mode"
> @@ -549,7 +549,7 @@ cmd_deinit()
> die "$(eval_gettext "Use '.' if you really want to deinitialize all submodules")"
> fi
>
> - git submodule--helper list --prefix "$wt_prefix" "$@" |
> + git -C "$wt_prefix" submodule--helper list "$@" |
> while read mode sha1 stage sm_path
> do
> die_if_unmatched "$mode"
> @@ -683,7 +683,7 @@ cmd_update()
> fi
>
> cloned_modules=
> - git submodule--helper list --prefix "$wt_prefix" "$@" | {
> + git -C "$wt_prefix" submodule--helper list "$@" | {
> err=
> while read mode sha1 stage sm_path
> do
> @@ -1121,7 +1121,7 @@ cmd_status()
> shift
> done
>
> - git submodule--helper list --prefix "$wt_prefix" "$@" |
> + git -C "$wt_prefix" submodule--helper list "$@" |
> while read mode sha1 stage sm_path
> do
> die_if_unmatched "$mode"
> @@ -1199,7 +1199,7 @@ cmd_sync()
> esac
> done
> cd_to_toplevel
> - git submodule--helper list --prefix "$wt_prefix" "$@" |
> + git -C "$wt_prefix" submodule--helper list "$@" |
> while read mode sha1 stage sm_path
> do
> die_if_unmatched "$mode"
> --
> 2.8.0.rc4.10.g52f3f33
>
Regards,
Jake
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html