[PATCH 0/5] New get_pathspec()

[PATCH 0/5] New get_pathspec()

[Nguyễn Thái Ngọc Duy]

This is on top of jc/magic-pathspec. It implements new get_pathspec().
Remaining work includes

 - implement verify_pathspec() (see notes on 5/5)

 - make use of pathspec_item.plain_len introduced in 3/5

 - convert the rest to use new get_pathspec(). Can we make it a GSoC
   project?

 - get rid of old pathspec code.

 - implement a whole lot more of pathspec magic.

I think that's it. We also need to think if we can use similar syntax
for gitattr/gitignore, reuse as much code as possible.


[PATCH 1/5] Rename functions in preparation for get_pathspec() restructure

  I figure I cannot bring all commands to the new get_pathspec() at
  once and remove the old one. So instead I rename the old one so that
  we can have both implementations running in parallel.


[PATCH 2/5] Replace has_wildcard with PATHSPEC_NOGLOB

  This effectively kills Junio's use_wildcard rename patch.


[PATCH 3/5] Convert prefix_pathspec() to produce struct pathspec_item

  Also put plain_len field in place for literal matching on prefix.
  Nobody uses it yet though.


[PATCH 4/5] Implement new get_pathspec()

  I don't move prefix_pathspec() to dir.c yet. It may need some
  updates until things are stablized. It will be moved at latest when
  get_pathspec_old() is retired.


[PATCH 5/5] grep: convert to use the new get_pathspec()

  The first user of the new get_pathspec(), just to make sure I don't
  introduce too many bugs.
  The verify_filename() issue remains. I suggest we keep an
  incarnation of it to catch simple pathspecs, then implement
  verify_pathspec() for wildcard pathspecs.

 archive.c              |    2 +-
 builtin/add.c          |    6 ++--
 builtin/checkout.c     |    8 ++--
 builtin/clean.c        |    4 +-
 builtin/commit.c       |    6 ++--
 builtin/grep.c         |   14 ++------
 builtin/ls-files.c     |    8 ++--
 builtin/ls-tree.c      |    2 +-
 builtin/mv.c           |    2 +-
 builtin/rerere.c       |    2 +-
 builtin/reset.c        |    4 +-
 builtin/rm.c           |    4 +-
 builtin/update-index.c |    2 +-
 cache.h                |   26 +++++++++++++--
 dir.c                  |   81 +++++++++++++++++++++++++++++++++++------------
 dir.h                  |    4 +-
 read-cache.c           |    4 +-
 rerere.c               |    2 +-
 resolve-undo.c         |    2 +-
 revision.c             |    2 +-
 setup.c                |   45 +++++++++++---------------
 tree-walk.c            |   25 +++++++--------
 wt-status.c            |    4 +-
 23 files changed, 151 insertions(+), 108 deletions(-)

-- 
1.7.4.74.g639db

[PATCH 1/5] Rename functions in preparation for get_pathspec() restructure

[Nguyễn Thái Ngọc Duy]

This renames:

 - get_pathspec()         to get_pathspec_old()
 - match_pathspec()       to match_pathspec_old()
 - match_pathspec_depth() to match_pathspec()

The name get_pathspec() will be used for a new function which produces
struct pathspec directly.

Both get_pathspec implementations will co-exist for a while until
everything is converted to get_pathspec(). By that point, both
match_pathspec_old() and get_pathspec_old() would be removed.

Because two get_pathspec impl must provide the same functionality, the
new get_pathspec() can't (or shouldn't) implement any magic other than
'top' and 'icase' already supported by get_pathspec_old().
---
 archive.c              |    2 +-
 builtin/add.c          |    6 +++---
 builtin/checkout.c     |    8 ++++----
 builtin/clean.c        |    4 ++--
 builtin/commit.c       |    6 +++---
 builtin/grep.c         |    6 +++---
 builtin/ls-files.c     |    8 ++++----
 builtin/ls-tree.c      |    2 +-
 builtin/mv.c           |    2 +-
 builtin/rerere.c       |    2 +-
 builtin/reset.c        |    4 ++--
 builtin/rm.c           |    4 ++--
 builtin/update-index.c |    2 +-
 cache.h                |    2 +-
 dir.c                  |    4 ++--
 dir.h                  |    4 ++--
 read-cache.c           |    4 ++--
 rerere.c               |    2 +-
 resolve-undo.c         |    2 +-
 revision.c             |    2 +-
 setup.c                |    2 +-
 wt-status.c            |    4 ++--
 22 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/archive.c b/archive.c
