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

Derrick Stolee [off-list ref] writes:

On 9/23/2021 9:39 AM, Derrick Stolee wrote:

quoted

On 9/22/2021 7:13 PM, Junio C Hamano wrote:

quoted

"Derrick Stolee via GitGitGadget" [off-list ref] writes:

quoted

+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.