renaming pathspec_prefix (was: Re: [PATCH v2] commit: allow partial commits with relative paths)

From: Clemens Buchacher <drizzd@aon.at>
Date: Sep 4, 2011, 10:41 UTC
Message-ID: <1315132921-26949-1-git-send-email-drizzd@aon.at>
In-Reply-To: <7vbow7ebzw.fsf@alter.siamese.dyndns.org>

On Tue, Aug 02, 2011 at 02:31:47PM -0700, Junio C Hamano wrote:

Show 25 quoted lines

> Clemens Buchacher <drizzd@aon.at> writes:
> 
> > diff --git a/setup.c b/setup.c
> > index 5ea5502..2c51a9a 100644
> > --- a/setup.c
> > +++ b/setup.c
> > @@ -264,6 +264,38 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
> >  	return pathspec;
> >  }
> >  
> > +const char *pathspec_prefix(const char *prefix, const char **pathspec)
> > +{
> 
> As a public function, this sorely needs a comment that describes what it
> does. More importantly, when I tried to come up with an example
> description, it became very clear that this neither prefixes anything to
> pathspec, nor prefixes pathspec to anything else.
> 
> As an internal helper in ls-files implementation it was perfectly
> fine that the function was slightly misnamed, but if you are making it
> into a public API, we should get its name right.
> 
> Perhaps "common_prefix()"?
> 
> Don't you also want to consolidate dir.c:common_prefix() with this?

Yes. This should do it:

[PATCH 1/3] remove prefix argument from pathspec_prefix [PATCH 2/3] consolidate pathspec_prefix and common_prefix [PATCH 3/3] rename pathspec_prefix -> common_prefix and move to

> What happens when pathspec has the recently introduced magic elements,
> e.g. ':/' that widens the match to the whole tree?

If I understand correctly that is resolved in get_pathspec. And pathspec_prefix (now common_prefix) is called later.

Clemens