git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] submodule helper: accept '.' for repositories with no submodules

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 22, 2016, 21:38 UTC
Message-ID
<xmqqtwjyfb0s.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<CAGZ79ka=EJ-L+Zaqh0kWUdjmyUva3fEcbwJn18uxshe3R5ABBQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
Show 9 quoted lines
>> I do not think that buys us much.  You have already shown how to
>> implement "filter first and then pathspec match" if a caller
>> wants to (which turned out to be a regression in this case, but
>> that is besides the point).
>
> And by including this filtering into the pathspec machine we can pass
> a flag DONT_BREAK_ON_NO_FILTER_RESULTS_WHEN_HAVING_OTHER_MATCHES
> (name for illustration purpose only ;) which is how I understand this
> regression?

But you do not even need that if you fix the regression with something like this, no? Do we need to add complexity to pathspec machinery only to make it easier to misuse it and then add another DONT_BREAK_... band-aid to fix a bug that can come from such a misuse?

 builtin/submodule--helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index ed764c9..740b57a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -37,10 +37,11 @@ static int module_list_compute(int argc, const char **argv,
 	for (i = 0; i < active_nr; i++) {
 		const struct cache_entry *ce = active_cache[i];
 
-		if (!S_ISGITLINK(ce->ce_mode) ||
-		    !match_pathspec(pathspec, ce->name, ce_namelen(ce),
+		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce),
 				    0, ps_matched, 1))
 			continue;
+		if (!S_ISGITLINK(ce->ce_mode))
+			continue;
 
 		ALLOC_GROW(list->entries, list->nr + 1, list->alloc);
 		list->entries[list->nr++] = ce;
Previous: Stefan BellerNext: Stefan Beller
Message 6 of 10 in “submodule helper: accept '.' for repositories with no submodules”
  1. submodule helper: accept '.' for repositories with no submodulesStefan Beller, Mar 22, 2016
  2. Junio C HamanoMar 22, 2016
  3. Stefan BellerMar 22, 2016
  4. Junio C HamanoMar 22, 2016
  5. Stefan BellerMar 22, 2016
  6. Junio C HamanoMar 22, 2016
  7. Stefan BellerMar 22, 2016
  8. Junio C HamanoMar 22, 2016
  9. Stefan BellerMar 22, 2016
  10. Junio C HamanoMar 22, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.