index 1944ed4..884bb2f 100644
--- a/archive.c
+++ b/archive.c
@@ -231,7 +231,7 @@ static int path_exists(struct tree *tree, const char *path)
 static void parse_pathspec_arg(const char **pathspec,
 		struct archiver_args *ar_args)
 {
-	ar_args->pathspec = pathspec = get_pathspec("", pathspec);
+	ar_args->pathspec = pathspec = get_pathspec_old("", pathspec);
 	if (pathspec) {
 		while (*pathspec) {
 			if (!path_exists(ar_args->tree, *pathspec))
diff --git a/builtin/add.c b/builtin/add.c
index e127d5a..552801a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -112,7 +112,7 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
 		return;
 	for (i = 0; i < active_nr; i++) {
 		struct cache_entry *ce = active_cache[i];
-		match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen);
+		match_pathspec_old(pathspec, ce->name, ce_namelen(ce), 0, seen);
 	}
 }
 
@@ -142,7 +142,7 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int
 	i = dir->nr;
 	while (--i >= 0) {
 		struct dir_entry *entry = *src++;
-		if (match_pathspec(pathspec, entry->name, entry->len,
+		if (match_pathspec_old(pathspec, entry->name, entry->len,
 				   prefix, seen))
 			*dst++ = entry;
 	}
@@ -197,7 +197,7 @@ static void refresh(int verbose, const char **pathspec)
 
 static const char **validate_pathspec(int argc, const char **argv, const char *prefix)
 {
-	const char **pathspec = get_pathspec(prefix, argv);
+	const char **pathspec = get_pathspec_old(prefix, argv);
 
 	if (pathspec) {
 		const char **p;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2bf02f2..3bcff35 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -223,7 +223,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
-		match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
+		match_pathspec_old(pathspec, ce->name, ce_namelen(ce), 0, ps_matched);
 	}
 
 	if (report_path_error(ps_matched, pathspec, 0))
@@ -236,7 +236,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	/* Any unmerged paths? */
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
-		if (match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
+		if (match_pathspec_old(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
 			if (!ce_stage(ce))
 				continue;
 			if (opts->force) {
@@ -261,7 +261,7 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	state.refresh_cache = 1;
 	for (pos = 0; pos < active_nr; pos++) {
 		struct cache_entry *ce = active_cache[pos];
-		if (match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
+		if (match_pathspec_old(pathspec, ce->name, ce_namelen(ce), 0, NULL)) {
 			if (!ce_stage(ce)) {
 				errs |= checkout_entry(ce, &state, NULL);
 				continue;
@@ -1005,7 +1005,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
 		opts.track = git_branch_track;
 
 	if (argc) {
-		const char **pathspec = get_pathspec(prefix, argv);
+		const char **pathspec = get_pathspec_old(prefix, argv);
 
 		if (!pathspec)
 			die("invalid path specification");
diff --git a/builtin/clean.c b/builtin/clean.c
index 4a312ab..92889c6 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -95,7 +95,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	for (i = 0; i < exclude_list.nr; i++)
 		add_exclude(exclude_list.items[i].string, "", 0, dir.exclude_list);
 
-	pathspec = get_pathspec(prefix, argv);
+	pathspec = get_pathspec_old(prefix, argv);
 
 	fill_directory(&dir, pathspec);
 
@@ -137,7 +137,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 
 		if (pathspec) {
 			memset(seen, 0, argc > 0 ? argc : 1);
-			matches = match_pathspec(pathspec, ent->name, len,
+			matches = match_pathspec_old(pathspec, ent->name, len,
 						 0, seen);
 		}
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 3979b82..efdc7ae 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -264,7 +264,7 @@ static int list_paths(struct string_list *list, const char *with_tree,
 
 		if (ce->ce_flags & CE_UPDATE)
 			continue;
-		if (!match_pathspec(pattern, ce->name, ce_namelen(ce), 0, m))
+		if (!match_pathspec_old(pattern, ce->name, ce_namelen(ce), 0, m))
 			continue;
 		item = string_list_insert(list, ce->name);
 		if (ce_skip_worktree(ce))
@@ -350,7 +350,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int
 	}
 
 	if (*argv)
-		pathspec = get_pathspec(prefix, argv);
+		pathspec = get_pathspec_old(prefix, argv);
 
 	if (read_cache_preload(pathspec) < 0)
 		die("index file corrupt");
@@ -1197,7 +1197,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	if (show_ignored_in_status)
 		s.show_ignored_files = 1;
 	if (*argv)
-		s.pathspec = get_pathspec(prefix, argv);
+		s.pathspec = get_pathspec_old(prefix, argv);
 
 	read_cache_preload(s.pathspec);
 	refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, s.pathspec, NULL, NULL);
diff --git a/builtin/grep.c b/builtin/grep.c
index 0bf8c01..2826ca8 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -490,7 +490,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
 		struct cache_entry *ce = active_cache[nr];
 		if (!S_ISREG(ce->ce_mode))
 			continue;
-		if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
+		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL))
 			continue;
 		/*
 		 * If CE_VALID is on, we assume worktree file and its cache entry
@@ -627,7 +627,7 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec)
 	for (i = 0; i < dir.nr; i++) {
 		const char *name = dir.entries[i]->name;
 		int namelen = strlen(name);
-		if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL))
+		if (!match_pathspec(pathspec, name, namelen, 0, NULL))
 			continue;
 		hit |= grep_file(opt, dir.entries[i]->name);
 		if (hit && opt->status_only)
@@ -957,7 +957,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	}
 
 	if (i < argc)
-		paths = get_pathspec(prefix, argv + i);
+		paths = get_pathspec_old(prefix, argv + i);
 	else if (prefix) {
 		paths = xcalloc(2, sizeof(const char *));
 		paths[0] = prefix;
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index fb2d5f4..8e39503 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -58,7 +58,7 @@ static void show_dir_entry(const char *tag, struct dir_entry *ent)
 	if (len >= ent->len)
 		die("git ls-files: internal error - directory entry not superset of prefix");
 
-	if (!match_pathspec(pathspec, ent->name, ent->len, len, ps_matched))
+	if (!match_pathspec_old(pathspec, ent->name, ent->len, len, ps_matched))
 		return;
 
 	fputs(tag, stdout);
@@ -133,7 +133,7 @@ static void show_ce_entry(const char *tag, struct cache_entry *ce)
 	if (len >= ce_namelen(ce))
 		die("git ls-files: internal error - cache entry not superset of prefix");
 
-	if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), len, ps_matched))
+	if (!match_pathspec_old(pathspec, ce->name, ce_namelen(ce), len, ps_matched))
 		return;
 
 	if (tag && *tag && show_valid_bit &&
@@ -187,7 +187,7 @@ static void show_ru_info(void)
 		len = strlen(path);
 		if (len < max_prefix_len)
 			continue; /* outside of the prefix */
-		if (!match_pathspec(pathspec, path, len, max_prefix_len, ps_matched))
+		if (!match_pathspec_old(pathspec, path, len, max_prefix_len, ps_matched))
 			continue; /* uninterested */
 		for (i = 0; i < 3; i++) {
 			if (!ui->mode[i])
@@ -568,7 +568,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
 	if (require_work_tree && !is_inside_work_tree())
 		setup_work_tree();
 
-	pathspec = get_pathspec(prefix, argv);
+	pathspec = get_pathspec_old(prefix, argv);
 
 	/* be nice with submodule paths ending in a slash */
 	if (pathspec)
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index f73e6bd..f55dba9 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -166,7 +166,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	if (get_sha1(argv[0], sha1))
 		die("Not a valid object name %s", argv[0]);
 
-	pathspec = get_pathspec(prefix, argv + 1);
+	pathspec = get_pathspec_old(prefix, argv + 1);
 	tree = parse_tree_indirect(sha1);
 	if (!tree)
 		die("not a tree object");
diff --git a/builtin/mv.c b/builtin/mv.c
index 93e8995..37a285e 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -32,7 +32,7 @@ static const char **copy_pathspec(const char *prefix, const char **pathspec,
 			result[i] = base_name ? strdup(basename(it)) : it;
 		}
 	}
-	return get_pathspec(prefix, result);
+	return get_pathspec_old(prefix, result);
 }
 
 static const char *add_slash(const char *path)
diff --git a/builtin/rerere.c b/builtin/rerere.c
index 8235885..d31cc95 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -139,7 +139,7 @@ int cmd_rerere(int argc, const char **argv, const char *prefix)
 		const char **pathspec;
 		if (argc < 2)
 			warning("'git rerere forget' without paths is deprecated");
-		pathspec = get_pathspec(prefix, argv + 1);
+		pathspec = get_pathspec_old(prefix, argv + 1);
 		return rerere_forget(pathspec);
 	}
 
diff --git a/builtin/reset.c b/builtin/reset.c
index 5de2bce..7d8a29a 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -182,7 +182,7 @@ static int interactive_reset(const char *revision, const char **argv,
 	const char **pathspec = NULL;
 
 	if (*argv)
-		pathspec = get_pathspec(prefix, argv);
+		pathspec = get_pathspec_old(prefix, argv);
 
 	return run_add_interactive(revision, "--patch=reset", pathspec);
 }
@@ -195,7 +195,7 @@ static int read_from_tree(const char *prefix, const char **argv,
 	struct diff_options opt;
 
 	memset(&opt, 0, sizeof(opt));
-	diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
+	diff_tree_setup_paths(get_pathspec_old(prefix, (const char **)argv), &opt);
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
 	opt.format_callback_data = &index_was_discarded;
diff --git a/builtin/rm.c b/builtin/rm.c
index ff491d7..65a0e07 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -161,7 +161,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	if (read_cache() < 0)
 		die("index file corrupt");
 
-	pathspec = get_pathspec(prefix, argv);
+	pathspec = get_pathspec_old(prefix, argv);
 	refresh_index(&the_index, REFRESH_QUIET, pathspec, NULL, NULL);
 
 	seen = NULL;
@@ -171,7 +171,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 
 	for (i = 0; i < active_nr; i++) {
 		struct cache_entry *ce = active_cache[i];
-		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen))
+		if (!match_pathspec_old(pathspec, ce->name, ce_namelen(ce), 0, seen))
 			continue;
 		ALLOC_GROW(list.name, list.nr + 1, list.alloc);
 		list.name[list.nr++] = ce->name;
diff --git a/builtin/update-index.c b/builtin/update-index.c
index d7850c6..4ac72cf 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -546,7 +546,7 @@ static int do_reupdate(int ac, const char **av,
 	 */
 	int pos;
 	int has_head = 1;
-	const char **paths = get_pathspec(prefix, av + 1);
+	const char **paths = get_pathspec_old(prefix, av + 1);
 	struct pathspec pathspec;
 
 	init_pathspec(&pathspec, paths);
diff --git a/cache.h b/cache.h
index be6ce72..27ac8a7 100644
--- a/cache.h
+++ b/cache.h
@@ -425,7 +425,7 @@ extern void set_git_work_tree(const char *tree);
 
 #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES"
 
-extern const char **get_pathspec(const char *prefix, const char **pathspec);
+extern const char **get_pathspec_old(const char *prefix, const char **pathspec);
 extern void setup_work_tree(void);
 extern const char *setup_git_directory_gently(int *);
 extern const char *setup_git_directory(void);
diff --git a/dir.c b/dir.c
index 325fb56..6eb04ea 100644
--- a/dir.c
+++ b/dir.c
@@ -172,7 +172,7 @@ static int match_one(const char *match, const char *name, int namelen)
  * and a mark is left in seen[] array for pathspec element that
  * actually matched anything.
  */
-int match_pathspec(const char **pathspec, const char *name, int namelen,
+int match_pathspec_old(const char **pathspec, const char *name, int namelen,
 		int prefix, char *seen)
 {
 	int i, retval = 0;
@@ -244,7 +244,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
  * and a mark is left in seen[] array for pathspec element that
  * actually matched anything.
  */
-int match_pathspec_depth(const struct pathspec *ps,
+int match_pathspec(const struct pathspec *ps,
 			 const char *name, int namelen,
 			 int prefix, char *seen)
 {
diff --git a/dir.h b/dir.h
index aa511da..758ab6c 100644
--- a/dir.h
+++ b/dir.h
@@ -64,8 +64,8 @@ struct dir_struct {
 #define MATCHED_RECURSIVELY 1
 #define MATCHED_FNMATCH 2
 #define MATCHED_EXACTLY 3
-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,
+extern int match_pathspec_old(const char **pathspec, const char *name, int namelen, int prefix, char *seen);
+extern int match_pathspec(const struct pathspec *pathspec,
 				const char *name, int namelen,
 				int prefix, char *seen);
 extern int within_depth(const char *name, int namelen, int depth, int max_depth);
diff --git a/read-cache.c b/read-cache.c
index 98d526b..6644869 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -708,7 +708,7 @@ int ce_same_name(struct cache_entry *a, struct cache_entry *b)
 
 int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec)
 {
-	return match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL);
+	return match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL);
 }
 
 /*
@@ -1130,7 +1130,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 			continue;
 		}
 
-		if (pathspec && !match_pathspec(pathspec, ce->name, strlen(ce->name), 0, seen))
+		if (pathspec && !match_pathspec_old(pathspec, ce->name, strlen(ce->name), 0, seen))
 			continue;
 
 		new = refresh_cache_ent(istate, ce, options, &cache_errno);
diff --git a/rerere.c b/rerere.c
index 22996bd..af42948 100644
--- a/rerere.c
+++ b/rerere.c
@@ -665,7 +665,7 @@ int rerere_forget(const char **pathspec)
 	find_conflict(&conflict);
 	for (i = 0; i < conflict.nr; i++) {
 		struct string_list_item *it = &conflict.items[i];
-		if (!match_pathspec(pathspec, it->string, strlen(it->string),
+		if (!match_pathspec_old(pathspec, it->string, strlen(it->string),
 				    0, NULL))
 			continue;
 		rerere_forget_one_path(it->string, &merge_rr);
diff --git a/resolve-undo.c b/resolve-undo.c
index 72b4612..8400db6 100644
--- a/resolve-undo.c
+++ b/resolve-undo.c
@@ -165,7 +165,7 @@ void unmerge_index(struct index_state *istate, const char **pathspec)
 
 	for (i = 0; i < istate->cache_nr; i++) {
 		struct cache_entry *ce = istate->cache[i];
-		if (!match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL))
+		if (!match_pathspec_old(pathspec, ce->name, ce_namelen(ce), 0, NULL))
 			continue;
 		i = unmerge_index_entry_at(istate, i);
 	}
diff --git a/revision.c b/revision.c
index 86d2470..b1edc2f 100644
--- a/revision.c
+++ b/revision.c
@@ -1617,7 +1617,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	}
 
 	if (prune_data)
-		init_pathspec(&revs->prune_data, get_pathspec(revs->prefix, prune_data));
+		init_pathspec(&revs->prune_data, get_pathspec_old(revs->prefix, prune_data));
 
 	if (revs->def == NULL)
 		revs->def = opt ? opt->def : NULL;
diff --git a/setup.c b/setup.c
index 51e354c..4e5ac5e 100644
--- a/setup.c
+++ b/setup.c
@@ -251,7 +251,7 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
 	return retval;
 }
 
-const char **get_pathspec(const char *prefix, const char **pathspec)
+const char **get_pathspec_old(const char *prefix, const char **pathspec)
 {
 	const char *entry = *pathspec;
 	const char **src, **dst;
diff --git a/wt-status.c b/wt-status.c
index 53558d7..2785bc3 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -472,7 +472,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
 	for (i = 0; i < dir.nr; i++) {
 		struct dir_entry *ent = dir.entries[i];
 		if (cache_name_is_other(ent->name, ent->len) &&
-		    match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+		    match_pathspec_old(s->pathspec, ent->name, ent->len, 0, NULL))
 			string_list_insert(&s->untracked, ent->name);
 		free(ent);
 	}
@@ -484,7 +484,7 @@ static void wt_status_collect_untracked(struct wt_status *s)
 		for (i = 0; i < dir.nr; i++) {
 			struct dir_entry *ent = dir.entries[i];
 			if (cache_name_is_other(ent->name, ent->len) &&
-			    match_pathspec(s->pathspec, ent->name, ent->len, 0, NULL))
+			    match_pathspec_old(s->pathspec, ent->name, ent->len, 0, NULL))
 				string_list_insert(&s->ignored, ent->name);
 			free(ent);
 		}
-- 
1.7.4.74.g639db

[PATCH 2/5] Replace has_wildcard with PATHSPEC_NOGLOB

[Nguyễn Thái Ngọc Duy]

---
 cache.h     |   22 ++++++++++++++++++++--
 dir.c       |   11 +++++++----
 setup.c     |   17 -----------------
 tree-walk.c |   25 ++++++++++++-------------
 4 files changed, 39 insertions(+), 36 deletions(-)

diff --git a/cache.h b/cache.h
index 27ac8a7..da1a46c 100644
--- a/cache.h
+++ b/cache.h
@@ -501,16 +501,34 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
 extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
 
+/*
+ * 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" },
+ *	{ PATHSPEC_RECURSIVE, '*', "recursive" },
+ *	{ PATHSPEC_REGEXP, '\0', "regexp" },
+ *
+ */
+#define PATHSPEC_FROMTOP    (1<<0)
+#define PATHSPEC_ICASE      (1<<1)
+#define PATHSPEC_NOGLOB     (1<<2)
+
 struct pathspec {
 	const char **raw; /* get_pathspec() result, not freed by free_pathspec() */
 	int nr;
-	unsigned int has_wildcard:1;
+	unsigned int use_wildcard:1;
 	unsigned int recursive:1;
 	int max_depth;
 	struct pathspec_item {
 		const char *match;
 		int len;
-		unsigned int has_wildcard:1;
+		unsigned magic;
 	} *items;
 };
 
diff --git a/dir.c b/dir.c
index 6eb04ea..58cedd3 100644
--- a/dir.c
+++ b/dir.c
@@ -230,7 +230,8 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
 			return MATCHED_RECURSIVELY;
 	}
 
-	if (item->has_wildcard && !fnmatch(match, name, 0))
+	if (!(item->magic & PATHSPEC_NOGLOB) &&
+	    !fnmatch(match, name, 0))
 		return MATCHED_FNMATCH;
 
 	return 0;
@@ -1280,15 +1281,17 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 		return 0;
 
 	pathspec->items = xmalloc(sizeof(struct pathspec_item)*pathspec->nr);
+	memset(pathspec->items, 0, sizeof(struct pathspec_item)*pathspec->nr);
 	for (i = 0; i < pathspec->nr; i++) {
 		struct pathspec_item *item = pathspec->items+i;
 		const char *path = paths[i];
 
 		item->match = path;
 		item->len = strlen(path);
-		item->has_wildcard = !no_wildcard(path);
-		if (item->has_wildcard)
-			pathspec->has_wildcard = 1;
+		if (no_wildcard(path))
+			item->magic |= PATHSPEC_NOGLOB;
+		else
+			pathspec->use_wildcard = 1;
 	}
 
 	qsort(pathspec->items, pathspec->nr,
diff --git a/setup.c b/setup.c
index 4e5ac5e..1e7dfb1 100644
--- a/setup.c
+++ b/setup.c
@@ -126,23 +126,6 @@ void verify_non_filename(const char *prefix, const char *arg)
 	    "Use '--' to separate filenames from revisions", arg);
 }
 
-/*
- * 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" },
- *	{ PATHSPEC_RECURSIVE, '*', "recursive" },
- *	{ PATHSPEC_REGEXP, '\0', "regexp" },
- *
- */
-#define PATHSPEC_FROMTOP    (1<<0)
-#define PATHSPEC_ICASE      (1<<1)
-
 struct pathspec_magic {
 	unsigned bit;
 	char mnemonic; /* this cannot be ':'! */
diff --git a/tree-walk.c b/tree-walk.c
index 322becc..026d88e 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -559,7 +559,7 @@ int tree_entry_interesting(const struct name_entry *entry,
 {
 	int i;
 	int pathlen, baselen = base->len - base_offset;
-	int never_interesting = ps->has_wildcard ? 0 : -1;
+	int never_interesting = ps->use_wildcard ? 0 : -1;
 
 	if (!ps->nr) {
 		if (!ps->recursive || ps->max_depth == -1)
@@ -598,23 +598,22 @@ int tree_entry_interesting(const struct name_entry *entry,
 					&never_interesting))
 				return 1;
 
-			if (ps->items[i].has_wildcard) {
-				if (!fnmatch(match + baselen, entry->path, 0))
-					return 1;
+			if (ps->items[i].magic & PATHSPEC_NOGLOB)
+				continue;
 
-				/*
-				 * Match all directories. We'll try to
-				 * match files later on.
-				 */
-				if (ps->recursive && S_ISDIR(entry->mode))
-					return 1;
-			}
+			if (!fnmatch(match + baselen, entry->path, 0))
+				return 1;
 
-			continue;
+			/*
+			 * Match all directories. We'll try to match
+			 * files later on.
+			 */
+			if (ps->recursive && S_ISDIR(entry->mode))
+				return 1;
 		}
 
 match_wildcards:
-		if (!ps->items[i].has_wildcard)
+		if (ps->items[i].magic & PATHSPEC_NOGLOB)
 			continue;
 
 		/*
-- 
1.7.4.74.g639db

[PATCH 3/5] Convert prefix_pathspec() to produce struct pathspec_item

[Nguyễn Thái Ngọc Duy]

New field plain_len is used to mark the first part of 'match' where no
magic can be applied.
---
 cache.h |    1 +
 setup.c |   26 ++++++++++++++++++--------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/cache.h b/cache.h
index da1a46c..08b4633 100644
--- a/cache.h
+++ b/cache.h
@@ -528,6 +528,7 @@ struct pathspec {
 	struct pathspec_item {
 		const char *match;
 		int len;
+		int plain_len;	/* match[0..plain_len-1] does not have any kind of magic*/
 		unsigned magic;
 	} *items;
 };
diff --git a/setup.c b/setup.c
index 1e7dfb1..8a4a9d0 100644
--- a/setup.c
+++ b/setup.c
@@ -148,11 +148,12 @@ struct pathspec_magic {
  * the prefix part must always match literally, and a single stupid
  * string cannot express such a case.
  */
-const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
+int prefix_pathspec(struct pathspec_item *item,
+		    const char *prefix, int prefixlen,
+		    const char *elt)
 {
 	unsigned magic = 0;
 	const char *copyfrom = elt;
-	const char *retval;
 	int i, free_source = 0;
 
 	if (elt[0] != ':') {
@@ -184,7 +185,8 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
 			copyfrom++;
 	} else if (!elt[1]) {
 		/* Just ':' -- no element! */
-		return NULL;
+		memset(item, 0, sizeof(*item));
+		return -1;
 	} else {
 		/* shorthand */
 		for (copyfrom = elt + 1;
@@ -225,13 +227,19 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)
 		}
 	}
 
+	memset(item, 0, sizeof(*item));
+	item->magic = magic;
+
 	if (magic & PATHSPEC_FROMTOP)
-		retval = xstrdup(copyfrom);
-	else
-		retval = prefix_path(prefix, prefixlen, copyfrom);
+		item->match = xstrdup(copyfrom);
+	else {
+		item->match = prefix_path(prefix, prefixlen, copyfrom);
+		item->plain_len = prefixlen;
+	}
+	item->len = strlen(item->match);
 	if (free_source)
 		free((char *)copyfrom);
-	return retval;
+	return 0;
 }
 
 const char **get_pathspec_old(const char *prefix, const char **pathspec)
@@ -255,7 +263,9 @@ const char **get_pathspec_old(const char *prefix, const char **pathspec)
 	dst = pathspec;
 	prefixlen = prefix ? strlen(prefix) : 0;
 	while (*src) {
-		*(dst++) = prefix_pathspec(prefix, prefixlen, *src);
+		struct pathspec_item item;
+		prefix_pathspec(&item, prefix, prefixlen, *src);
+		*(dst++) = item.match;
 		src++;
 	}
 	*dst = NULL;
-- 
1.7.4.74.g639db

[PATCH 4/5] Implement new get_pathspec()

[Nguyễn Thái Ngọc Duy]

This function is as same as get_pathspec_old() except that it produces
struct pathspec directly.

no_prefix code path is necessary because init_pathspec() can be called
without get_pathspec_old() in "diff --no-index" case. Without this
exception, prefix_path() will be eventually called and die because
there is not worktree.
---
 cache.h |    1 +
 dir.c   |   74 ++++++++++++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 56 insertions(+), 19 deletions(-)

diff --git a/cache.h b/cache.h
index 08b4633..de57dff 100644
--- a/cache.h
+++ b/cache.h
@@ -534,6 +534,7 @@ struct pathspec {
 };
 
 extern int init_pathspec(struct pathspec *, const char **);
+extern int get_pathspec(struct pathspec *, const char *, int, const char **);
 extern void free_pathspec(struct pathspec *);
 extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
 extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, enum object_type type, const char *path, int format_check);
diff --git a/dir.c b/dir.c
index 58cedd3..64348a1 100644
--- a/dir.c
+++ b/dir.c
@@ -1265,33 +1265,59 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
 	return strcmp(a->match, b->match);
 }
 
-int init_pathspec(struct pathspec *pathspec, const char **paths)
+extern int prefix_pathspec(struct pathspec_item *item, const char *prefix,
+			   int prefixlen, const char *elt);
+const char *no_prefix = "We do not want outside repository check.";
+int get_pathspec(struct pathspec *pathspec, const char *prefix,
+		 int argc, const char **argv)
 {
-	const char **p = paths;
-	int i;
+	struct pathspec_item *pitem;
+	const char **dst;
+	int prefixlen;
 
 	memset(pathspec, 0, sizeof(*pathspec));
-	if (!p)
+
+	if ((!prefix || prefix == no_prefix) && !argc)
 		return 0;
-	while (*p)
-		p++;
-	pathspec->raw = paths;
-	pathspec->nr = p - paths;
-	if (!pathspec->nr)
+
+	if (!argc) {
+		static const char *spec[2];
+		spec[0] = prefix;
+		spec[1] = NULL;
+		init_pathspec(pathspec, spec);
+		pathspec->items[0].plain_len = pathspec->items[0].len;
 		return 0;
+	}
 
-	pathspec->items = xmalloc(sizeof(struct pathspec_item)*pathspec->nr);
-	memset(pathspec->items, 0, sizeof(struct pathspec_item)*pathspec->nr);
-	for (i = 0; i < pathspec->nr; i++) {
-		struct pathspec_item *item = pathspec->items+i;
-		const char *path = paths[i];
+	if (!strcmp(*argv, ":"))
+		return 0;
 
-		item->match = path;
-		item->len = strlen(path);
-		if (no_wildcard(path))
-			item->magic |= PATHSPEC_NOGLOB;
+	prefixlen = prefix && prefix != no_prefix ? strlen(prefix) : 0;
+	pathspec->nr = argc; /* be optimistic, lower it later if necessary */
+	pathspec->items = xmalloc(sizeof(struct pathspec_item) * argc);
+	pathspec->raw = argv;
+	pitem = pathspec->items;
+	dst = argv;
+
+	while (*argv) {
+		if (prefix == no_prefix) {
+			memset(pitem, 0, sizeof(*pitem));
+			pitem->match = *argv;
+			pitem->len = strlen(pitem->match);
+		}
+		else
+			prefix_pathspec(pitem, prefix, prefixlen, *argv);
+		*dst = *argv++;
+		if (pitem->match) {
+			if (no_wildcard(pitem->match + pitem->plain_len))
+				pitem->magic |= PATHSPEC_NOGLOB;
+			else
+				pathspec->use_wildcard = 1;
+			pitem++;
+			dst++;
+		}
 		else
-			pathspec->use_wildcard = 1;
+			pathspec->nr--;
 	}
 
 	qsort(pathspec->items, pathspec->nr,
@@ -1300,6 +1326,16 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
 	return 0;
 }
 
+int init_pathspec(struct pathspec *pathspec, const char **paths)
+{
+	const char **p = paths;
+
+	while (p && *p)
+		p++;
+
+	return get_pathspec(pathspec, no_prefix, p - paths, paths);
+}
+
 void free_pathspec(struct pathspec *pathspec)
 {
 	free(pathspec->items);
-- 
1.7.4.74.g639db

[PATCH 5/5] grep: convert to use the new get_pathspec()

[Nguyễn Thái Ngọc Duy]

---
 builtin/grep.c |   10 +---------
 1 files changed, 1 insertions(+), 9 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index 2826ca8..af16deb 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -734,7 +734,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	const char *show_in_pager = NULL, *default_pager = "dummy";
 	struct grep_opt opt;
 	struct object_array list = OBJECT_ARRAY_INIT;
-	const char **paths = NULL;
 	struct pathspec pathspec;
 	struct string_list path_list = STRING_LIST_INIT_NODUP;
 	int i;
@@ -956,14 +955,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			verify_filename(prefix, argv[j]);
 	}
 
-	if (i < argc)
-		paths = get_pathspec_old(prefix, argv + i);
-	else if (prefix) {
-		paths = xcalloc(2, sizeof(const char *));
-		paths[0] = prefix;
-		paths[1] = NULL;
-	}
-	init_pathspec(&pathspec, paths);
+	get_pathspec(&pathspec, prefix, argc - i, argv + i);
 	pathspec.max_depth = opt.max_depth;
 	pathspec.recursive = 1;
 
-- 
1.7.4.74.g639db

Re: [PATCH 0/5] New get_pathspec()

[Junio C Hamano]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> [PATCH 1/5] Rename functions in preparation for get_pathspec() restructure
>
>   I figure I cannot bring all commands to the new get_pathspec() at
>   once and remove the old one. So instead I rename the old one so that
>   we can have both implementations running in parallel.

Horribly bad move.  Keep the old one running under the same name, and keep
the unconverted ones working the same way without new features.  Introduce
a new infrastructure under a new API function name.

Otherwise you cannot merge in new calles that expect the old
get_pathspec() behaviour.

Re: [PATCH 2/5] Replace has_wildcard with PATHSPEC_NOGLOB

[Junio C Hamano]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> ---
>  cache.h     |   22 ++++++++++++++++++++--
>  dir.c       |   11 +++++++----
>  setup.c     |   17 -----------------
>  tree-walk.c |   25 ++++++++++++-------------
>  4 files changed, 39 insertions(+), 36 deletions(-)
>
> diff --git a/cache.h b/cache.h
> index 27ac8a7..da1a46c 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -501,16 +501,34 @@ extern int index_name_is_other(const struct index_state *, const char *, int);
>  extern int ie_match_stat(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
>  extern int ie_modified(const struct index_state *, struct cache_entry *, struct stat *, unsigned int);
>  
> +/*
> + * 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" },
> + *	{ PATHSPEC_RECURSIVE, '*', "recursive" },
> + *	{ PATHSPEC_REGEXP, '\0', "regexp" },
> + *
> + */

Gaah, why did you butcher this to heve only the whole comment and struct
type declaration here, without moving the "Possible future magic" comment
that clearly belongs to the structure definition you still have in dir.c
to where it belongs?  Also if NOGLOB is no longer "Possible future", why
is it still here?

Re: [PATCH 5/5] grep: convert to use the new get_pathspec()

[Junio C Hamano]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> ---
>  builtin/grep.c |   10 +---------
>  1 files changed, 1 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 2826ca8..af16deb 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -734,7 +734,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  	const char *show_in_pager = NULL, *default_pager = "dummy";
>  	struct grep_opt opt;
>  	struct object_array list = OBJECT_ARRAY_INIT;
> -	const char **paths = NULL;
>  	struct pathspec pathspec;
>  	struct string_list path_list = STRING_LIST_INIT_NODUP;
>  	int i;
> @@ -956,14 +955,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  			verify_filename(prefix, argv[j]);
>  	}
>  
> -	if (i < argc)
> -		paths = get_pathspec_old(prefix, argv + i);
> -	else if (prefix) {
> -		paths = xcalloc(2, sizeof(const char *));
> -		paths[0] = prefix;
> -		paths[1] = NULL;
> -	}
> -	init_pathspec(&pathspec, paths);
> +	get_pathspec(&pathspec, prefix, argc - i, argv + i);

This assumes that the new API function will default to "if run without
pathspec, the calling command wants to limit to cwd", doesn't it?

That is why I mentioned that the caller would need to pass a hint as to
what should happen in that case in my earlier message.  Probably the new
API function should be something like:

    setup_pathspec(&pathspec, prefix, argc, argv, opts)

where opts is a bitmask to carry that hint (or a pointer to a structure
that caller to carry a set of hints richer than a bitmask can express),
and "add -u" and "grep" should set PATHSPEC_DEFAULT_LOCAL in the bitmask.
The call to setup_pathspec() from the log family would not want "no user
specified pathspec means limited to local" semantics.

Then when somebody wants to flip the "add -u" default in future versions,
the call from "add -u" codepath can instead use PATHSPEC_DEFAULT_TREEWIDE
(or perhaps the lack of PATHSPEC_DEFAULT_LOCAL bit may mean tree-wide)
there.

Re: [PATCH 2/5] Replace has_wildcard with PATHSPEC_NOGLOB

[Nguyen Thai Ngoc Duy]

2011/4/10 Junio C Hamano <gitster@pobox.com>:
>> +/*
>> + * 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" },
>> + *   { PATHSPEC_RECURSIVE, '*', "recursive" },
>> + *   { PATHSPEC_REGEXP, '\0', "regexp" },
>> + *
>> + */
>
> Gaah, why did you butcher this to heve only the whole comment and struct
> type declaration here, without moving the "Possible future magic" comment
> that clearly belongs to the structure definition you still have in dir.c
> to where it belongs?  Also if NOGLOB is no longer "Possible future", why
> is it still here?

Carelessness, I guess. Will move it (with the NOGLOB future line
removed) to dir.c, close to get_pathspec().
-- 
Duy

Re: [PATCH 5/5] grep: convert to use the new get_pathspec()

[Nguyen Thai Ngoc Duy]

2011/4/10 Junio C Hamano <gitster@pobox.com>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> This assumes that the new API function will default to "if run without
> pathspec, the calling command wants to limit to cwd", doesn't it?

Yes, I was blindly reimplementing get_pathspec() behavior...

> That is why I mentioned that the caller would need to pass a hint as to
> what should happen in that case in my earlier message.  Probably the new
> API function should be something like:
>
>    setup_pathspec(&pathspec, prefix, argc, argv, opts)
>
> where opts is a bitmask to carry that hint (or a pointer to a structure
> that caller to carry a set of hints richer than a bitmask can express),
> and "add -u" and "grep" should set PATHSPEC_DEFAULT_LOCAL in the bitmask.
> The call to setup_pathspec() from the log family would not want "no user
> specified pathspec means limited to local" semantics.
>
> Then when somebody wants to flip the "add -u" default in future versions,
> the call from "add -u" codepath can instead use PATHSPEC_DEFAULT_TREEWIDE
> (or perhaps the lack of PATHSPEC_DEFAULT_LOCAL bit may mean tree-wide)
> there.

... and forgot about this.
-- 
Duy