On 4/18/2022 1:23 PM, Ævar Arnfjörð Bjarmason wrote:
> Add a path_match_flags() function and have the two sets of
> starts_with_dot_{,dot_}slash() functions added in
> 63e95beb085 (submodule: port resolve_relative_url from shell to C,
> 2016-04-15) and a2b26ffb1a8 (fsck: convert gitmodules url to URL
> passed to curl, 2020-04-18) be thin wrappers for it.
>
> As the latter of those notes the fsck version was copied from the
> initial builtin/submodule--helper.c version.
>
> Since the code added in a2b26ffb1a8 was doing really doing the same as
s/doing really doing/really doing/
> win32_is_dir_sep() added in 1cadad6f658 (git clone <url>
> C:\cygwin\home\USER\repo' is working (again), 2018-12-15) let's move
> the latter to git-compat-util.h is a is_xplatform_dir_sep(). We can
> then call either it or the platform-specific is_dir_sep() from this
> new function.
>
> Let's likewise change code in various other places that was hardcoding
> checks for "'/' || '\\'" with the new is_xplatform_dir_sep(). As can
> be seen in those callers some of them still concern themselves with
> ':' (Mac OS classic?), but let's leave the question of whether that
> should be consolidated for some other time.
This feels like it could be its own change before the refactor
of the starts_with_dot_{,dot}slash() functions. The diff is pretty
big and all over the place.
If you start with the addition of is_xplatform_dir_sep() (and maybe
the change of how is_dir_sep() is created) then the rest of the
change is more focused.
> As we expect to make wider use of the "native" case in the future,
> define and use two starts_with_dot_{,dot_}slash_native() convenience
> wrappers. This makes the diff in builtin/submodule--helper.c much
> smaller.
> +static int starts_with_dot_slash(const char *const path)
> +{
> + return starts_with_dot_slash_native(path);;
Double semi-colon.
> +int path_match_flags(const char *const str, const enum path_match_flags flags)
I feel like "path_match_flags()" is too generic of a name here.
Maybe something like "path_starts_with_dotslash_flags()" would be
sufficiently descriptive.
> +{
> + const char *p = str;
> +
> + if (flags & PATH_MATCH_NATIVE &&
> + flags & PATH_MATCH_XPLATFORM)
> + BUG("path_match_flags() must get one match kind, not multiple!");
> + else if (!(flags & PATH_MATCH_KINDS_MASK))
> + BUG("path_match_flags() must get at least one match kind!");
> +
> + if (flags & PATH_MATCH_STARTS_WITH_DOT_SLASH &&
> + flags & PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH)
> + BUG("path_match_flags() must get one platform kind, not multiple!");
> + else if (!(flags & PATH_MATCH_PLATFORM_MASK))
> + BUG("path_match_flags() must get at least one platform kind!");
These would be easier and more robust if we had a simple
popcount function. It's not worth extracting one out of
ewah/ewok.h just for this, though.
> + if (*p++ != '.')
> + return 0;
> + if (flags & PATH_MATCH_STARTS_WITH_DOT_DOT_SLASH &&
> + *p++ != '.')
> + return 0;
> +
> + if (flags & PATH_MATCH_NATIVE)
> + return is_dir_sep(*p);
> + else if (flags & PATH_MATCH_XPLATFORM)
> + return is_xplatform_dir_sep(*p);
> + BUG("unreachable");
> +}
Thanks,
-Stolee
