Re: [PATCH v2] ls-files: add pathspec matching for submodules
- From
- Brandon Williams <bmwill@google.com>
- Date
- Sep 20, 2016, 21:03 UTC
- Message-ID
- <CAKoko1qS0+DgnMeNnjayEK3sWnvpuiS4oRDBSR=6s8i4okQ_Hw@mail.gmail.com>
- In-Reply-To
- <CAKoko1oU+QR61Vy0eSxaRe_w8u4q_bC9gx9H7oMqH=CwNzBVCA@mail.gmail.com>
Show 28 quoted lines
>>> +
>>> + if (item->flags & PATHSPEC_ONESTAR) {
>>> + return WM_MATCH;
>>> + } else if (item->magic & PATHSPEC_GLOB) {
>>> + return wildmatch(pattern, string,
>>> + WM_PATHNAME |
>>> + (item->magic & PATHSPEC_ICASE ?
>>> + WM_CASEFOLD : 0),
>>> + NULL);
>>
>> Isn't this last one overly tight? I am wondering about a scenario
>> where you have a submodule at "sub/" in the superproject, and "sub/"
>> has a "file" at the top of its working tree. And you do:
>>
>> git ls-files --recurse-submodules ':(glob)??b/fi?e'
>>
>> at the top of the superproject. The "pattern" would be '??b/fi?e"
>> while string would be 'sub', and wildmatch() would not like it, but
>> there is no way for this caller to append anything to 'sub' before
>> making this call, as it hasn't looked into what paths appear in the
>> submodule repository (and it should not want to). And I think we
>> would want it to recurse to find sub/file. IOW, this looks like a
>> false negative we must avoid in this function. As we cannot afford
>> to check if anything that matches 'fi?e' is in the index file of the
>> submodule repository, we shouldn't try to match 'fi?e' portion of
>> the given pathspec pattern.
>
> good point. Let me think about this some more.On a similar but slightly different note. In general do we want the pathspec '??b' to match against the sib/ directory and subsequently have ls-files print all entries inside of the sib/ directory? (this is in the non-recursive case)