On Tue, Aug 02, 2011 at 02:31:47PM -0700, Junio C Hamano wrote:
> Clemens Buchacher <drizzd@xxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html