Re: [PATCH v3 03/14] dir: extract directory-matching logic

> Derrick Stolee <stolee@gmail.com> 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@gmail.com> 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.