Re: [PATCH 1/2] git-submodule: replace duplicated code with a module_list function

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 1/2] git-submodule: replace duplicated code with a module_list function

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:12

David Aguilar [off-list ref] writes:

quoted hunk

@@ -54,6 +54,15 @@ resolve_relative_url ()
 }
 
 #
+# Get submodule info for registered submodules
+# $@ = path to limit submodule list
+#
+module_list()
+{
+	git ls-files --stage -- "$@" | grep '^160000 '
+}
+
+#
 # Map submodule path to submodule name
 #
 # $1 = path
@@ -206,7 +215,7 @@ cmd_add()
 #
 cmd_foreach()
 {
-	git ls-files --stage | grep '^160000 ' |
+	module_list |

Thanks.

I think the original "foreach" implementation does not pay attention to
"$@" not by design but by mistake, and we should pass "$@" here as well.

Other than that I do not see anything obviously wrong with the patch.

Mark?

Re: [PATCH 1/2] git-submodule: replace duplicated code with a module_list function

From: Mark Levedahl <hidden>
Date: 2016-06-15 22:45:12

Junio C Hamano wrote:

quoted

 cmd_foreach()
 {
-	git ls-files --stage | grep '^160000 ' |
+	module_list |

Thanks.

I think the original "foreach" implementation does not pay attention to
"$@" not by design but by mistake, and we should pass "$@" here as well.

Other than that I do not see anything obviously wrong with the patch.

Mark?

Actually, this was by design, not mistake, though we did not discuss 
this at all. I'm not sure what the semantics would / should be: first of 
all, some part of "$@" is the command to be executed in each submodule, 
and as written "$@" in its entirety is what is used. Also, as written 
and documented, foreach operates in each checked out submodule, not a 
subset. I guess the basic questions are:
a)  What specific option or options to git ls-files makes sense here?
b) How do we distinguish the ls-files options from the command to be 
executed?

Mark

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help