Re: [PATCH v2 06/45] Add parse_pathspec() that converts cmdline args to struct pathspec

diff --git a/Documentation/technical/api-setup.txt b/Documentation/technical/api-setup.txt
index 4f63a04..59a947e 100644
--- a/Documentation/technical/api-setup.txt
+++ b/Documentation/technical/api-setup.txt

@@ -8,6 +8,23 @@ Talk about
 * is_inside_git_dir()
 * is_inside_work_tree()
 * setup_work_tree()
-* get_pathspec()
 
 (Dscho)
+
+Pathspec
+========

+
+See glossary-context.txt for the syntax of pathspec. In memory, a
+pathspec set is represented by "struct pathspec" and is prepared by
+parse_pathspec(). This function takes several arguments:
+
+- magic_mask specifies what features that are NOT supported by the
+  following code. If a user attempts to use such a feature,
+  parse_pathspec() can reject it early.
+
+- flags specifies other things that the caller wants parse_pathspec to
+  perform.
+
+- prefix and args come from cmd_* functions
+
+get_pathspec() is obsolete and should never be used in new code.

diff --git a/dir.c b/dir.c
index 97ad45b..a442467 100644
--- a/dir.c
+++ b/dir.c

@@ -338,7 +338,7 @@ int match_pathspec_depth(const struct pathspec *ps,
 /*
  * Return the length of the "simple" part of a path match limiter.
  */
-static int simple_length(const char *match)
+int simple_length(const char *match)
 {
 	int len = -1;
 

@@ -350,7 +350,7 @@ static int simple_length(const char *match)
 	}
 }
 
-static int no_wildcard(const char *string)
+int no_wildcard(const char *string)
 {
 	return string[simple_length(string)] == '\0';
 }

diff --git a/dir.h b/dir.h
index c3eb4b5..89427fd 100644
--- a/dir.h
+++ b/dir.h

