From: Junio C Hamano Date: Tue, 22 Mar 2016 21:38:59 GMT Subject: Re: [PATCH] submodule helper: accept '.' for repositories with no submodules Message-ID: In-Reply-To: Stefan Beller writes: >> 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;