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

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

Clemens Buchacher [off-list ref] writes:

quoted

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