On 9/23/2021 9:39 AM, Derrick Stolee wrote:
> On 9/22/2021 7:13 PM, Junio C Hamano wrote:
>> "Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>>
>>> +static int path_matches_dir_pattern(const char *pathname,
>>> + int pathlen,
>>> + int *dtype,
>>> + struct path_pattern *pattern,
>>> + struct index_state *istate)
>>> +{
>>> + *dtype = resolve_dtype(*dtype, istate, pathname, pathlen);
>>> + if (*dtype != DT_DIR)
>>> + return 0;
>>> +
>>> + return 1;
>>> +}
>>
>> The function name and parameter list have "pattern" but as far as I
>> can see any "matches" or "pattern" comes into the picture. The code
>> in the caller after calling this function may be doing pattern
>> matching, but not this one.
>>
>> What this helper is doing is "signal if the pathname in the working
>> tree is supposed to be a directory with the return value, while
>> filling *dtype with what kind of thing it is."
>>
>> path_must_be_dir_in_working_tree() or something, perhaps?
>
> Yes, a rename would be prudent here. Thanks.
Of course, when I go to amend the commit, the commit message says
We will expand the path_matches_dir_pattern() method in a following
change.
which means that more will follow that will actually care about the
pattern and matching as a directory.
After looking at the extension in the next patch, do you still think a
rename is necessary? Specifically, this diff hunk:
diff --git a/dir.c b/dir.c
index 652135df896..9ea6cfe61cb 100644
--- a/dir.c
+++ b/dir.c
@@ -1305,10 +1305,35 @@ int match_pathname(const char *pathname, int pathlen,
static int path_matches_dir_pattern(const char *pathname,
int pathlen,
+ struct strbuf **path_parent,
int *dtype,
struct path_pattern *pattern,
struct index_state *istate)
{
+ if (!*path_parent) {
+ char *slash;
+ CALLOC_ARRAY(*path_parent, 1);
+ strbuf_add(*path_parent, pathname, pathlen);
+ slash = find_last_dir_sep((*path_parent)->buf);
+
+ if (slash)
+ strbuf_setlen(*path_parent, slash - (*path_parent)->buf);
+ else
+ strbuf_setlen(*path_parent, 0);
+ }
+
+ /*
+ * If the parent directory matches the pattern, then we do not
+ * need to check for dtype.
+ */
+ if ((*path_parent)->len &&
+ match_pathname((*path_parent)->buf, (*path_parent)->len,
+ pattern->base,
+ pattern->baselen ? pattern->baselen - 1 : 0,
+ pattern->pattern, pattern->nowildcardlen,
+ pattern->patternlen, pattern->flags))
+ return 1;
+
*dtype = resolve_dtype(*dtype, istate, pathname, pathlen);
if (*dtype != DT_DIR)
return 0;
Thanks,
-Stolee
