Subject: Re: [PATCH v2 1/2] path: add a function to check for path suffix

From: SZEDER Gábor <[email-masked]>
Date: Tue, 13 Aug 2019 08:36:18 +0200

On Mon, Aug 12, 2019 at 09:49:20AM -0700, Junio C Hamano wrote:
> "brian m. carlson" <[email-masked]> writes:
> 
> > On 2019-08-12 at 00:32:26, Junio C Hamano wrote:
> >> "brian m. carlson" <[email-masked]> writes:
> >> > +static ssize_t stripped_path_suffix_offset(const char *path, const char *suffix)
> >> 
> >> Perhaps
> >> 
> >>     static ssize_t last_path_component_offset(const char *path, const char *name)
> >> 
> >> I am tempted to also call the second parameter to this function
> >> "basename", as we know from the proposed log message that you wish
> >> "basename" were usable for this purpose, but basename itself has
> >> another confusing factor (i.e. "are we stripping ".exe" extension?",
> >> to which the answer is no in the context of these functions).
> >> 
> >> If we agree with the "last path component" phrasing, has_path_suffix()
> >> would become something like:
> >> 
> >>     int last_path_component_equals(const char *path, const char *name);
> >
> > Except this is not necessarily the last path component. It could match
> > one or more path components with the way the function is written.
> 
> That's fair.  Is the feature that allows the function called
> ends_with_component*S* like Szeder suggests designed one, i.e. with
> an explicit purpose of supporting callers that pass "foo/bar" as the
> "suffix" to it, or is it merely that the implementation happens to
> work that way, even though the expected use that is supported is to
> pass only one level component but the implementation did not even
> bother asserting that the "suffix" does not have a slash in it?

The plural in the suggested function name was intentional on my part,
even though in this callsite in question we are only interested in the
filename, i.e. only a single path component.

I was hoping that the names of these related functions will be kept in
sync, and all will somehow contain the "path_components" substring,
e.g. strip_path_suffix() becomes strip_suffix_path_components() or
something.  And that function must be able to handle multiple path
components, becase there is this callsite:

  exec-cmd.c:         !(prefix = strip_path_suffix(executable_dirname, GIT_EXEC_PATH)) &&

and the build sets '-DGIT_EXEC_PATH="libexec/git-core"' by default.