On Tue, May 1, 2018 at 2:09 PM, Casey Fitzpatrick <kcghost@xxxxxxxxx> wrote:
> submodule: Add --dissociate option to add/update commands
s/Add/add/
> Add --dissociate option to add and update commands, both clone helper commands
> that already have the --reference option --dissociate pairs with.
> Add documentation.
> Add tests.
>
> Signed-off-by: Casey Fitzpatrick <kcghost@xxxxxxxxx>
> ---
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> @@ -1075,6 +1075,9 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
> + if (dissociate) {
> + argv_array_push(&cp.args, "--dissociate");
> + }
Style: drop unnecessary braces
> @@ -1208,6 +1212,8 @@ static int module_clone(int argc, const char **argv, const char *prefix)
> + OPT_BOOL(0, "dissociate", &dissociate,
> + N_("use --reference only while cloning")),
s/reference/dissociate/
> @@ -1575,6 +1584,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)
> + OPT_BOOL(0, "dissociate", &suc.dissociate,
> + N_("use --reference only while cloning")),
s/reference/dissociate/
> diff --git a/git-submodule.sh b/git-submodule.sh
> + --dissociate)
> + dissociate="--dissociate"
> @@ -258,7 +262,7 @@ or you are unsure what this means choose another name with the '--name' option."
> - git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${depth:+"$depth"} || exit
> + git submodule--helper clone ${GIT_QUIET:+--quiet} ${progress:+"$progress"} --prefix "$wt_prefix" --path "$sm_path" --name "$sm_name" --url "$realrepo" ${reference:+"$reference"} ${dissociate:+"$dissociate"} ${depth:+"$depth"} || exit
I realize that you're just following existing practice in this script,
but it's a bit off-putting to see expansions for the new --progress
and --dissociate options being done via unnecessarily complex
${foobar:+"$foobar"} when the simpler $foobar would work just as well.
Just a comment; not necessarily a request for change. (A separate
preparatory cleanup patch which simplifies the existing complex
expansion expressions would be welcome but could also be considered
outside the scope of this patch series.)
> @@ -493,6 +497,9 @@ cmd_update()
> + --dissociate)
> + dissociate="--dissociate"
> + ;;
> @@ -550,6 +557,7 @@ cmd_update()
> ${reference:+"$reference"} \
> + ${dissociate:+"$dissociate"} \
> ${depth:+--depth "$depth"} \