@@ -125,6 +125,8 @@ struct dir_struct {
 #define MATCHED_RECURSIVELY 1
 #define MATCHED_FNMATCH 2
 #define MATCHED_EXACTLY 3
+extern int simple_length(const char *match);
+extern int no_wildcard(const char *string);
 extern char *common_prefix(const char **pathspec);
 extern int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen);
 extern int match_pathspec_depth(const struct pathspec *pathspec,

diff --git a/pathspec.c b/pathspec.c
index ab53b8a..ebe9844 100644
--- a/pathspec.c
+++ b/pathspec.c

@@ -103,10 +103,6 @@ void die_if_path_beyond_symlink(const char *path, const char *prefix)
 /*
  * Magic pathspec
  *
- * NEEDSWORK: These need to be moved to dir.h or even to a new
- * pathspec.h when we restructure get_pathspec() users to use the
- * "struct pathspec" interface.
- *
  * Possible future magic semantics include stuff like:
  *
  *	{ PATHSPEC_NOGLOB, '!', "noglob" },

@@ -115,7 +111,6 @@ void die_if_path_beyond_symlink(const char *path, const char *prefix)
  *	{ PATHSPEC_REGEXP, '\0', "regexp" },
  *
  */
-#define PATHSPEC_FROMTOP    (1<<0)
 
 static struct pathspec_magic {
 	unsigned bit;

@@ -127,7 +122,7 @@ static struct pathspec_magic {
 
 /*
  * Take an element of a pathspec and check for magic signatures.
- * Append the result to the prefix.
+ * Append the result to the prefix. Return the magic bitmap.
  *
  * For now, we only parse the syntax and throw out anything other than
  * "top" magic.

@@ -138,10 +133,15 @@ static struct pathspec_magic {
  * the prefix part must always match literally, and a single stupid
  * string cannot express such a case.
  */
-static const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
+static unsigned prefix_pathspec(struct pathspec_item *item,
+				unsigned *p_short_magic,
+				const char **raw, unsigned flags,
+				const char *prefix, int prefixlen,
+				const char *elt)
 {
-	unsigned magic = 0;
+	unsigned magic = 0, short_magic = 0;
 	const char *copyfrom = elt;
+	char *match;
 	int i;
 
 	if (elt[0] != ':') {

@@ -183,7 +183,7 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char
 				break;
 			for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)
 				if (pathspec_magic[i].mnemonic == ch) {
-					magic |= pathspec_magic[i].bit;
+					short_magic |= pathspec_magic[i].bit;
 					break;
 				}
 			if (ARRAY_SIZE(pathspec_magic) <= i)

@@ -194,15 +194,128 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char
 			copyfrom++;
 	}
 
+	magic |= short_magic;
+	*p_short_magic = short_magic;
+
 	if (magic & PATHSPEC_FROMTOP)
-		return xstrdup(copyfrom);
+		match = xstrdup(copyfrom);
 	else
-		return prefix_path(prefix, prefixlen, copyfrom);
+		match = prefix_path(prefix, prefixlen, copyfrom);
+	*raw = item->match = match;
+	item->len = strlen(item->match);
+	if (limit_pathspec_to_literal())
+		item->nowildcard_len = item->len;
+	else
+		item->nowildcard_len = simple_length(item->match);
+	item->flags = 0;
+	if (item->nowildcard_len < item->len &&
+	    item->match[item->nowildcard_len] == '*' &&
+	    no_wildcard(item->match + item->nowildcard_len + 1))
+		item->flags |= PATHSPEC_ONESTAR;
+	return magic;
+}
+
+static int pathspec_item_cmp(const void *a_, const void *b_)
+{
+	struct pathspec_item *a, *b;
+
+	a = (struct pathspec_item *)a_;
+	b = (struct pathspec_item *)b_;
+	return strcmp(a->match, b->match);
+}
+
+static void NORETURN unsupported_magic(const char *pattern,
+				       unsigned magic,
+				       unsigned short_magic)
+{
+	struct strbuf sb = STRBUF_INIT;
+	int i, n;
+	for (n = i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+		const struct pathspec_magic *m = pathspec_magic + i;
+		if (!(magic & m->bit))
+			continue;
+		if (sb.len)
+			strbuf_addstr(&sb, " ");
+		if (short_magic & m->bit)
+			strbuf_addf(&sb, "'%c'", m->mnemonic);
+		else
+			strbuf_addf(&sb, "'%s'", m->name);
+		n++;
+	}
+	/*
+	 * We may want to substitue "this command" with a command
+	 * name. E.g. when add--interactive dies when running
+	 * "checkout -p"
+	 */
+	die(_("%s: pathspec magic not supported by this command: %s"),
+	    pattern, sb.buf);
+}
+
+/*
+ * Given command line arguments and a prefix, convert the input to
+ * pathspec. die() any magic in magic_mask is used.
+ */
+void parse_pathspec(struct pathspec *pathspec,
+		    unsigned magic_mask, unsigned flags,
+		    const char *prefix, const char **argv)
+{
+	struct pathspec_item *item;
+	const char *entry = argv ? *argv : NULL;
+	int i, n, prefixlen;
+
+	memset(pathspec, 0, sizeof(*pathspec));
+
+	/* No arguments, no prefix -> no pathspec */
+	if (!entry && !prefix)
+		return;
+
+	/* No arguments with prefix -> prefix pathspec */
+	if (!entry) {
+		static const char *raw[2];
+
+		pathspec->items = item = xmalloc(sizeof(*item));
+		memset(item, 0, sizeof(*item));
+		item->match = prefix;
+		item->nowildcard_len = item->len = strlen(prefix);
+		raw[0] = prefix;
+		raw[1] = NULL;
+		pathspec->nr = 1;
+		pathspec->raw = raw;
+		return;
+	}
+
+	n = 0;
+	while (argv[n])
+		n++;
+
+	pathspec->nr = n;
+	pathspec->items = item = xmalloc(sizeof(*item) * n);
+	pathspec->raw = argv;
+	prefixlen = prefix ? strlen(prefix) : 0;
+
+	for (i = 0; i < n; i++) {
+		unsigned short_magic;
+		entry = argv[i];
+
+		item[i].magic = prefix_pathspec(item + i, &short_magic,
+						argv + i, flags,
+						prefix, prefixlen, entry);
+		if (item[i].magic & magic_mask)
+			unsupported_magic(entry,
+					  item[i].magic & magic_mask,
+					  short_magic);
+		if (item[i].nowildcard_len < item[i].len)
+			pathspec->has_wildcard = 1;
+		pathspec->magic |= item[i].magic;
+	}
+
+	qsort(pathspec->items, pathspec->nr,
+	      sizeof(struct pathspec_item), pathspec_item_cmp);
 }
 
 /*
  * N.B. get_pathspec() is deprecated in favor of the "struct pathspec"
- * based interface - see pathspec_magic above.
+ * based interface - see pathspec.c:parse_pathspec().
  *
  * Arguments:
  *  - prefix - a path relative to the root of the working tree

@@ -221,32 +334,11 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char
  */
 const char **get_pathspec(const char *prefix, const char **pathspec)
 {
-	const char *entry = *pathspec;
-	const char **src, **dst;
-	int prefixlen;
-
-	if (!prefix && !entry)
-		return NULL;
-
-	if (!entry) {
-		static const char *spec[2];
-		spec[0] = prefix;
-		spec[1] = NULL;
-		return spec;
-	}
-
-	/* Otherwise we have to re-write the entries.. */
-	src = pathspec;
-	dst = pathspec;
-	prefixlen = prefix ? strlen(prefix) : 0;
-	while (*src) {
-		*(dst++) = prefix_pathspec(prefix, prefixlen, *src);
-		src++;
-	}
-	*dst = NULL;
-	if (!*pathspec)
-		return NULL;
-	return pathspec;
+	struct pathspec ps;
+	parse_pathspec(&ps,
+		       PATHSPEC_ALL_MAGIC & ~PATHSPEC_FROMTOP,
+		       0, prefix, pathspec);
+	return ps.raw;
 }
 
 void copy_pathspec(struct pathspec *dst, const struct pathspec *src)

diff --git a/pathspec.h b/pathspec.h
index a621676..937ec91 100644
--- a/pathspec.h
+++ b/pathspec.h

@@ -1,6 +1,10 @@
 #ifndef PATHSPEC_H
 #define PATHSPEC_H
 
+/* Pathspec magic */
+#define PATHSPEC_FROMTOP	(1<<0)
+#define PATHSPEC_ALL_MAGIC PATHSPEC_FROMTOP
+
 #define PATHSPEC_ONESTAR 1	/* the pathspec pattern sastisfies GFNM_ONESTAR */
 
 struct pathspec {

@@ -8,9 +12,11 @@ struct pathspec {
 	int nr;
 	unsigned int has_wildcard:1;
 	unsigned int recursive:1;
+	unsigned magic;
 	int max_depth;
 	struct pathspec_item {
 		const char *match;
+		unsigned magic;
 		int len;
 		int nowildcard_len;
 		int flags;

@@ -18,6 +24,11 @@ struct pathspec {
 };
 
 extern int init_pathspec(struct pathspec *, const char **);
+extern void parse_pathspec(struct pathspec *pathspec,
+			   unsigned magic_mask,
+			   unsigned flags,
+			   const char *prefix,
+			   const char **args);
 extern void copy_pathspec(struct pathspec *dst, const struct pathspec *src);
 extern void free_pathspec(struct pathspec *);