Thread (23 messages) 23 messages, 4 authors, 2018-05-02

Re: [PATCH 2/2] submodule: Add --dissociate option to add/update commands

From: Eric Sunshine <hidden>
Date: 2018-05-01 20:25:30

On Tue, May 1, 2018 at 2:09 PM, Casey Fitzpatrick [off-list ref] wrote:
submodule: Add --dissociate option to add/update commands
s/Add/add/
quoted hunk ↗ jump to hunk
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 <redacted>
---
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
quoted hunk ↗ jump to hunk
@@ -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/
quoted hunk ↗ jump to hunk
@@ -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/
quoted hunk ↗ jump to hunk
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.)
quoted hunk ↗ jump to hunk
@@ -493,6 +497,9 @@ cmd_update()
+               --dissociate)
+                       dissociate="--dissociate"
+                       ;;
@@ -550,6 +557,7 @@ cmd_update()
                ${reference:+"$reference"} \
+               ${dissociate:+"$dissociate"} \
                ${depth:+--depth "$depth"} \
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help