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 *);