Derrick Stolee <stolee@xxxxxxxxx> writes:
> 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?
When the focus and purpose of the function changes, it may warrant a
rename to include "matching" or "pattern", but not before.
Or we might be seeing a premature refactoring with these two steps.
Are we gaining multiple callers of this function before it gets
extended to care about pattern and matching? If not, perhaps
teaching the inlined codepath about the pattern and matching in
place first before extracting the code to a helper function for
readability and reusability may help make the resulting series
easier to follow, and we do not have to see a function with a
misleading name.
