{"thread":{"id":"32464","subject":"[PATCH v3 05/19] dir.c: rename excluded_from_list() to is_excluded_from_list()","startedAt":"2012-12-27T02:32:19Z","lastAt":"2013-01-06T23:19:15Z","messageCount":55,"participants":["Adam Spiers","Michael Leal","Junio C Hamano","Antoine Pelisse"],"isPatch":true,"patchVersion":3,"patchTotal":19},"messages":[{"id":"205541","messageId":"1356575558-2674-1-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":null,"subject":"[PATCH v3 00/19] new git check-ignore sub-command","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:19Z","receivedAt":"2012-12-27T02:32:19Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"This v3 re-roll of my check-ignore series is a reasonably substantial\nrevamp over v2, and applies on top of Junio's current\nnd/attr-match-optim-more branch (82dce998c202).\n\nAll feedback and patches from the v2 series has been incorporated, and\nseveral other improvements too, including:\n\n - composite exclude_lists have been split up into\n exclude_list_groups each containing one exclude_list per source\n\n - smaller commits for easier review\n\n - minor memory leaks have been fixed and verified via valgrind\n\n - t0007-ignores.sh has been renumbered to t0008-ignores.sh to avoid\n a conflict with t0007-git-var.sh\n\n - improvements to documentation and comments\n\nFor reference, the v2 series was announced here:\n\n http://thread.gmane.org/gmane.comp.version-control.git/204661/focus=206074\n\nAll tests pass except for t91*, since there seems to be some\npre-existing breakage in 82dce998c202 relating to git-svn.\n\nAdam Spiers (19):\n api-directory-listing.txt: update to match code\n Improve documentation and comments regarding directory traversal API\n dir.c: rename cryptic 'which' variable to more consistent name\n dir.c: rename path_excluded() to is_path_excluded()\n dir.c: rename excluded_from_list() to is_excluded_from_list()\n dir.c: rename excluded() to is_excluded()\n dir.c: refactor is_excluded_from_list()\n dir.c: refactor is_excluded()\n dir.c: refactor is_path_excluded()\n dir.c: rename free_excludes() to clear_exclude_list()\n dir.c: use a single struct exclude_list per source of excludes\n dir.c: keep track of where patterns came from\n dir.c: provide clear_directory() for reclaiming dir_struct memory\n add.c: refactor treat_gitlinks()\n add.c: remove unused argument from validate_pathspec()\n pathspec.c: move reusable code from builtin/add.c\n pathspec.c: extract new validate_path() for reuse\n setup.c: document get_pathspec()\n Add git-check-ignore sub-command\n\n .gitignore | 1 +\n Documentation/git-check-ignore.txt | 89 ++++\n Documentation/gitignore.txt | 6 +-\n Documentation/technical/api-directory-listing.txt | 35 +-\n Makefile | 3 +\n attr.c | 2 +-\n builtin.h | 1 +\n builtin/add.c | 84 +--\n builtin/check-ignore.c | 170 +++++++\n builtin/clean.c | 3 +-\n builtin/ls-files.c | 11 +-\n command-list.txt | 1 +\n contrib/completion/git-completion.bash | 1 +\n dir.c | 243 +++++++--\n dir.h | 87 +++-\n git.c | 1 +\n pathspec.c | 107 ++++\n pathspec.h | 6 +\n setup.c | 15 +\n t/t0008-ignores.sh | 595 ++++++++++++++++++++++\n unpack-trees.c | 14 +-\n 21 files changed, 1305 insertions(+), 170 deletions(-)\n create mode 100644 Documentation/git-check-ignore.txt\n create mode 100644 builtin/check-ignore.c\n create mode 100644 pathspec.c\n create mode 100644 pathspec.h\n create mode 100755 t/t0008-ignores.sh\n\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205542","messageId":"1356575558-2674-2-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 01/19] api-directory-listing.txt: update to match code","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:20Z","receivedAt":"2012-12-27T02:32:20Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"7c4c97c0ac turned the flags in struct dir_struct into a single bitfield\nvariable, but forgot to update this document.\n\nSigned-off-by: Adam Spiers \n---\nFor some reason this patch was dropped from the v2 series when it was\napplied to the 'pu' branch.\n\n Documentation/technical/api-directory-listing.txt | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex add6f43..0356d25 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -17,24 +17,24 @@ options are:\n \tThe name of the file to be read in each directory for excluded\n \tfiles (typically `.gitignore`).\n \n-`collect_ignored`::\n+`flags`::\n \n-\tInclude paths that are to be excluded in the result.\n+\tA bit-field of options:\n \n-`show_ignored`::\n+`DIR_SHOW_IGNORED`:::\n \n \tThe traversal is for finding just ignored files, not unignored\n \tfiles.\n \n-`show_other_directories`::\n+`DIR_SHOW_OTHER_DIRECTORIES`:::\n \n \tInclude a directory that is not tracked.\n \n-`hide_empty_directories`::\n+`DIR_HIDE_EMPTY_DIRECTORIES`:::\n \n \tDo not include a directory that is not tracked and is empty.\n \n-`no_gitlinks`::\n+`DIR_NO_GITLINKS`:::\n \n \tIf set, recurse into a directory that looks like a git\n \tdirectory. Otherwise it is shown as a directory.\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205527","messageId":"1356575558-2674-3-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 02/19] Improve documentation and comments regarding directory traversal API","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:21Z","receivedAt":"2012-12-27T02:32:21Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":">From the perspective of a newcomer to the codebase, the directory\ntraversal API has a few potentially confusing properties. These\ncomments clarify a few key aspects and will hopefully make it easier\nto understand for other newcomers in the future.\n\nSigned-off-by: Adam Spiers \n---\nFor some reason this patch was dropped from the v2 series when it was\napplied to the 'pu' branch.\n\n Documentation/technical/api-directory-listing.txt | 9 +++++---\n dir.c | 8 ++++++-\n dir.h | 26 +++++++++++++++++++++--\n 3 files changed, 37 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex 0356d25..944fc39 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -9,8 +9,11 @@ Data structure\n --------------\n \n `struct dir_struct` structure is used to pass directory traversal\n-options to the library and to record the paths discovered. The notable\n-options are:\n+options to the library and to record the paths discovered. A single\n+`struct dir_struct` is used regardless of whether or not the traversal\n+recursively descends into subdirectories.\n+\n+The notable options are:\n \n `exclude_per_dir`::\n \n@@ -39,7 +42,7 @@ options are:\n \tIf set, recurse into a directory that looks like a git\n \tdirectory. Otherwise it is shown as a directory.\n \n-The result of the enumeration is left in these fields::\n+The result of the enumeration is left in these fields:\n \n `entries[]`::\n \ndiff --git a/dir.c b/dir.c\nindex ee8e711..89e27a6 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2,6 +2,8 @@\n * This handles recursive filename detection with exclude\n * files, index knowledge etc..\n *\n+ * See Documentation/technical/api-directory-listing.txt\n+ *\n * Copyright (C) Linus Torvalds, 2005-2006\n *\t\t Junio Hamano, 2005-2006\n */\n@@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)\n \t\tdie(\"cannot use %s as an exclude file\", fname);\n }\n \n+/*\n+ * Loads the per-directory exclude list for the substring of base\n+ * which has a char length of baselen.\n+ */\n static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n {\n \tstruct exclude_list *el;\n@@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \t (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))\n \t\treturn; /* too long a path -- ignore */\n \n-\t/* Pop the ones that are not the prefix of the path being checked. */\n+\t/* Pop the directories that are not the prefix of the path being checked. */\n \tel = &dir->exclude_list[EXC_DIRS];\n \twhile ((stk = dir->exclude_stack) != NULL) {\n \t\tif (stk->baselen <= baselen &&\ndiff --git a/dir.h b/dir.h\nindex f5c89e3..e0869bc 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -1,6 +1,8 @@\n #ifndef DIR_H\n #define DIR_H\n \n+/* See Documentation/technical/api-directory-listing.txt */\n+\n #include \"strbuf.h\"\n \n struct dir_entry {\n@@ -13,6 +15,12 @@ struct dir_entry {\n #define EXC_FLAG_MUSTBEDIR 8\n #define EXC_FLAG_NEGATIVE 16\n \n+/*\n+ * Each .gitignore file will be parsed into patterns which are then\n+ * appended to the relevant exclude_list (either EXC_DIRS or\n+ * EXC_FILE). exclude_lists are also used to represent the list of\n+ * --exclude values passed via CLI args (EXC_CMDL).\n+ */\n struct exclude_list {\n \tint nr;\n \tint alloc;\n@@ -26,9 +34,15 @@ struct exclude_list {\n \t} **excludes;\n };\n \n+/*\n+ * The contents of the per-directory exclude files are lazily read on\n+ * demand and then cached in memory, one per exclude_stack struct, in\n+ * order to avoid opening and parsing each one every time that\n+ * directory is traversed.\n+ */\n struct exclude_stack {\n-\tstruct exclude_stack *prev;\n-\tchar *filebuf;\n+\tstruct exclude_stack *prev; /* the struct exclude_stack for the parent directory */\n+\tchar *filebuf; /* remember pointer to per-directory exclude file contents so we can free() */\n \tint baselen;\n \tint exclude_ix;\n };\n@@ -59,6 +73,14 @@ struct dir_struct {\n #define EXC_DIRS 1\n #define EXC_FILE 2\n \n+\t/*\n+\t * Temporary variables which are used during loading of the\n+\t * per-directory exclude lists.\n+\t *\n+\t * exclude_stack points to the top of the exclude_stack, and\n+\t * basebuf contains the full path to the current\n+\t * (sub)directory in the traversal.\n+\t */\n \tstruct exclude_stack *exclude_stack;\n \tchar basebuf[PATH_MAX];\n };\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205545","messageId":"1356575558-2674-4-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 03/19] dir.c: rename cryptic 'which' variable to more consistent name","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:22Z","receivedAt":"2012-12-27T02:32:22Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"'el' is only *slightly* less cryptic, but is already used as the\nvariable name for a struct exclude_list pointer in numerous other\nplaces, so this reduces the number of cryptic variable names in use by\none :-)\n\nSigned-off-by: Adam Spiers \n---\n dir.c | 10 +++++-----\n dir.h | 4 ++--\n 2 files changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 89e27a6..f31aa59 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -349,7 +349,7 @@ void parse_exclude_pattern(const char **pattern,\n }\n \n void add_exclude(const char *string, const char *base,\n-\t\t int baselen, struct exclude_list *which)\n+\t\t int baselen, struct exclude_list *el)\n {\n \tstruct exclude *x;\n \tint patternlen;\n@@ -373,8 +373,8 @@ void add_exclude(const char *string, const char *base,\n \tx->base = base;\n \tx->baselen = baselen;\n \tx->flags = flags;\n-\tALLOC_GROW(which->excludes, which->nr + 1, which->alloc);\n-\twhich->excludes[which->nr++] = x;\n+\tALLOC_GROW(el->excludes, el->nr + 1, el->alloc);\n+\tel->excludes[el->nr++] = x;\n }\n \n static void *read_skip_worktree_file_from_index(const char *path, size_t *size)\n@@ -416,7 +416,7 @@ int add_excludes_from_file_to_list(const char *fname,\n \t\t\t\t const char *base,\n \t\t\t\t int baselen,\n \t\t\t\t char **buf_p,\n-\t\t\t\t struct exclude_list *which,\n+\t\t\t\t struct exclude_list *el,\n \t\t\t\t int check_index)\n {\n \tstruct stat st;\n@@ -463,7 +463,7 @@ int add_excludes_from_file_to_list(const char *fname,\n \t\tif (buf[i] == '\\n') {\n \t\t\tif (entry != buf + i && entry[0] != '#') {\n \t\t\t\tbuf[i - (i && buf[i-1] == '\\r')] = 0;\n-\t\t\t\tadd_exclude(entry, base, baselen, which);\n+\t\t\t\tadd_exclude(entry, base, baselen, el);\n \t\t\t}\n \t\t\tentry = buf + i + 1;\n \t\t}\ndiff --git a/dir.h b/dir.h\nindex e0869bc..680c1eb 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -127,11 +127,11 @@ extern int path_excluded(struct path_exclude_check *, const char *, int namelen,\n \n \n extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,\n-\t\t\t\t\t char **buf_p, struct exclude_list *which, int check_index);\n+\t\t\t\t\t char **buf_p, struct exclude_list *el, int check_index);\n extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n extern void parse_exclude_pattern(const char **string, int *patternlen, int *flags, int *nowildcardlen);\n extern void add_exclude(const char *string, const char *base,\n-\t\t\tint baselen, struct exclude_list *which);\n+\t\t\tint baselen, struct exclude_list *el);\n extern void free_excludes(struct exclude_list *el);\n extern int file_exists(const char *);\n \n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205544","messageId":"1356575558-2674-5-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 04/19] dir.c: rename path_excluded() to is_path_excluded()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:23Z","receivedAt":"2012-12-27T02:32:23Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Start adopting clearer names for exclude functions. This 'is_*'\nnaming pattern for functions returning booleans was agreed here:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924\n\nSigned-off-by: Adam Spiers \n---\n builtin/add.c | 2 +-\n builtin/ls-files.c | 2 +-\n dir.c | 4 ++--\n dir.h | 2 +-\n unpack-trees.c | 2 +-\n 5 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 89dce56..c689f37 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -453,7 +453,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t\t && !file_exists(pathspec[i])) {\n \t\t\t\tif (ignore_missing) {\n \t\t\t\t\tint dtype = DT_UNKNOWN;\n-\t\t\t\t\tif (path_excluded(&check, pathspec[i], -1, &dtype))\n+\t\t\t\t\tif (is_path_excluded(&check, pathspec[i], -1, &dtype))\n \t\t\t\t\t\tdir_add_ignored(&dir, pathspec[i], strlen(pathspec[i]));\n \t\t\t\t} else\n \t\t\t\t\tdie(_(\"pathspec '%s' did not match any files\"),\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 31b3f2d..ef7f99a 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -203,7 +203,7 @@ static void show_ru_info(void)\n static int ce_excluded(struct path_exclude_check *check, struct cache_entry *ce)\n {\n \tint dtype = ce_to_dtype(ce);\n-\treturn path_excluded(check, ce->name, ce_namelen(ce), &dtype);\n+\treturn is_path_excluded(check, ce->name, ce_namelen(ce), &dtype);\n }\n \n static void show_files(struct dir_struct *dir)\ndiff --git a/dir.c b/dir.c\nindex f31aa59..f1c0abd 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -685,8 +685,8 @@ void path_exclude_check_clear(struct path_exclude_check *check)\n * A path to a directory known to be excluded is left in check->path to\n * optimize for repeated checks for files in the same excluded directory.\n */\n-int path_excluded(struct path_exclude_check *check,\n-\t\t const char *name, int namelen, int *dtype)\n+int is_path_excluded(struct path_exclude_check *check,\n+\t\t const char *name, int namelen, int *dtype)\n {\n \tint i;\n \tstruct strbuf *path = &check->path;\ndiff --git a/dir.h b/dir.h\nindex 680c1eb..c59bad8 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -123,7 +123,7 @@ struct path_exclude_check {\n };\n extern void path_exclude_check_init(struct path_exclude_check *, struct dir_struct *);\n extern void path_exclude_check_clear(struct path_exclude_check *);\n-extern int path_excluded(struct path_exclude_check *, const char *, int namelen, int *dtype);\n+extern int is_path_excluded(struct path_exclude_check *, const char *, int namelen, int *dtype);\n \n \n extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 33a5819..3ac6370 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1372,7 +1372,7 @@ static int check_ok_to_remove(const char *name, int len, int dtype,\n \t\treturn 0;\n \n \tif (o->dir &&\n-\t path_excluded(o->path_exclude_check, name, -1, &dtype))\n+\t is_path_excluded(o->path_exclude_check, name, -1, &dtype))\n \t\t/*\n \t\t * ce->name is explicitly excluded, so it is Ok to\n \t\t * overwrite it.\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205526","messageId":"1356575558-2674-6-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 05/19] dir.c: rename excluded_from_list() to is_excluded_from_list()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:24Z","receivedAt":"2012-12-27T02:32:24Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Continue adopting clearer names for exclude functions. This 'is_*'\nnaming pattern for functions returning booleans was discussed here:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924\n\nAlso adjust their callers as necessary.\n\nSigned-off-by: Adam Spiers \n---\n dir.c | 11 ++++++-----\n dir.h | 4 ++--\n unpack-trees.c | 8 +++++---\n 3 files changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex f1c0abd..0800491 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -605,9 +605,9 @@ int match_pathname(const char *pathname, int pathlen,\n /* Scan the list and let the last match determine the fate.\n * Return 1 for exclude, 0 for include and -1 for undecided.\n */\n-int excluded_from_list(const char *pathname,\n-\t\t int pathlen, const char *basename, int *dtype,\n-\t\t struct exclude_list *el)\n+int is_excluded_from_list(const char *pathname,\n+\t\t\t int pathlen, const char *basename, int *dtype,\n+\t\t\t struct exclude_list *el)\n {\n \tint i;\n \n@@ -654,8 +654,9 @@ static int excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)\n \n \tprep_exclude(dir, pathname, basename-pathname);\n \tfor (st = EXC_CMDL; st <= EXC_FILE; st++) {\n-\t\tswitch (excluded_from_list(pathname, pathlen, basename,\n-\t\t\t\t\t dtype_p, &dir->exclude_list[st])) {\n+\t\tswitch (is_excluded_from_list(pathname, pathlen,\n+\t\t\t\t\t basename, dtype_p,\n+\t\t\t\t\t &dir->exclude_list[st])) {\n \t\tcase 0:\n \t\t\treturn 0;\n \t\tcase 1:\ndiff --git a/dir.h b/dir.h\nindex c59bad8..554f87a 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -98,8 +98,8 @@ extern int within_depth(const char *name, int namelen, int depth, int max_depth)\n extern int fill_directory(struct dir_struct *dir, const char **pathspec);\n extern int read_directory(struct dir_struct *, const char *path, int len, const char **pathspec);\n \n-extern int excluded_from_list(const char *pathname, int pathlen, const char *basename,\n-\t\t\t int *dtype, struct exclude_list *el);\n+extern int is_excluded_from_list(const char *pathname, int pathlen, const char *basename,\n+\t\t\t\t int *dtype, struct exclude_list *el);\n struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len);\n \n /*\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 3ac6370..c0da094 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -836,7 +836,8 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n {\n \tstruct cache_entry **cache_end;\n \tint dtype = DT_DIR;\n-\tint ret = excluded_from_list(prefix, prefix_len, basename, &dtype, el);\n+\tint ret = is_excluded_from_list(prefix, prefix_len,\n+\t\t\t\t\tbasename, &dtype, el);\n \n \tprefix[prefix_len++] = '/';\n \n@@ -855,7 +856,7 @@ static int clear_ce_flags_dir(struct cache_entry **cache, int nr,\n \t * with ret (iow, we know in advance the incl/excl\n \t * decision for the entire directory), clear flag here without\n \t * calling clear_ce_flags_1(). That function will call\n-\t * the expensive excluded_from_list() on every entry.\n+\t * the expensive is_excluded_from_list() on every entry.\n \t */\n \treturn clear_ce_flags_1(cache, cache_end - cache,\n \t\t\t\tprefix, prefix_len,\n@@ -938,7 +939,8 @@ static int clear_ce_flags_1(struct cache_entry **cache, int nr,\n \n \t\t/* Non-directory */\n \t\tdtype = ce_to_dtype(ce);\n-\t\tret = excluded_from_list(ce->name, ce_namelen(ce), name, &dtype, el);\n+\t\tret = is_excluded_from_list(ce->name, ce_namelen(ce),\n+\t\t\t\t\t name, &dtype, el);\n \t\tif (ret < 0)\n \t\t\tret = defval;\n \t\tif (ret > 0)\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205539","messageId":"1356575558-2674-7-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 06/19] dir.c: rename excluded() to is_excluded()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:25Z","receivedAt":"2012-12-27T02:32:25Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Continue adopting clearer names for exclude functions. This is_*\nnaming pattern for functions returning booleans was discussed here:\n\nhttp://thread.gmane.org/gmane.comp.version-control.git/204661/focus=204924\n\nSigned-off-by: Adam Spiers \n---\n attr.c | 2 +-\n dir.c | 10 +++++-----\n dir.h | 4 ++--\n 3 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/attr.c b/attr.c\nindex 2fc6353..5362563 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -284,7 +284,7 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,\n * (reading the file from top to bottom), .gitattribute of the root\n * directory (again, reading the file from top to bottom) down to the\n * current directory, and then scan the list backwards to find the first match.\n- * This is exactly the same as what excluded() does in dir.c to deal with\n+ * This is exactly the same as what is_excluded() does in dir.c to deal with\n * .gitignore\n */\n \ndiff --git a/dir.c b/dir.c\nindex 0800491..8c99dc4 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -645,7 +645,7 @@ int is_excluded_from_list(const char *pathname,\n \treturn -1; /* undecided */\n }\n \n-static int excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)\n+static int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)\n {\n \tint pathlen = strlen(pathname);\n \tint st;\n@@ -695,7 +695,7 @@ int is_path_excluded(struct path_exclude_check *check,\n \t/*\n \t * we allow the caller to pass namelen as an optimization; it\n \t * must match the length of the name, as we eventually call\n-\t * excluded() on the whole name string.\n+\t * is_excluded() on the whole name string.\n \t */\n \tif (namelen < 0)\n \t\tnamelen = strlen(name);\n@@ -712,7 +712,7 @@ int is_path_excluded(struct path_exclude_check *check,\n \n \t\tif (ch == '/') {\n \t\t\tint dt = DT_DIR;\n-\t\t\tif (excluded(check->dir, path->buf, &dt))\n+\t\t\tif (is_excluded(check->dir, path->buf, &dt))\n \t\t\t\treturn 1;\n \t\t}\n \t\tstrbuf_addch(path, ch);\n@@ -721,7 +721,7 @@ int is_path_excluded(struct path_exclude_check *check,\n \t/* An entry in the index; cannot be a directory with subentries */\n \tstrbuf_setlen(path, 0);\n \n-\treturn excluded(check->dir, name, dtype);\n+\treturn is_excluded(check->dir, name, dtype);\n }\n \n static struct dir_entry *dir_entry_new(const char *pathname, int len)\n@@ -1021,7 +1021,7 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t\t\t\t\t const struct path_simplify *simplify,\n \t\t\t\t\t int dtype, struct dirent *de)\n {\n-\tint exclude = excluded(dir, path->buf, &dtype);\n+\tint exclude = is_excluded(dir, path->buf, &dtype);\n \tif (exclude && (dir->flags & DIR_COLLECT_IGNORED)\n \t && exclude_matches_pathspec(path->buf, path->len, simplify))\n \t\tdir_add_ignored(dir, path->buf, path->len);\ndiff --git a/dir.h b/dir.h\nindex 554f87a..d68a997 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -113,8 +113,8 @@ extern int match_pathname(const char *, int,\n \t\t\t const char *, int, int, int);\n \n /*\n- * The excluded() API is meant for callers that check each level of leading\n- * directory hierarchies with excluded() to avoid recursing into excluded\n+ * The is_excluded() API is meant for callers that check each level of leading\n+ * directory hierarchies with is_excluded() to avoid recursing into excluded\n * directories. Callers that do not do so should use this API instead.\n */\n struct path_exclude_check {\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205528","messageId":"1356575558-2674-8-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 07/19] dir.c: refactor is_excluded_from_list()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:26Z","receivedAt":"2012-12-27T02:32:26Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"The excluded function uses a new helper function called\nlast_exclude_matching_from_list() to perform the inner loop over all of\nthe exclude patterns. The helper just tells us whether the path is\nincluded, excluded, or undecided.\n\nHowever, it may be useful to know _which_ pattern was triggered. So\nlet's pass out the entire exclude match, which contains the status\ninformation we were already passing out.\n\nFurther patches can make use of this.\n\nThis is a modified forward port of a patch from 2009 by Jeff King:\nhttp://article.gmane.org/gmane.comp.version-control.git/108815\n\nSigned-off-by: Adam Spiers \n---\n dir.c | 37 ++++++++++++++++++++++++++++---------\n 1 file changed, 28 insertions(+), 9 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 8c99dc4..d1a0413 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -602,22 +602,26 @@ int match_pathname(const char *pathname, int pathlen,\n \treturn fnmatch_icase(pattern, name, FNM_PATHNAME) == 0;\n }\n \n-/* Scan the list and let the last match determine the fate.\n- * Return 1 for exclude, 0 for include and -1 for undecided.\n+/*\n+ * Scan the given exclude list in reverse to see whether pathname\n+ * should be ignored. The first match (i.e. the last on the list), if\n+ * any, determines the fate. Returns the exclude_list element which\n+ * matched, or NULL for undecided.\n */\n-int is_excluded_from_list(const char *pathname,\n-\t\t\t int pathlen, const char *basename, int *dtype,\n-\t\t\t struct exclude_list *el)\n+static struct exclude *last_exclude_matching_from_list(const char *pathname,\n+\t\t\t\t\t\t int pathlen,\n+\t\t\t\t\t\t const char *basename,\n+\t\t\t\t\t\t int *dtype,\n+\t\t\t\t\t\t struct exclude_list *el)\n {\n \tint i;\n \n \tif (!el->nr)\n-\t\treturn -1;\t/* undefined */\n+\t\treturn NULL;\t/* undefined */\n \n \tfor (i = el->nr - 1; 0 <= i; i--) {\n \t\tstruct exclude *x = el->excludes[i];\n \t\tconst char *exclude = x->pattern;\n-\t\tint to_exclude = x->flags & EXC_FLAG_NEGATIVE ? 0 : 1;\n \t\tint prefix = x->nowildcardlen;\n \n \t\tif (x->flags & EXC_FLAG_MUSTBEDIR) {\n@@ -632,7 +636,7 @@ int is_excluded_from_list(const char *pathname,\n \t\t\t\t\t pathlen - (basename - pathname),\n \t\t\t\t\t exclude, prefix, x->patternlen,\n \t\t\t\t\t x->flags))\n-\t\t\t\treturn to_exclude;\n+\t\t\t\treturn x;\n \t\t\tcontinue;\n \t\t}\n \n@@ -640,8 +644,23 @@ int is_excluded_from_list(const char *pathname,\n \t\tif (match_pathname(pathname, pathlen,\n \t\t\t\t x->base, x->baselen ? x->baselen - 1 : 0,\n \t\t\t\t exclude, prefix, x->patternlen, x->flags))\n-\t\t\treturn to_exclude;\n+\t\t\treturn x;\n \t}\n+\treturn NULL; /* undecided */\n+}\n+\n+/*\n+ * Scan the list and let the last match determine the fate.\n+ * Return 1 for exclude, 0 for include and -1 for undecided.\n+ */\n+int is_excluded_from_list(const char *pathname,\n+\t\t\t int pathlen, const char *basename, int *dtype,\n+\t\t\t struct exclude_list *el)\n+{\n+\tstruct exclude *exclude;\n+\texclude = last_exclude_matching_from_list(pathname, pathlen, basename, dtype, el);\n+\tif (exclude)\n+\t\treturn exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;\n \treturn -1; /* undecided */\n }\n \n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205529","messageId":"1356575558-2674-9-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 08/19] dir.c: refactor is_excluded()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:27Z","receivedAt":"2012-12-27T02:32:27Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"In a similar way to the previous commit, this extracts a new helper\nfunction last_exclude_matching() which returns the last exclude_list\nelement which matched, or NULL if no match was found. is_excluded()\nbecomes a wrapper around this, and just returns 0 or 1 depending on\nwhether any matching exclude_list element was found.\n\nThis allows callers to find out _why_ a given path was excluded,\nrather than just whether it was or not, paving the way for a new git\nsub-command which allows users to test their exclude lists from the\ncommand line.\n\nSigned-off-by: Adam Spiers \n---\n dir.c | 38 +++++++++++++++++++++++++++++---------\n 1 file changed, 29 insertions(+), 9 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex d1a0413..b9d4234 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -664,24 +664,44 @@ int is_excluded_from_list(const char *pathname,\n \treturn -1; /* undecided */\n }\n \n-static int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)\n+/*\n+ * Loads the exclude lists for the directory containing pathname, then\n+ * scans all exclude lists to determine whether pathname is excluded.\n+ * Returns the exclude_list element which matched, or NULL for\n+ * undecided.\n+ */\n+static struct exclude *last_exclude_matching(struct dir_struct *dir,\n+\t\t\t\t\t const char *pathname,\n+\t\t\t\t\t int *dtype_p)\n {\n \tint pathlen = strlen(pathname);\n \tint st;\n+\tstruct exclude *exclude;\n \tconst char *basename = strrchr(pathname, '/');\n \tbasename = (basename) ? basename+1 : pathname;\n \n \tprep_exclude(dir, pathname, basename-pathname);\n \tfor (st = EXC_CMDL; st <= EXC_FILE; st++) {\n-\t\tswitch (is_excluded_from_list(pathname, pathlen,\n-\t\t\t\t\t basename, dtype_p,\n-\t\t\t\t\t &dir->exclude_list[st])) {\n-\t\tcase 0:\n-\t\t\treturn 0;\n-\t\tcase 1:\n-\t\t\treturn 1;\n-\t\t}\n+\t\texclude = last_exclude_matching_from_list(\n+\t\t\tpathname, pathlen, basename, dtype_p,\n+\t\t\t&dir->exclude_list[st]);\n+\t\tif (exclude)\n+\t\t\treturn exclude;\n \t}\n+\treturn NULL;\n+}\n+\n+/*\n+ * Loads the exclude lists for the directory containing pathname, then\n+ * scans all exclude lists to determine whether pathname is excluded.\n+ * Returns 1 if true, otherwise 0.\n+ */\n+static int is_excluded(struct dir_struct *dir, const char *pathname, int *dtype_p)\n+{\n+\tstruct exclude *exclude =\n+\t\tlast_exclude_matching(dir, pathname, dtype_p);\n+\tif (exclude)\n+\t\treturn exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;\n \treturn 0;\n }\n \n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205543","messageId":"1356575558-2674-10-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 09/19] dir.c: refactor is_path_excluded()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:28Z","receivedAt":"2012-12-27T02:32:28Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"In a similar way to the previous commit, this extracts a new helper\nfunction last_exclude_matching_path() which return the last\nexclude_list element which matched, or NULL if no match was found.\nis_path_excluded() becomes a wrapper around this, and just returns 0\nor 1 depending on whether any matching exclude_list element was found.\n\nThis allows callers to find out _why_ a given path was excluded,\nrather than just whether it was or not, paving the way for a new git\nsub-command which allows users to test their exclude lists from the\ncommand line.\n\nSigned-off-by: Adam Spiers \n---\n dir.c | 47 ++++++++++++++++++++++++++++++++++++++---------\n dir.h | 3 +++\n 2 files changed, 41 insertions(+), 9 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex b9d4234..16e10b0 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -709,6 +709,7 @@ void path_exclude_check_init(struct path_exclude_check *check,\n \t\t\t struct dir_struct *dir)\n {\n \tcheck->dir = dir;\n+\tcheck->exclude = NULL;\n \tstrbuf_init(&check->path, 256);\n }\n \n@@ -718,18 +719,21 @@ void path_exclude_check_clear(struct path_exclude_check *check)\n }\n \n /*\n- * Is this name excluded? This is for a caller like show_files() that\n- * do not honor directory hierarchy and iterate through paths that are\n- * possibly in an ignored directory.\n+ * For each subdirectory in name, starting with the top-most, checks\n+ * to see if that subdirectory is excluded, and if so, returns the\n+ * corresponding exclude structure. Otherwise, checks whether name\n+ * itself (which is presumably a file) is excluded.\n *\n * A path to a directory known to be excluded is left in check->path to\n * optimize for repeated checks for files in the same excluded directory.\n */\n-int is_path_excluded(struct path_exclude_check *check,\n-\t\t const char *name, int namelen, int *dtype)\n+struct exclude *last_exclude_matching_path(struct path_exclude_check *check,\n+\t\t\t\t\t const char *name, int namelen,\n+\t\t\t\t\t int *dtype)\n {\n \tint i;\n \tstruct strbuf *path = &check->path;\n+\tstruct exclude *exclude;\n \n \t/*\n \t * we allow the caller to pass namelen as an optimization; it\n@@ -739,11 +743,17 @@ int is_path_excluded(struct path_exclude_check *check,\n \tif (namelen < 0)\n \t\tnamelen = strlen(name);\n \n+\t/*\n+\t * If path is non-empty, and name is equal to path or a\n+\t * subdirectory of path, name should be excluded, because\n+\t * it's inside a directory which is already known to be\n+\t * excluded and was previously left in check->path.\n+\t */\n \tif (path->len &&\n \t path->len <= namelen &&\n \t !memcmp(name, path->buf, path->len) &&\n \t (!name[path->len] || name[path->len] == '/'))\n-\t\treturn 1;\n+\t\treturn check->exclude;\n \n \tstrbuf_setlen(path, 0);\n \tfor (i = 0; name[i]; i++) {\n@@ -751,8 +761,12 @@ int is_path_excluded(struct path_exclude_check *check,\n \n \t\tif (ch == '/') {\n \t\t\tint dt = DT_DIR;\n-\t\t\tif (is_excluded(check->dir, path->buf, &dt))\n-\t\t\t\treturn 1;\n+\t\t\texclude = last_exclude_matching(check->dir,\n+\t\t\t\t\t\t\tpath->buf, &dt);\n+\t\t\tif (exclude) {\n+\t\t\t\tcheck->exclude = exclude;\n+\t\t\t\treturn exclude;\n+\t\t\t}\n \t\t}\n \t\tstrbuf_addch(path, ch);\n \t}\n@@ -760,7 +774,22 @@ int is_path_excluded(struct path_exclude_check *check,\n \t/* An entry in the index; cannot be a directory with subentries */\n \tstrbuf_setlen(path, 0);\n \n-\treturn is_excluded(check->dir, name, dtype);\n+\treturn last_exclude_matching(check->dir, name, dtype);\n+}\n+\n+/*\n+ * Is this name excluded? This is for a caller like show_files() that\n+ * do not honor directory hierarchy and iterate through paths that are\n+ * possibly in an ignored directory.\n+ */\n+int is_path_excluded(struct path_exclude_check *check,\n+\t\t const char *name, int namelen, int *dtype)\n+{\n+\tstruct exclude *exclude =\n+\t\tlast_exclude_matching_path(check, name, namelen, dtype);\n+\tif (exclude)\n+\t\treturn exclude->flags & EXC_FLAG_NEGATIVE ? 0 : 1;\n+\treturn 0;\n }\n \n static struct dir_entry *dir_entry_new(const char *pathname, int len)\ndiff --git a/dir.h b/dir.h\nindex d68a997..dcb1ad3 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -119,10 +119,13 @@ extern int match_pathname(const char *, int,\n */\n struct path_exclude_check {\n \tstruct dir_struct *dir;\n+\tstruct exclude *exclude;\n \tstruct strbuf path;\n };\n extern void path_exclude_check_init(struct path_exclude_check *, struct dir_struct *);\n extern void path_exclude_check_clear(struct path_exclude_check *);\n+extern struct exclude *last_exclude_matching_path(struct path_exclude_check *, const char *,\n+\t\t\t\t\t\t int namelen, int *dtype);\n extern int is_path_excluded(struct path_exclude_check *, const char *, int namelen, int *dtype);\n \n \n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205530","messageId":"1356575558-2674-11-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 10/19] dir.c: rename free_excludes() to clear_exclude_list()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:29Z","receivedAt":"2012-12-27T02:32:29Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"It is clearer to use a 'clear_' prefix for functions which empty\nand deallocate the contents of a data structure without freeing\nthe structure itself, and a 'free_' prefix for functions which\nalso free the structure itself.\n\nhttp://article.gmane.org/gmane.comp.version-control.git/206128\n\nSigned-off-by: Adam Spiers \n---\n dir.c | 6 +++++-\n dir.h | 2 +-\n unpack-trees.c | 2 +-\n 3 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 16e10b0..41f141c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -400,7 +400,11 @@ static void *read_skip_worktree_file_from_index(const char *path, size_t *size)\n \treturn data;\n }\n \n-void free_excludes(struct exclude_list *el)\n+/*\n+ * Frees memory within el which was allocated for exclude patterns and\n+ * the file buffer. Does not free el itself.\n+ */\n+void clear_exclude_list(struct exclude_list *el)\n {\n \tint i;\n \ndiff --git a/dir.h b/dir.h\nindex dcb1ad3..5664ba8 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -135,7 +135,7 @@ extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n extern void parse_exclude_pattern(const char **string, int *patternlen, int *flags, int *nowildcardlen);\n extern void add_exclude(const char *string, const char *base,\n \t\t\tint baselen, struct exclude_list *el);\n-extern void free_excludes(struct exclude_list *el);\n+extern void clear_exclude_list(struct exclude_list *el);\n extern int file_exists(const char *);\n \n extern int is_inside_dir(const char *dir);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex c0da094..ad621d9 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1153,7 +1153,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \t\t*o->dst_index = o->result;\n \n done:\n-\tfree_excludes(&el);\n+\tclear_exclude_list(&el);\n \tif (o->path_exclude_check) {\n \t\tpath_exclude_check_clear(o->path_exclude_check);\n \t\tfree(o->path_exclude_check);\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205532","messageId":"1356575558-2674-12-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:30Z","receivedAt":"2012-12-27T02:32:30Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Previously each exclude_list could potentially contain patterns\nfrom multiple sources. For example dir->exclude_list[EXC_FILE]\nwould typically contain patterns from .git/info/exclude and\ncore.excludesfile, and dir->exclude_list[EXC_DIRS] could contain\npatterns from multiple per-directory .gitignore files during\ndirectory traversal (i.e. when dir->exclude_stack was more than\none item deep).\n\nWe split these composite exclude_lists up into three groups of\nexclude_lists (EXC_CMDL / EXC_DIRS / EXC_FILE as before), so that each\nexclude_list now contains patterns from a single source. This will\nallow us to cleanly track the origin of each pattern simply by adding\na src field to struct exclude_list, rather than to struct exclude,\nwhich would make memory management of the source string tricky in the\nEXC_DIRS case where its contents are dynamically generated.\n\nSimilarly, by moving the filebuf member from struct exclude_stack to\nstruct exclude_list, it allows us to track and subsequently free\nmemory buffers allocated during the parsing of all exclude files,\nrather than only tracking buffers allocated for files in the EXC_DIRS\ngroup.\n\nSigned-off-by: Adam Spiers \n---\n Documentation/technical/api-directory-listing.txt | 12 +++--\n builtin/clean.c | 3 +-\n builtin/ls-files.c | 8 +--\n dir.c | 60 ++++++++++++++++-------\n dir.h | 36 ++++++++++----\n unpack-trees.c | 2 +-\n 6 files changed, 83 insertions(+), 38 deletions(-)\n\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex 944fc39..fa9c8ae 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -67,11 +67,13 @@ marked. If you to exclude files, make sure you have loaded index first.\n * Prepare `struct dir_struct dir` and clear it with `memset(&dir, 0,\n sizeof(dir))`.\n \n-* Call `add_exclude()` to add single exclude pattern,\n- `add_excludes_from_file()` to add patterns from a file\n- (e.g. `.git/info/exclude`), and/or set `dir.exclude_per_dir`. A\n- short-hand function `setup_standard_excludes()` can be used to set up\n- the standard set of exclude settings.\n+* To add single exclude pattern, call `add_exclude_list()` and then\n+ `add_exclude()`.\n+\n+* To add patterns from a file (e.g. `.git/info/exclude`), call\n+ `add_excludes_from_file()` , and/or set `dir.exclude_per_dir`. A\n+ short-hand function `setup_standard_excludes()` can be used to set\n+ up the standard set of exclude settings.\n \n * Set options described in the Data Structure section above.\n \ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex 0c7b3d0..bd18b88 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tif (!ignored)\n \t\tsetup_standard_excludes(&dir);\n \n+\tadd_exclude_list(&dir, EXC_CMDL);\n \tfor (i = 0; i < exclude_list.nr; i++)\n \t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n-\t\t\t &dir.exclude_list[EXC_CMDL]);\n+\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0]);\n \n \tpathspec = get_pathspec(prefix, argv);\n \ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex ef7f99a..c448e06 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,\n static int option_parse_exclude(const struct option *opt,\n \t\t\t\tconst char *arg, int unset)\n {\n-\tstruct exclude_list *list = opt->value;\n+\tstruct exclude_list_group *group = opt->value;\n \n \texc_given = 1;\n-\tadd_exclude(arg, \"\", 0, list);\n+\tadd_exclude(arg, \"\", 0, &group->ary[0]);\n \n \treturn 0;\n }\n@@ -488,7 +488,8 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t\t\t\"show unmerged files in the output\"),\n \t\tOPT_BOOLEAN(0, \"resolve-undo\", &show_resolve_undo,\n \t\t\t \"show resolve-undo information\"),\n-\t\t{ OPTION_CALLBACK, 'x', \"exclude\", &dir.exclude_list[EXC_CMDL], \"pattern\",\n+\t\t{ OPTION_CALLBACK, 'x', \"exclude\",\n+\t\t\t&dir.exclude_list_groups[EXC_CMDL], \"pattern\",\n \t\t\t\"skip files matching pattern\",\n \t\t\t0, option_parse_exclude },\n \t\t{ OPTION_CALLBACK, 'X', \"exclude-from\", &dir, \"file\",\n@@ -523,6 +524,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \tif (read_cache() < 0)\n \t\tdie(\"index file corrupt\");\n \n+\tadd_exclude_list(&dir, EXC_CMDL);\n \targc = parse_options(argc, argv, prefix, builtin_ls_files_options,\n \t\t\tls_files_usage, 0);\n \tif (show_tag || show_valid_bit) {\ndiff --git a/dir.c b/dir.c\nindex 41f141c..3a9d89b 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -411,15 +411,16 @@ void clear_exclude_list(struct exclude_list *el)\n \tfor (i = 0; i < el->nr; i++)\n \t\tfree(el->excludes[i]);\n \tfree(el->excludes);\n+\tfree(el->filebuf);\n \n \tel->nr = 0;\n \tel->excludes = NULL;\n+\tel->filebuf = NULL;\n }\n \n int add_excludes_from_file_to_list(const char *fname,\n \t\t\t\t const char *base,\n \t\t\t\t int baselen,\n-\t\t\t\t char **buf_p,\n \t\t\t\t struct exclude_list *el,\n \t\t\t\t int check_index)\n {\n@@ -460,8 +461,7 @@ int add_excludes_from_file_to_list(const char *fname,\n \t\tclose(fd);\n \t}\n \n-\tif (buf_p)\n-\t\t*buf_p = buf;\n+\tel->filebuf = buf;\n \tentry = buf;\n \tfor (i = 0; i < size; i++) {\n \t\tif (buf[i] == '\\n') {\n@@ -475,10 +475,26 @@ int add_excludes_from_file_to_list(const char *fname,\n \treturn 0;\n }\n \n+struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type)\n+{\n+\tstruct exclude_list *el;\n+\tstruct exclude_list_group *group;\n+\n+\tgroup = &dir->exclude_list_groups[group_type];\n+\tALLOC_GROW(group->ary, group->nr + 1, group->alloc);\n+\tel = &group->ary[group->nr++];\n+\tmemset(el, 0, sizeof(*el));\n+\treturn el;\n+}\n+\n+/*\n+ * Used to set up core.excludesfile and .git/info/exclude lists.\n+ */\n void add_excludes_from_file(struct dir_struct *dir, const char *fname)\n {\n-\tif (add_excludes_from_file_to_list(fname, \"\", 0, NULL,\n-\t\t\t\t\t &dir->exclude_list[EXC_FILE], 0) < 0)\n+\tstruct exclude_list *el;\n+\tel = add_exclude_list(dir, EXC_FILE);\n+\tif (add_excludes_from_file_to_list(fname, \"\", 0, el, 0) < 0)\n \t\tdie(\"cannot use %s as an exclude file\", fname);\n }\n \n@@ -488,6 +504,7 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)\n */\n static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n {\n+\tstruct exclude_list_group *group;\n \tstruct exclude_list *el;\n \tstruct exclude_stack *stk = NULL;\n \tint current;\n@@ -496,17 +513,18 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \t (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))\n \t\treturn; /* too long a path -- ignore */\n \n+\tgroup = &dir->exclude_list_groups[EXC_DIRS];\n+\n \t/* Pop the directories that are not the prefix of the path being checked. */\n-\tel = &dir->exclude_list[EXC_DIRS];\n \twhile ((stk = dir->exclude_stack) != NULL) {\n \t\tif (stk->baselen <= baselen &&\n \t\t !strncmp(dir->basebuf, base, stk->baselen))\n \t\t\tbreak;\n+\t\tel = &group->ary[dir->exclude_stack->exclude_ix];\n \t\tdir->exclude_stack = stk->prev;\n-\t\twhile (stk->exclude_ix < el->nr)\n-\t\t\tfree(el->excludes[--el->nr]);\n-\t\tfree(stk->filebuf);\n+\t\tclear_exclude_list(el);\n \t\tfree(stk);\n+\t\tgroup->nr--;\n \t}\n \n \t/* Read from the parent directories and push them down. */\n@@ -527,13 +545,14 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \t\t}\n \t\tstk->prev = dir->exclude_stack;\n \t\tstk->baselen = cp - base;\n-\t\tstk->exclude_ix = el->nr;\n \t\tmemcpy(dir->basebuf + current, base + current,\n \t\t stk->baselen - current);\n \t\tstrcpy(dir->basebuf + stk->baselen, dir->exclude_per_dir);\n+\t\tel = add_exclude_list(dir, EXC_DIRS);\n+\t\tstk->exclude_ix = group->nr - 1;\n \t\tadd_excludes_from_file_to_list(dir->basebuf,\n \t\t\t\t\t dir->basebuf, stk->baselen,\n-\t\t\t\t\t &stk->filebuf, el, 1);\n+\t\t\t\t\t el, 1);\n \t\tdir->exclude_stack = stk;\n \t\tcurrent = stk->baselen;\n \t}\n@@ -679,18 +698,23 @@ static struct exclude *last_exclude_matching(struct dir_struct *dir,\n \t\t\t\t\t int *dtype_p)\n {\n \tint pathlen = strlen(pathname);\n-\tint st;\n+\tint i, j;\n+\tstruct exclude_list_group *group;\n \tstruct exclude *exclude;\n \tconst char *basename = strrchr(pathname, '/');\n \tbasename = (basename) ? basename+1 : pathname;\n \n \tprep_exclude(dir, pathname, basename-pathname);\n-\tfor (st = EXC_CMDL; st <= EXC_FILE; st++) {\n-\t\texclude = last_exclude_matching_from_list(\n-\t\t\tpathname, pathlen, basename, dtype_p,\n-\t\t\t&dir->exclude_list[st]);\n-\t\tif (exclude)\n-\t\t\treturn exclude;\n+\n+\tfor (i = EXC_CMDL; i <= EXC_FILE; i++) {\n+\t\tgroup = &dir->exclude_list_groups[i];\n+\t\tfor (j = group->nr - 1; j >= 0; j--) {\n+\t\t\texclude = last_exclude_matching_from_list(\n+\t\t\t\tpathname, pathlen, basename, dtype_p,\n+\t\t\t\t&group->ary[j]);\n+\t\t\tif (exclude)\n+\t\t\t\treturn exclude;\n+\t\t}\n \t}\n \treturn NULL;\n }\ndiff --git a/dir.h b/dir.h\nindex 5664ba8..9408fbe 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -16,14 +16,18 @@ struct dir_entry {\n #define EXC_FLAG_NEGATIVE 16\n \n /*\n- * Each .gitignore file will be parsed into patterns which are then\n- * appended to the relevant exclude_list (either EXC_DIRS or\n- * EXC_FILE). exclude_lists are also used to represent the list of\n- * --exclude values passed via CLI args (EXC_CMDL).\n+ * Each excludes file will be parsed into a fresh exclude_list which\n+ * is appended to the relevant exclude_list_group (either EXC_DIRS or\n+ * EXC_FILE). An exclude_list within the EXC_CMDL exclude_list_group\n+ * can also be used to represent the list of --exclude values passed\n+ * via CLI args.\n */\n struct exclude_list {\n \tint nr;\n \tint alloc;\n+\t/* remember pointer to exclude file contents so we can free() */\n+\tchar *filebuf;\n+\n \tstruct exclude {\n \t\tconst char *pattern;\n \t\tint patternlen;\n@@ -42,9 +46,13 @@ struct exclude_list {\n */\n struct exclude_stack {\n \tstruct exclude_stack *prev; /* the struct exclude_stack for the parent directory */\n-\tchar *filebuf; /* remember pointer to per-directory exclude file contents so we can free() */\n \tint baselen;\n-\tint exclude_ix;\n+\tint exclude_ix; /* index of exclude_list within EXC_DIRS exclude_list_group */\n+};\n+\n+struct exclude_list_group {\n+\tint nr, alloc;\n+\tstruct exclude_list *ary;\n };\n \n struct dir_struct {\n@@ -62,16 +70,23 @@ struct dir_struct {\n \n \t/* Exclude info */\n \tconst char *exclude_per_dir;\n-\tstruct exclude_list exclude_list[3];\n+\n \t/*\n-\t * We maintain three exclude pattern lists:\n+\t * We maintain three groups of exclude pattern lists:\n+\t *\n \t * EXC_CMDL lists patterns explicitly given on the command line.\n \t * EXC_DIRS lists patterns obtained from per-directory ignore files.\n-\t * EXC_FILE lists patterns from fallback ignore files.\n+\t * EXC_FILE lists patterns from fallback ignore files, e.g.\n+\t * - .git/info/exclude\n+\t * - core.excludesfile\n+\t *\n+\t * Each group contains multiple exclude lists, a single list\n+\t * per source.\n \t */\n #define EXC_CMDL 0\n #define EXC_DIRS 1\n #define EXC_FILE 2\n+\tstruct exclude_list_group exclude_list_groups[3];\n \n \t/*\n \t * Temporary variables which are used during loading of the\n@@ -129,8 +144,9 @@ extern struct exclude *last_exclude_matching_path(struct path_exclude_check *, c\n extern int is_path_excluded(struct path_exclude_check *, const char *, int namelen, int *dtype);\n \n \n+extern struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type);\n extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,\n-\t\t\t\t\t char **buf_p, struct exclude_list *el, int check_index);\n+\t\t\t\t\t struct exclude_list *el, int check_index);\n extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n extern void parse_exclude_pattern(const char **string, int *patternlen, int *flags, int *nowildcardlen);\n extern void add_exclude(const char *string, const char *base,\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ad621d9..de8da46 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -1019,7 +1019,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options\n \tif (!core_apply_sparse_checkout || !o->update)\n \t\to->skip_sparse_checkout = 1;\n \tif (!o->skip_sparse_checkout) {\n-\t\tif (add_excludes_from_file_to_list(git_path(\"info/sparse-checkout\"), \"\", 0, NULL, &el, 0) < 0)\n+\t\tif (add_excludes_from_file_to_list(git_path(\"info/sparse-checkout\"), \"\", 0, &el, 0) < 0)\n \t\t\to->skip_sparse_checkout = 1;\n \t\telse\n \t\t\to->el = ⪙\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205531","messageId":"1356575558-2674-13-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 12/19] dir.c: keep track of where patterns came from","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:31Z","receivedAt":"2012-12-27T02:32:31Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"For exclude patterns read in from files, the filename is stored in the\nexclude list, and the originating line number is stored in the\nindividual exclude (counting starting at 1).\n\nFor exclude patterns provided on the command line, a string describing\nthe source of the patterns is stored in the exclude list, and the\nsequence number assigned to each exclude pattern is negative, with\ncounting starting at -1. So for example the 2nd pattern provided via\n--exclude would be numbered -2. This allows any future consumers of\nthat data to easily distinguish between exclude patterns from files\nvs. from the CLI.\n\nSigned-off-by: Adam Spiers \n---\n builtin/clean.c | 4 ++--\n builtin/ls-files.c | 5 +++--\n dir.c | 26 ++++++++++++++++++++------\n dir.h | 21 +++++++++++++++++++--\n 4 files changed, 44 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex bd18b88..72d2876 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -97,10 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tif (!ignored)\n \t\tsetup_standard_excludes(&dir);\n \n-\tadd_exclude_list(&dir, EXC_CMDL);\n+\tadd_exclude_list(&dir, EXC_CMDL, \"--exclude option\");\n \tfor (i = 0; i < exclude_list.nr; i++)\n \t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n-\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0]);\n+\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0], -(i+1));\n \n \tpathspec = get_pathspec(prefix, argv);\n \ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex c448e06..d4e55c2 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -35,6 +35,7 @@ static int error_unmatch;\n static char *ps_matched;\n static const char *with_tree;\n static int exc_given;\n+static int exclude_args;\n \n static const char *tag_cached = \"\";\n static const char *tag_unmerged = \"\";\n@@ -423,7 +424,7 @@ static int option_parse_exclude(const struct option *opt,\n \tstruct exclude_list_group *group = opt->value;\n \n \texc_given = 1;\n-\tadd_exclude(arg, \"\", 0, &group->ary[0]);\n+\tadd_exclude(arg, \"\", 0, &group->ary[0], --exclude_args);\n \n \treturn 0;\n }\n@@ -524,7 +525,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \tif (read_cache() < 0)\n \t\tdie(\"index file corrupt\");\n \n-\tadd_exclude_list(&dir, EXC_CMDL);\n+\tadd_exclude_list(&dir, EXC_CMDL, \"--exclude option\");\n \targc = parse_options(argc, argv, prefix, builtin_ls_files_options,\n \t\t\tls_files_usage, 0);\n \tif (show_tag || show_valid_bit) {\ndiff --git a/dir.c b/dir.c\nindex 3a9d89b..aefe2bb 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -349,7 +349,7 @@ void parse_exclude_pattern(const char **pattern,\n }\n \n void add_exclude(const char *string, const char *base,\n-\t\t int baselen, struct exclude_list *el)\n+\t\t int baselen, struct exclude_list *el, int srcpos)\n {\n \tstruct exclude *x;\n \tint patternlen;\n@@ -373,8 +373,10 @@ void add_exclude(const char *string, const char *base,\n \tx->base = base;\n \tx->baselen = baselen;\n \tx->flags = flags;\n+\tx->srcpos = srcpos;\n \tALLOC_GROW(el->excludes, el->nr + 1, el->alloc);\n \tel->excludes[el->nr++] = x;\n+\tx->el = el;\n }\n \n static void *read_skip_worktree_file_from_index(const char *path, size_t *size)\n@@ -425,7 +427,7 @@ int add_excludes_from_file_to_list(const char *fname,\n \t\t\t\t int check_index)\n {\n \tstruct stat st;\n-\tint fd, i;\n+\tint fd, i, lineno = 1;\n \tsize_t size = 0;\n \tchar *buf, *entry;\n \n@@ -467,15 +469,17 @@ int add_excludes_from_file_to_list(const char *fname,\n \t\tif (buf[i] == '\\n') {\n \t\t\tif (entry != buf + i && entry[0] != '#') {\n \t\t\t\tbuf[i - (i && buf[i-1] == '\\r')] = 0;\n-\t\t\t\tadd_exclude(entry, base, baselen, el);\n+\t\t\t\tadd_exclude(entry, base, baselen, el, lineno);\n \t\t\t}\n+\t\t\tlineno++;\n \t\t\tentry = buf + i + 1;\n \t\t}\n \t}\n \treturn 0;\n }\n \n-struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type)\n+struct exclude_list *add_exclude_list(struct dir_struct *dir,\n+\t\t\t\t int group_type, const char *src)\n {\n \tstruct exclude_list *el;\n \tstruct exclude_list_group *group;\n@@ -484,6 +488,7 @@ struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type)\n \tALLOC_GROW(group->ary, group->nr + 1, group->alloc);\n \tel = &group->ary[group->nr++];\n \tmemset(el, 0, sizeof(*el));\n+\tel->src = src;\n \treturn el;\n }\n \n@@ -493,7 +498,7 @@ struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type)\n void add_excludes_from_file(struct dir_struct *dir, const char *fname)\n {\n \tstruct exclude_list *el;\n-\tel = add_exclude_list(dir, EXC_FILE);\n+\tel = add_exclude_list(dir, EXC_FILE, fname);\n \tif (add_excludes_from_file_to_list(fname, \"\", 0, el, 0) < 0)\n \t\tdie(\"cannot use %s as an exclude file\", fname);\n }\n@@ -522,6 +527,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \t\t\tbreak;\n \t\tel = &group->ary[dir->exclude_stack->exclude_ix];\n \t\tdir->exclude_stack = stk->prev;\n+\t\tfree((char *)el->src); /* see strdup() below */\n \t\tclear_exclude_list(el);\n \t\tfree(stk);\n \t\tgroup->nr--;\n@@ -548,7 +554,15 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n \t\tmemcpy(dir->basebuf + current, base + current,\n \t\t stk->baselen - current);\n \t\tstrcpy(dir->basebuf + stk->baselen, dir->exclude_per_dir);\n-\t\tel = add_exclude_list(dir, EXC_DIRS);\n+\t\t/*\n+\t\t * dir->basebuf gets reused by the traversal, but we\n+\t\t * need fname to remain unchanged to ensure the src\n+\t\t * member of each struct exclude correctly\n+\t\t * back-references its source file. Other invocations\n+\t\t * of add_exclude_list provide stable strings, so we\n+\t\t * strdup() and free() here in the caller.\n+\t\t */\n+\t\tel = add_exclude_list(dir, EXC_DIRS, strdup(dir->basebuf));\n \t\tstk->exclude_ix = group->nr - 1;\n \t\tadd_excludes_from_file_to_list(dir->basebuf,\n \t\t\t\t\t dir->basebuf, stk->baselen,\ndiff --git a/dir.h b/dir.h\nindex 9408fbe..f91770a 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -25,16 +25,32 @@ struct dir_entry {\n struct exclude_list {\n \tint nr;\n \tint alloc;\n+\n \t/* remember pointer to exclude file contents so we can free() */\n \tchar *filebuf;\n \n+\t/* origin of list, e.g. path to filename, or descriptive string */\n+\tconst char *src;\n+\n \tstruct exclude {\n+\t\t/*\n+\t\t * This allows callers of last_exclude_matching() etc.\n+\t\t * to determine the origin of the matching pattern.\n+\t\t */\n+\t\tstruct exclude_list *el;\n+\n \t\tconst char *pattern;\n \t\tint patternlen;\n \t\tint nowildcardlen;\n \t\tconst char *base;\n \t\tint baselen;\n \t\tint flags;\n+\n+\t\t/*\n+\t\t * Counting starts from 1 for line numbers in ignore files,\n+\t\t * and from -1 decrementing for patterns from CLI args.\n+\t\t */\n+\t\tint srcpos;\n \t} **excludes;\n };\n \n@@ -144,13 +160,14 @@ extern struct exclude *last_exclude_matching_path(struct path_exclude_check *, c\n extern int is_path_excluded(struct path_exclude_check *, const char *, int namelen, int *dtype);\n \n \n-extern struct exclude_list *add_exclude_list(struct dir_struct *dir, int group_type);\n+extern struct exclude_list *add_exclude_list(struct dir_struct *dir,\n+\t\t\t\t\t int group_type, const char *src);\n extern int add_excludes_from_file_to_list(const char *fname, const char *base, int baselen,\n \t\t\t\t\t struct exclude_list *el, int check_index);\n extern void add_excludes_from_file(struct dir_struct *, const char *fname);\n extern void parse_exclude_pattern(const char **string, int *patternlen, int *flags, int *nowildcardlen);\n extern void add_exclude(const char *string, const char *base,\n-\t\t\tint baselen, struct exclude_list *el);\n+\t\t\tint baselen, struct exclude_list *el, int srcpos);\n extern void clear_exclude_list(struct exclude_list *el);\n extern int file_exists(const char *);\n \n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205533","messageId":"1356575558-2674-14-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 13/19] dir.c: provide clear_directory() for reclaiming dir_struct memory","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:32Z","receivedAt":"2012-12-27T02:32:32Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"By the end of a directory traversal, a dir_struct instance will\ntypically contains pointers to various data structures on the heap.\nclear_directory() provides a convenient way to reclaim that memory.\n\nSigned-off-by: Adam Spiers \n---\n Documentation/technical/api-directory-listing.txt | 2 ++\n dir.c | 30 +++++++++++++++++++++++\n dir.h | 1 +\n 3 files changed, 33 insertions(+)\n\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex fa9c8ae..fbceb62 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -81,4 +81,6 @@ marked. If you to exclude files, make sure you have loaded index first.\n \n * Use `dir.entries[]`.\n \n+* Call `free_directory()` when none of the contained elements are no longer in use.\n+\n (JC)\ndiff --git a/dir.c b/dir.c\nindex aefe2bb..0d1b7e9 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1555,3 +1555,33 @@ void free_pathspec(struct pathspec *pathspec)\n \tfree(pathspec->items);\n \tpathspec->items = NULL;\n }\n+\n+/*\n+ * Frees memory within dir which was allocated for exclude lists and\n+ * the exclude_stack. Does not free dir itself.\n+ */\n+void clear_directory(struct dir_struct *dir)\n+{\n+\tint i, j;\n+\tstruct exclude_list_group *group;\n+\tstruct exclude_list *el;\n+\tstruct exclude_stack *stk;\n+\n+\tfor (i = EXC_CMDL; i <= EXC_FILE; i++) {\n+\t\tgroup = &dir->exclude_list_groups[i];\n+\t\tfor (j = 0; j < group->nr; j++) {\n+\t\t\tel = &group->ary[j];\n+\t\t\tif (i == EXC_DIRS)\n+\t\t\t\tfree((char *)el->src);\n+\t\t\tclear_exclude_list(el);\n+\t\t}\n+\t\tfree(group->ary);\n+\t}\n+\n+\tstk = dir->exclude_stack;\n+\twhile (stk) {\n+\t\tstruct exclude_stack *prev = stk->prev;\n+\t\tfree(stk);\n+\t\tstk = prev;\n+\t}\n+}\ndiff --git a/dir.h b/dir.h\nindex f91770a..286de4e 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -169,6 +169,7 @@ extern void parse_exclude_pattern(const char **string, int *patternlen, int *fla\n extern void add_exclude(const char *string, const char *base,\n \t\t\tint baselen, struct exclude_list *el, int srcpos);\n extern void clear_exclude_list(struct exclude_list *el);\n+extern void clear_directory(struct dir_struct *dir);\n extern int file_exists(const char *);\n \n extern int is_inside_dir(const char *dir);\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205538","messageId":"1356575558-2674-15-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 14/19] add.c: refactor treat_gitlinks()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:33Z","receivedAt":"2012-12-27T02:32:33Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Extract the body of the for loop in treat_gitlinks() into a separate\ntreat_gitlink() function so that it can be reused elsewhere. This\npaves the way for a new check-ignore sub-command.\n\nSigned-off-by: Adam Spiers \n---\n builtin/add.c | 50 ++++++++++++++++++++++++++++++++------------------\n 1 file changed, 32 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex c689f37..856d232 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -153,31 +153,45 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int\n \treturn seen;\n }\n \n-static void treat_gitlinks(const char **pathspec)\n+/*\n+ * Check whether path refers to a submodule, or something inside a\n+ * submodule. If the former, returns the path with any trailing slash\n+ * stripped. If the latter, dies with an error message.\n+ */\n+const char *treat_gitlink(const char *path)\n {\n-\tint i;\n-\n-\tif (!pathspec || !*pathspec)\n-\t\treturn;\n-\n+\tint i, path_len = strlen(path);\n \tfor (i = 0; i < active_nr; i++) {\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tif (S_ISGITLINK(ce->ce_mode)) {\n-\t\t\tint len = ce_namelen(ce), j;\n-\t\t\tfor (j = 0; pathspec[j]; j++) {\n-\t\t\t\tint len2 = strlen(pathspec[j]);\n-\t\t\t\tif (len2 <= len || pathspec[j][len] != '/' ||\n-\t\t\t\t memcmp(ce->name, pathspec[j], len))\n-\t\t\t\t\tcontinue;\n-\t\t\t\tif (len2 == len + 1)\n-\t\t\t\t\t/* strip trailing slash */\n-\t\t\t\t\tpathspec[j] = xstrndup(ce->name, len);\n-\t\t\t\telse\n-\t\t\t\t\tdie (_(\"Path '%s' is in submodule '%.*s'\"),\n-\t\t\t\t\t\tpathspec[j], len, ce->name);\n+\t\t\tint ce_len = ce_namelen(ce);\n+\t\t\tif (path_len <= ce_len || path[ce_len] != '/' ||\n+\t\t\t memcmp(ce->name, path, ce_len))\n+\t\t\t\t/* path does not refer to this\n+\t\t\t\t * submodule or anything inside it */\n+\t\t\t\tcontinue;\n+\t\t\tif (path_len == ce_len + 1) {\n+\t\t\t\t/* path refers to submodule;\n+\t\t\t\t * strip trailing slash */\n+\t\t\t\treturn xstrndup(ce->name, ce_len);\n+\t\t\t} else {\n+\t\t\t\tdie (_(\"Path '%s' is in submodule '%.*s'\"),\n+\t\t\t\t path, ce_len, ce->name);\n \t\t\t}\n \t\t}\n \t}\n+\treturn path;\n+}\n+\n+void treat_gitlinks(const char **pathspec)\n+{\n+\tint i;\n+\n+\tif (!pathspec || !*pathspec)\n+\t\treturn;\n+\n+\tfor (i = 0; pathspec[i]; i++)\n+\t\tpathspec[i] = treat_gitlink(pathspec[i]);\n }\n \n static void refresh(int verbose, const char **pathspec)\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205540","messageId":"1356575558-2674-16-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 15/19] add.c: remove unused argument from validate_pathspec()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:34Z","receivedAt":"2012-12-27T02:32:34Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"The 'argc' argument passed to validate_pathspec() was never used.\n\nSigned-off-by: Adam Spiers \n---\n builtin/add.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 856d232..1ba2a86 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -211,7 +211,7 @@ static void refresh(int verbose, const char **pathspec)\n free(seen);\n }\n \n-static const char **validate_pathspec(int argc, const char **argv, const char *prefix)\n+static const char **validate_pathspec(const char **argv, const char *prefix)\n {\n \tconst char **pathspec = get_pathspec(prefix, argv);\n \n@@ -262,7 +262,7 @@ int interactive_add(int argc, const char **argv, const char *prefix, int patch)\n \tconst char **pathspec = NULL;\n \n \tif (argc) {\n-\t\tpathspec = validate_pathspec(argc, argv, prefix);\n+\t\tpathspec = validate_pathspec(argv, prefix);\n \t\tif (!pathspec)\n \t\t\treturn -1;\n \t}\n@@ -428,7 +428,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tfprintf(stderr, _(\"Maybe you wanted to say 'git add .'?\\n\"));\n \t\treturn 0;\n \t}\n-\tpathspec = validate_pathspec(argc, argv, prefix);\n+\tpathspec = validate_pathspec(argv, prefix);\n \n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205536","messageId":"1356575558-2674-17-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:35Z","receivedAt":"2012-12-27T02:32:35Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Extract the following functions from builtin/add.c to pathspec.c, in\npreparation for reuse by a new git check-ignore command:\n\n - fill_pathspec_matches()\n - find_used_pathspec()\n - treat_gitlink()\n - treat_gitlinks()\n - validate_pathspec()\n\nThe functions being extracted are not changed in any way, except\nremoval of the 'static' qualifier.\n\nAlso add a comment documenting validate_pathspec().\n\nSigned-off-by: Adam Spiers \n---\n Makefile | 2 ++\n builtin/add.c | 92 +-----------------------------------------------------\n pathspec.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n pathspec.h | 5 +++\n 4 files changed, 107 insertions(+), 91 deletions(-)\n create mode 100644 pathspec.c\n create mode 100644 pathspec.h\n\ndiff --git a/Makefile b/Makefile\nindex 13293d3..48facad 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -645,6 +645,7 @@ LIB_H += pack-refs.h\n LIB_H += pack-revindex.h\n LIB_H += parse-options.h\n LIB_H += patch-ids.h\n+LIB_H += pathspec.h\n LIB_H += pkt-line.h\n LIB_H += progress.h\n LIB_H += prompt.h\n@@ -758,6 +759,7 @@ LIB_OBJS += parse-options-cb.o\n LIB_OBJS += patch-delta.o\n LIB_OBJS += patch-ids.o\n LIB_OBJS += path.o\n+LIB_OBJS += pathspec.o\n LIB_OBJS += pkt-line.o\n LIB_OBJS += preload-index.o\n LIB_OBJS += pretty.o\ndiff --git a/builtin/add.c b/builtin/add.c\nindex 1ba2a86..d3bae78 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -6,6 +6,7 @@\n #include \"cache.h\"\n #include \"builtin.h\"\n #include \"dir.h\"\n+#include \"pathspec.h\"\n #include \"exec_cmd.h\"\n #include \"cache-tree.h\"\n #include \"run-command.h\"\n@@ -97,39 +98,6 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n \treturn !!data.add_errors;\n }\n \n-static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)\n-{\n-\tint num_unmatched = 0, i;\n-\n-\t/*\n-\t * Since we are walking the index as if we were walking the directory,\n-\t * we have to mark the matched pathspec as seen; otherwise we will\n-\t * mistakenly think that the user gave a pathspec that did not match\n-\t * anything.\n-\t */\n-\tfor (i = 0; i < specs; i++)\n-\t\tif (!seen[i])\n-\t\t\tnum_unmatched++;\n-\tif (!num_unmatched)\n-\t\treturn;\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n-\t\tmatch_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen);\n-\t}\n-}\n-\n-static char *find_used_pathspec(const char **pathspec)\n-{\n-\tchar *seen;\n-\tint i;\n-\n-\tfor (i = 0; pathspec[i]; i++)\n-\t\t; /* just counting */\n-\tseen = xcalloc(i, 1);\n-\tfill_pathspec_matches(pathspec, seen, i);\n-\treturn seen;\n-}\n-\n static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n {\n \tchar *seen;\n@@ -153,47 +121,6 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int\n \treturn seen;\n }\n \n-/*\n- * Check whether path refers to a submodule, or something inside a\n- * submodule. If the former, returns the path with any trailing slash\n- * stripped. If the latter, dies with an error message.\n- */\n-const char *treat_gitlink(const char *path)\n-{\n-\tint i, path_len = strlen(path);\n-\tfor (i = 0; i < active_nr; i++) {\n-\t\tstruct cache_entry *ce = active_cache[i];\n-\t\tif (S_ISGITLINK(ce->ce_mode)) {\n-\t\t\tint ce_len = ce_namelen(ce);\n-\t\t\tif (path_len <= ce_len || path[ce_len] != '/' ||\n-\t\t\t memcmp(ce->name, path, ce_len))\n-\t\t\t\t/* path does not refer to this\n-\t\t\t\t * submodule or anything inside it */\n-\t\t\t\tcontinue;\n-\t\t\tif (path_len == ce_len + 1) {\n-\t\t\t\t/* path refers to submodule;\n-\t\t\t\t * strip trailing slash */\n-\t\t\t\treturn xstrndup(ce->name, ce_len);\n-\t\t\t} else {\n-\t\t\t\tdie (_(\"Path '%s' is in submodule '%.*s'\"),\n-\t\t\t\t path, ce_len, ce->name);\n-\t\t\t}\n-\t\t}\n-\t}\n-\treturn path;\n-}\n-\n-void treat_gitlinks(const char **pathspec)\n-{\n-\tint i;\n-\n-\tif (!pathspec || !*pathspec)\n-\t\treturn;\n-\n-\tfor (i = 0; pathspec[i]; i++)\n-\t\tpathspec[i] = treat_gitlink(pathspec[i]);\n-}\n-\n static void refresh(int verbose, const char **pathspec)\n {\n \tchar *seen;\n@@ -211,23 +138,6 @@ static void refresh(int verbose, const char **pathspec)\n free(seen);\n }\n \n-static const char **validate_pathspec(const char **argv, const char *prefix)\n-{\n-\tconst char **pathspec = get_pathspec(prefix, argv);\n-\n-\tif (pathspec) {\n-\t\tconst char **p;\n-\t\tfor (p = pathspec; *p; p++) {\n-\t\t\tif (has_symlink_leading_path(*p, strlen(*p))) {\n-\t\t\t\tint len = prefix ? strlen(prefix) : 0;\n-\t\t\t\tdie(_(\"'%s' is beyond a symbolic link\"), *p + len);\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\treturn pathspec;\n-}\n-\n int run_add_interactive(const char *revision, const char *patch_mode,\n \t\t\tconst char **pathspec)\n {\ndiff --git a/pathspec.c b/pathspec.c\nnew file mode 100644\nindex 0000000..8aea0d2\n--- /dev/null\n+++ b/pathspec.c\n@@ -0,0 +1,99 @@\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"pathspec.h\"\n+\n+void fill_pathspec_matches(const char **pathspec, char *seen, int specs)\n+{\n+\tint num_unmatched = 0, i;\n+\n+\t/*\n+\t * Since we are walking the index as if we were walking the directory,\n+\t * we have to mark the matched pathspec as seen; otherwise we will\n+\t * mistakenly think that the user gave a pathspec that did not match\n+\t * anything.\n+\t */\n+\tfor (i = 0; i < specs; i++)\n+\t\tif (!seen[i])\n+\t\t\tnum_unmatched++;\n+\tif (!num_unmatched)\n+\t\treturn;\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct cache_entry *ce = active_cache[i];\n+\t\tmatch_pathspec(pathspec, ce->name, ce_namelen(ce), 0, seen);\n+\t}\n+}\n+\n+char *find_used_pathspec(const char **pathspec)\n+{\n+\tchar *seen;\n+\tint i;\n+\n+\tfor (i = 0; pathspec[i]; i++)\n+\t\t; /* just counting */\n+\tseen = xcalloc(i, 1);\n+\tfill_pathspec_matches(pathspec, seen, i);\n+\treturn seen;\n+}\n+\n+/*\n+ * Check whether path refers to a submodule, or something inside a\n+ * submodule. If the former, returns the path with any trailing slash\n+ * stripped. If the latter, dies with an error message.\n+ */\n+const char *treat_gitlink(const char *path)\n+{\n+\tint i, path_len = strlen(path);\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct cache_entry *ce = active_cache[i];\n+\t\tif (S_ISGITLINK(ce->ce_mode)) {\n+\t\t\tint ce_len = ce_namelen(ce);\n+\t\t\tif (path_len <= ce_len || path[ce_len] != '/' ||\n+\t\t\t memcmp(ce->name, path, ce_len))\n+\t\t\t\t/* path does not refer to this\n+\t\t\t\t * submodule or anything inside it */\n+\t\t\t\tcontinue;\n+\t\t\tif (path_len == ce_len + 1) {\n+\t\t\t\t/* path refers to submodule;\n+\t\t\t\t * strip trailing slash */\n+\t\t\t\treturn xstrndup(ce->name, ce_len);\n+\t\t\t} else {\n+\t\t\t\tdie (_(\"Path '%s' is in submodule '%.*s'\"),\n+\t\t\t\t path, ce_len, ce->name);\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn path;\n+}\n+\n+void treat_gitlinks(const char **pathspec)\n+{\n+\tint i;\n+\n+\tif (!pathspec || !*pathspec)\n+\t\treturn;\n+\n+\tfor (i = 0; pathspec[i]; i++)\n+\t\tpathspec[i] = treat_gitlink(pathspec[i]);\n+}\n+\n+/*\n+ * Normalizes argv relative to prefix, via get_pathspec(), and then\n+ * dies if any path in the normalized list refers to a file inside a\n+ * symlinked directory.\n+ */\n+const char **validate_pathspec(const char **argv, const char *prefix)\n+{\n+\tconst char **pathspec = get_pathspec(prefix, argv);\n+\n+\tif (pathspec) {\n+\t\tconst char **p;\n+\t\tfor (p = pathspec; *p; p++) {\n+\t\t\tif (has_symlink_leading_path(*p, strlen(*p))) {\n+\t\t\t\tint len = prefix ? strlen(prefix) : 0;\n+\t\t\t\tdie(_(\"'%s' is beyond a symbolic link\"), *p + len);\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\treturn pathspec;\n+}\ndiff --git a/pathspec.h b/pathspec.h\nnew file mode 100644\nindex 0000000..8bb670b\n--- /dev/null\n+++ b/pathspec.h\n@@ -0,0 +1,5 @@\n+extern char *find_used_pathspec(const char **pathspec);\n+extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);\n+extern const char *treat_gitlink(const char *path);\n+extern void treat_gitlinks(const char **pathspec);\n+extern const char **validate_pathspec(const char **argv, const char *prefix);\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205534","messageId":"1356575558-2674-18-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 17/19] pathspec.c: extract new validate_path() for reuse","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:36Z","receivedAt":"2012-12-27T02:32:36Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"This will be reused by a new git check-ignore command.\n\nSigned-off-by: Adam Spiers \n---\n pathspec.c | 20 ++++++++++++++------\n pathspec.h | 1 +\n 2 files changed, 15 insertions(+), 6 deletions(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 8aea0d2..6724121 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -77,9 +77,20 @@ void treat_gitlinks(const char **pathspec)\n }\n \n /*\n+ * Dies if the given path refers to a file inside a symlinked\n+ * directory.\n+ */\n+void validate_path(const char *path, const char *prefix)\n+{\n+\tif (has_symlink_leading_path(path, strlen(path))) {\n+\t\tint len = prefix ? strlen(prefix) : 0;\n+\t\tdie(_(\"'%s' is beyond a symbolic link\"), path + len);\n+\t}\n+}\n+\n+/*\n * Normalizes argv relative to prefix, via get_pathspec(), and then\n- * dies if any path in the normalized list refers to a file inside a\n- * symlinked directory.\n+ * runs validate_path() on each path in the normalized list.\n */\n const char **validate_pathspec(const char **argv, const char *prefix)\n {\n@@ -88,10 +99,7 @@ const char **validate_pathspec(const char **argv, const char *prefix)\n \tif (pathspec) {\n \t\tconst char **p;\n \t\tfor (p = pathspec; *p; p++) {\n-\t\t\tif (has_symlink_leading_path(*p, strlen(*p))) {\n-\t\t\t\tint len = prefix ? strlen(prefix) : 0;\n-\t\t\t\tdie(_(\"'%s' is beyond a symbolic link\"), *p + len);\n-\t\t\t}\n+\t\t\tvalidate_path(*p, prefix);\n \t\t}\n \t}\n \ndiff --git a/pathspec.h b/pathspec.h\nindex 8bb670b..c251441 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -2,4 +2,5 @@ extern char *find_used_pathspec(const char **pathspec);\n extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);\n extern const char *treat_gitlink(const char *path);\n extern void treat_gitlinks(const char **pathspec);\n+extern void validate_path(const char *path, const char *prefix);\n extern const char **validate_pathspec(const char **argv, const char *prefix);\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205535","messageId":"1356575558-2674-19-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 18/19] setup.c: document get_pathspec()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:37Z","receivedAt":"2012-12-27T02:32:37Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"Since we have just created a new pathspec-handling library, now is a\ngood time to add some comments explaining get_pathspec().\n\nSigned-off-by: Adam Spiers \n---\n setup.c | 15 +++++++++++++++\n 1 file changed, 15 insertions(+)\n\ndiff --git a/setup.c b/setup.c\nindex 7663a4c..03d6d5c 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -249,6 +249,21 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char\n \t\treturn prefix_path(prefix, prefixlen, copyfrom);\n }\n \n+/*\n+ * prefix - a path relative to the root of the working tree\n+ * pathspec - a list of paths underneath the prefix path\n+ *\n+ * Iterates over pathspec, prepending each path with prefix,\n+ * and return the resulting list.\n+ *\n+ * If pathspec is empty, return a singleton list containing prefix.\n+ *\n+ * If pathspec and prefix are both empty, return an empty list.\n+ *\n+ * This is typically used by built-in commands such as add.c, in order\n+ * to normalize argv arguments provided to the built-in into a list of\n+ * paths to process, all relative to the root of the working tree.\n+ */\n const char **get_pathspec(const char *prefix, const char **pathspec)\n {\n \tconst char *entry = *pathspec;\n-- \n1.7.11.2.249.g31c7954\n"},{"id":"205537","messageId":"1356575558-2674-20-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"[PATCH v3 19/19] Add git-check-ignore sub-command","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-27T02:32:38Z","receivedAt":"2012-12-27T02:32:38Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"This works in a similar manner to git-check-attr.\n\nThanks to Jeff King and Junio C Hamano for the idea:\nhttp://thread.gmane.org/gmane.comp.version-control.git/108671/focus=108815\n\nSigned-off-by: Adam Spiers \n---\nSince v2, the missing documentation has been fixed, and the erroneous\ntweak to t/t9902-completion.sh has been removed.\n\n .gitignore | 1 +\n Documentation/git-check-ignore.txt | 89 ++++\n Documentation/gitignore.txt | 6 +-\n Documentation/technical/api-directory-listing.txt | 2 +-\n Makefile | 1 +\n builtin.h | 1 +\n builtin/check-ignore.c | 170 +++++++\n command-list.txt | 1 +\n contrib/completion/git-completion.bash | 1 +\n git.c | 1 +\n t/t0008-ignores.sh | 595 ++++++++++++++++++++++\n 11 files changed, 865 insertions(+), 3 deletions(-)\n create mode 100644 Documentation/git-check-ignore.txt\n create mode 100644 builtin/check-ignore.c\n create mode 100755 t/t0008-ignores.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex f1acd3e..20ef4e8 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -19,6 +19,7 @@\n /git-bundle\n /git-cat-file\n /git-check-attr\n+/git-check-ignore\n /git-check-ref-format\n /git-checkout\n /git-checkout-index\ndiff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt\nnew file mode 100644\nindex 0000000..854e4d0\n--- /dev/null\n+++ b/Documentation/git-check-ignore.txt\n@@ -0,0 +1,89 @@\n+git-check-ignore(1)\n+===================\n+\n+NAME\n+----\n+git-check-ignore - Debug gitignore / exclude files\n+\n+\n+SYNOPSIS\n+--------\n+[verse]\n+'git check-ignore' [options] pathname...\n+'git check-ignore' [options] --stdin < \n+\n+DESCRIPTION\n+-----------\n+\n+For each pathname given via the command-line or from a file via\n+`--stdin`, show the pattern from .gitignore (or other input files to\n+the exclude mechanism) that decides if the pathname is excluded or\n+included. Later patterns within a file take precedence over earlier\n+ones.\n+\n+OPTIONS\n+-------\n+-q, --quiet::\n+\tDon't output anything, just set exit status. This is only\n+\tvalid with a single pathname.\n+\n+-v, --verbose::\n+\tAlso output details about the matching pattern (if any)\n+\tfor each given pathname.\n+\n+--stdin::\n+\tRead file names from stdin instead of from the command-line.\n+\n+-z::\n+\tThe output format is modified to be machine-parseable (see\n+\tbelow). If `--stdin` is also given, input paths are separated\n+\twith a NUL character instead of a linefeed character.\n+\n+OUTPUT\n+------\n+\n+By default, any of the given pathnames which match an ignore pattern\n+will be output, one per line. If no pattern matches a given path,\n+nothing will be output for that path; this means that path will not be\n+ignored.\n+\n+If `--verbose` is specified, the output is a series of lines of the form:\n+\n+ \n+\n+ is the path of a file being queried, is the\n+matching pattern, is the pattern's source file, and \n+is the line number of the pattern within that source. If the pattern\n+contained a `!` prefix or `/` suffix, it will be preserved in the\n+output. will be an absolute path when referring to the file\n+configured by `core.excludesfile`, or relative to the repository root\n+when referring to `.git/info/exclude` or a per-directory exclude file.\n+\n+If `-z` is specified, the pathnames in the output are delimited by the\n+null character; if `--verbose` is also specified then null characters\n+are also used instead of colons and hard tabs:\n+\n+ \n+\n+\n+EXIT STATUS\n+-----------\n+\n+0::\n+\tOne or more of the provided paths is ignored.\n+\n+1::\n+\tNone of the provided paths are ignored.\n+\n+128::\n+\tA fatal error was encountered.\n+\n+SEE ALSO\n+--------\n+linkgit:gitignore[5]\n+linkgit:gitconfig[5]\n+linkgit:git-ls-files[5]\n+\n+GIT\n+---\n+Part of the linkgit:git[1] suite\ndiff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt\nindex 2e7328b..f401b8c 100644\n--- a/Documentation/gitignore.txt\n+++ b/Documentation/gitignore.txt\n@@ -153,8 +153,10 @@ The second .gitignore prevents git from ignoring\n \n SEE ALSO\n --------\n-linkgit:git-rm[1], linkgit:git-update-index[1],\n-linkgit:gitrepository-layout[5]\n+linkgit:git-rm[1],\n+linkgit:git-update-index[1],\n+linkgit:gitrepository-layout[5],\n+linkgit:git-check-ignore[1]\n \n GIT\n ---\ndiff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\nindex fbceb62..9d3e352 100644\n--- a/Documentation/technical/api-directory-listing.txt\n+++ b/Documentation/technical/api-directory-listing.txt\n@@ -81,6 +81,6 @@ marked. If you to exclude files, make sure you have loaded index first.\n \n * Use `dir.entries[]`.\n \n-* Call `free_directory()` when none of the contained elements are no longer in use.\n+* Call `clear_directory()` when none of the contained elements are no longer in use.\n \n (JC)\ndiff --git a/Makefile b/Makefile\nindex 48facad..8476fc8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -822,6 +822,7 @@ BUILTIN_OBJS += builtin/branch.o\n BUILTIN_OBJS += builtin/bundle.o\n BUILTIN_OBJS += builtin/cat-file.o\n BUILTIN_OBJS += builtin/check-attr.o\n+BUILTIN_OBJS += builtin/check-ignore.o\n BUILTIN_OBJS += builtin/check-ref-format.o\n BUILTIN_OBJS += builtin/checkout-index.o\n BUILTIN_OBJS += builtin/checkout.o\ndiff --git a/builtin.h b/builtin.h\nindex dffb34e..d57faf4 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -58,6 +58,7 @@ extern int cmd_cat_file(int argc, const char **argv, const char *prefix);\n extern int cmd_checkout(int argc, const char **argv, const char *prefix);\n extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\n+extern int cmd_check_ignore(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nnew file mode 100644\nindex 0000000..c825736\n--- /dev/null\n+++ b/builtin/check-ignore.c\n@@ -0,0 +1,170 @@\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"dir.h\"\n+#include \"quote.h\"\n+#include \"pathspec.h\"\n+#include \"parse-options.h\"\n+\n+static int quiet, verbose, stdin_paths;\n+static const char * const check_ignore_usage[] = {\n+\"git check-ignore [options] pathname...\",\n+\"git check-ignore [options] --stdin < \",\n+NULL\n+};\n+\n+static int null_term_line;\n+\n+static const struct option check_ignore_options[] = {\n+\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n+\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n+\tOPT_GROUP(\"\"),\n+\tOPT_BOOLEAN(0, \"stdin\", &stdin_paths,\n+\t\t N_(\"read file names from stdin\")),\n+\tOPT_BOOLEAN('z', NULL, &null_term_line,\n+\t\t N_(\"input paths are terminated by a null character\")),\n+\tOPT_END()\n+};\n+\n+static void output_exclude(const char *path, struct exclude *exclude)\n+{\n+\tchar *bang = exclude->flags & EXC_FLAG_NEGATIVE ? \"!\" : \"\";\n+\tchar *dir = (exclude->flags & EXC_FLAG_MUSTBEDIR) ? \"/\" : \"\";\n+\tif (!null_term_line) {\n+\t\tif (!verbose) {\n+\t\t\twrite_name_quoted(path, stdout, '\\n');\n+\t\t} else {\n+\t\t\tquote_c_style(exclude->el->src, NULL, stdout, 0);\n+\t\t\tprintf(\":%d:%s%s%s\\t\",\n+\t\t\t exclude->srcpos,\n+\t\t\t bang, exclude->pattern, dir);\n+\t\t\tquote_c_style(path, NULL, stdout, 0);\n+\t\t\tfputc('\\n', stdout);\n+\t\t}\n+\t} else {\n+\t\tif (!verbose) {\n+\t\t\tprintf(\"%s%c\", path, '\\0');\n+\t\t} else {\n+\t\t\tprintf(\"%s%c%d%c%s%s%s%c%s%c\",\n+\t\t\t exclude->el->src, '\\0',\n+\t\t\t exclude->srcpos, '\\0',\n+\t\t\t bang, exclude->pattern, dir, '\\0',\n+\t\t\t path, '\\0');\n+\t\t}\n+\t}\n+}\n+\n+static int check_ignore(const char *prefix, const char **pathspec)\n+{\n+\tstruct dir_struct dir;\n+\tconst char *path;\n+\tchar *seen = NULL;\n+\tint num_ignored = 0;\n+\n+\t/* read_cache() is only necessary so we can watch out for submodules. */\n+\tif (read_cache() < 0)\n+\t\tdie(_(\"index file corrupt\"));\n+\n+\tmemset(&dir, 0, sizeof(dir));\n+\tdir.flags |= DIR_COLLECT_IGNORED;\n+\tsetup_standard_excludes(&dir);\n+\n+\tif (pathspec) {\n+\t\tint i;\n+\t\tstruct path_exclude_check check;\n+\t\tstruct exclude *exclude;\n+\n+\t\tpath_exclude_check_init(&check, &dir);\n+\t\tif (!seen)\n+\t\t\tseen = find_used_pathspec(pathspec);\n+\t\tfor (i = 0; pathspec[i]; i++) {\n+\t\t\tconst char *full_path;\n+\t\t\tpath = pathspec[i];\n+\t\t\tfull_path = prefix_path(prefix, prefix\n+\t\t\t\t\t\t? strlen(prefix) : 0, path);\n+\t\t\tfull_path = treat_gitlink(full_path);\n+\t\t\tvalidate_path(full_path, prefix);\n+\t\t\tif (!seen[i] && path[0]) {\n+\t\t\t\tint dtype = DT_UNKNOWN;\n+\t\t\t\texclude = last_exclude_matching_path(&check, full_path,\n+\t\t\t\t\t\t\t\t -1, &dtype);\n+\t\t\t\tif (exclude) {\n+\t\t\t\t\tif (!quiet)\n+\t\t\t\t\t\toutput_exclude(path, exclude);\n+\t\t\t\t\tnum_ignored++;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tfree(seen);\n+\t\tclear_directory(&dir);\n+\t\tpath_exclude_check_clear(&check);\n+\t} else {\n+\t\tprintf(\"no pathspec\\n\");\n+\t}\n+\treturn num_ignored;\n+}\n+\n+static int check_ignore_stdin_paths(const char *prefix)\n+{\n+\tstruct strbuf buf, nbuf;\n+\tchar **pathspec = NULL;\n+\tsize_t nr = 0, alloc = 0;\n+\tint line_termination = null_term_line ? 0 : '\\n';\n+\tint num_ignored;\n+\n+\tstrbuf_init(&buf, 0);\n+\tstrbuf_init(&nbuf, 0);\n+\twhile (strbuf_getline(&buf, stdin, line_termination) != EOF) {\n+\t\tif (line_termination && buf.buf[0] == '\"') {\n+\t\t\tstrbuf_reset(&nbuf);\n+\t\t\tif (unquote_c_style(&nbuf, buf.buf, NULL))\n+\t\t\t\tdie(\"line is badly quoted\");\n+\t\t\tstrbuf_swap(&buf, &nbuf);\n+\t\t}\n+\t\tALLOC_GROW(pathspec, nr + 1, alloc);\n+\t\tpathspec[nr] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));\n+\t\tstrcpy(pathspec[nr++], buf.buf);\n+\t}\n+\tALLOC_GROW(pathspec, nr + 1, alloc);\n+\tpathspec[nr] = NULL;\n+\tnum_ignored = check_ignore(prefix, (const char **)pathspec);\n+\tmaybe_flush_or_die(stdout, \"attribute to stdout\");\n+\tstrbuf_release(&buf);\n+\tstrbuf_release(&nbuf);\n+\tfree(pathspec);\n+\treturn num_ignored;\n+}\n+\n+int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n+{\n+\tint num_ignored = 0;\n+\n+\tgit_config(git_default_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, check_ignore_options,\n+\t\t\t check_ignore_usage, 0);\n+\n+\tif (stdin_paths) {\n+\t\tif (0 < argc)\n+\t\t\tdie(_(\"cannot specify pathnames with --stdin\"));\n+\t} else {\n+\t\tif (null_term_line)\n+\t\t\tdie(_(\"-z only makes sense with --stdin\"));\n+\t\tif (argc == 0)\n+\t\t\tdie(_(\"no path specified\"));\n+\t}\n+\tif (quiet) {\n+\t\tif (argc > 1)\n+\t\t\tdie(_(\"--quiet is only valid with a single pathname\"));\n+\t\tif (verbose)\n+\t\t\tdie(_(\"cannot have both --quiet and --verbose\"));\n+\t}\n+\n+\tif (stdin_paths) {\n+\t\tnum_ignored = check_ignore_stdin_paths(prefix);\n+\t} else {\n+\t\tnum_ignored = check_ignore(prefix, argv);\n+\t\tmaybe_flush_or_die(stdout, \"ignore to stdout\");\n+\t}\n+\n+\treturn num_ignored > 0 ? 0 : 1;\n+}\ndiff --git a/command-list.txt b/command-list.txt\nindex 14ea67a..ef7f39c 100644\n--- a/command-list.txt\n+++ b/command-list.txt\n@@ -12,6 +12,7 @@ git-branch mainporcelain common\n git-bundle mainporcelain\n git-cat-file plumbinginterrogators\n git-check-attr purehelpers\n+git-check-ignore purehelpers\n git-checkout mainporcelain common\n git-checkout-index plumbingmanipulators\n git-check-ref-format purehelpers\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 2e1b5e1..1fb896b 100755\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -842,6 +842,7 @@ __git_list_porcelain_commands ()\n \t\tarchimport) : import;;\n \t\tcat-file) : plumbing;;\n \t\tcheck-attr) : plumbing;;\n+\t\tcheck-ignore) : plumbing;;\n \t\tcheck-ref-format) : plumbing;;\n \t\tcheckout-index) : plumbing;;\n \t\tcommit-tree) : plumbing;;\ndiff --git a/git.c b/git.c\nindex d232de9..0b31e66 100644\n--- a/git.c\n+++ b/git.c\n@@ -340,6 +340,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"bundle\", cmd_bundle, RUN_SETUP_GENTLY },\n \t\t{ \"cat-file\", cmd_cat_file, RUN_SETUP },\n \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP },\n+\t\t{ \"check-ignore\", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"check-ref-format\", cmd_check_ref_format },\n \t\t{ \"checkout\", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"checkout-index\", cmd_checkout_index,\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nnew file mode 100755\nindex 0000000..3937ca4\n--- /dev/null\n+++ b/t/t0008-ignores.sh\n@@ -0,0 +1,595 @@\n+#!/bin/sh\n+\n+test_description=check-ignore\n+\n+. ./test-lib.sh\n+\n+init_vars () {\n+\tglobal_excludes=\"$(pwd)/global-excludes\"\n+}\n+\n+enable_global_excludes () {\n+\tinit_vars\n+\tgit config core.excludesfile \"$global_excludes\"\n+}\n+\n+expect_in () {\n+\tdest=\"$HOME/expected-$1\" text=\"$2\"\n+\tif test -z \"$text\"\n+\tthen\n+\t\t>\"$dest\" # avoid newline\n+\telse\n+\t\techo \"$text\" >\"$dest\"\n+\tfi\n+}\n+\n+expect () {\n+\texpect_in stdout \"$1\"\n+}\n+\n+expect_from_stdin () {\n+\tcat >\"$HOME/expected-stdout\"\n+}\n+\n+test_stderr () {\n+\texpected=\"$1\"\n+\texpect_in stderr \"$1\" &&\n+\ttest_cmp \"$HOME/expected-stderr\" \"$HOME/stderr\"\n+}\n+\n+stderr_contains () {\n+\tregexp=\"$1\"\n+\tif grep -q \"$regexp\" \"$HOME/stderr\"\n+\tthen\n+\t\treturn 0\n+\telse\n+\t\techo \"didn't find /$regexp/ in $HOME/stderr\"\n+\t\tcat \"$HOME/stderr\"\n+\t\treturn 1\n+\tfi\n+}\n+\n+stderr_empty_on_success () {\n+\texpect_code=\"$1\"\n+\tif test $expect_code = 0\n+\tthen\n+\t\ttest_stderr \"\"\n+\telse\n+\t\t# If we expect failure then stderr might or might not be empty\n+\t\t# due to --quiet - the caller can check its contents\n+\t\treturn 0\n+\tfi\n+}\n+\n+test_check_ignore () {\n+\targs=\"$1\" expect_code=\"${2:-0}\" global_args=\"$3\"\n+\n+\tinit_vars &&\n+\trm -f \"$HOME/stdout\" \"$HOME/stderr\" \"$HOME/cmd\" &&\n+\techo git $global_args check-ignore $quiet_opt $verbose_opt $args \\\n+\t\t>\"$HOME/cmd\" &&\n+\ttest_expect_code \"$expect_code\" \\\n+\t\tgit $global_args check-ignore $quiet_opt $verbose_opt $args \\\n+\t\t>\"$HOME/stdout\" 2>\"$HOME/stderr\" &&\n+\ttest_cmp \"$HOME/expected-stdout\" \"$HOME/stdout\" &&\n+\tstderr_empty_on_success \"$expect_code\"\n+}\n+\n+test_expect_success_multi () {\n+\tprereq=\n+\tif test $# -eq 4\n+\tthen\n+\t\tprereq=$1\n+\t\tshift\n+\tfi\n+\ttestname=\"$1\" expect_verbose=\"$2\" code=\"$3\"\n+\n+\texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n+\n+\ttest_expect_success $prereq \"$testname\" \"\n+\t\texpect '$expect' &&\n+\t\t$code\n+\t\"\n+\n+\tfor quiet_opt in '-q' '--quiet'\n+\tdo\n+\t\ttest_expect_success $prereq \"$testname${quiet_opt:+ with $quiet_opt}\" \"\n+\t\t\texpect '' &&\n+\t\t\t$code\n+\t\t\"\n+\tdone\n+\tquiet_opt=\n+\n+\tfor verbose_opt in '-v' '--verbose'\n+\tdo\n+\t\ttest_expect_success $prereq \"$testname${verbose_opt:+ with $verbose_opt}\" \"\n+\t\t\texpect '$expect_verbose' &&\n+\t\t\t$code\n+\t\t\"\n+\tdone\n+\tverbose_opt=\n+}\n+\n+test_expect_success 'setup' '\n+\tinit_vars\n+\tmkdir -p a/b/ignored-dir a/submodule b &&\n+\tif test_have_prereq SYMLINKS\n+\tthen\n+\t\tln -s b a/symlink\n+\tfi &&\n+\t(\n+\t\tcd a/submodule &&\n+\t\tgit init &&\n+\t\techo a > a &&\n+\t\tgit add a &&\n+\t\tgit commit -m\"commit in submodule\"\n+\t) &&\n+\tgit add a/submodule &&\n+\tcat <<-\\EOF >.gitignore &&\n+\t\tone\n+\tEOF\n+\tcat <<-\\EOF >a/.gitignore &&\n+\t\ttwo*\n+\t\t*three\n+\tEOF\n+\tcat <<-\\EOF >a/b/.gitignore &&\n+\t\tfour\n+\t\tfive\n+\t\t# this comment should affect the line numbers\n+\t\tsix\n+\t\tignored-dir/\n+\t\t# and so should this blank line:\n+\n+\t\t!on*\n+\t\t!two\n+\tEOF\n+\techo \"seven\" >a/b/ignored-dir/.gitignore &&\n+\ttest -n \"$HOME\" &&\n+\tcat <<-\\EOF >\"$global_excludes\" &&\n+\t\tglobalone\n+\t\t!globaltwo\n+\t\tglobalthree\n+\tEOF\n+\tcat <<-\\EOF >>.git/info/exclude\n+\t\tper-repo\n+\tEOF\n+'\n+\n+############################################################################\n+#\n+# test invalid inputs\n+\n+test_expect_success_multi 'empty command line' '' '\n+\ttest_check_ignore \"\" 128 &&\n+\tstderr_contains \"fatal: no path specified\"\n+'\n+\n+test_expect_success '-q with multiple args' '\n+\texpect \"\" &&\n+\ttest_check_ignore \"-q one two\" 128 &&\n+\tstderr_contains \"fatal: --quiet is only valid with a single pathname\"\n+'\n+\n+test_expect_success '--quiet with multiple args' '\n+\texpect \"\" &&\n+\ttest_check_ignore \"--quiet one two\" 128 &&\n+\tstderr_contains \"fatal: --quiet is only valid with a single pathname\"\n+'\n+\n+for verbose_opt in '-v' '--verbose'\n+do\n+\tfor quiet_opt in '-q' '--quiet'\n+\tdo\n+\t\ttest_expect_success \"$quiet_opt $verbose_opt\" \"\n+\t\t\texpect '' &&\n+\t\t\ttest_check_ignore '$quiet_opt $verbose_opt foo' 128 &&\n+\t\t\tstderr_contains 'fatal: cannot have both --quiet and --verbose'\n+\t\t\"\n+\tdone\n+done\n+\n+test_expect_success '--quiet with multiple args' '\n+\texpect \"\" &&\n+\ttest_check_ignore \"--quiet one two\" 128 &&\n+\tstderr_contains \"fatal: --quiet is only valid with a single pathname\"\n+'\n+\n+test_expect_success_multi 'erroneous use of --' '' '\n+\ttest_check_ignore \"--\" 128 &&\n+\tstderr_contains \"fatal: no path specified\"\n+'\n+\n+test_expect_success_multi '--stdin with superfluous arg' '' '\n+\ttest_check_ignore \"--stdin foo\" 128 &&\n+\tstderr_contains \"fatal: cannot specify pathnames with --stdin\"\n+'\n+\n+test_expect_success_multi '--stdin -z with superfluous arg' '' '\n+\ttest_check_ignore \"--stdin -z foo\" 128 &&\n+\tstderr_contains \"fatal: cannot specify pathnames with --stdin\"\n+'\n+\n+test_expect_success_multi '-z without --stdin' '' '\n+\ttest_check_ignore \"-z\" 128 &&\n+\tstderr_contains \"fatal: -z only makes sense with --stdin\"\n+'\n+\n+test_expect_success_multi '-z without --stdin and superfluous arg' '' '\n+\ttest_check_ignore \"-z foo\" 128 &&\n+\tstderr_contains \"fatal: -z only makes sense with --stdin\"\n+'\n+\n+test_expect_success_multi 'needs work tree' '' '\n+\t(\n+\t\tcd .git &&\n+\t\ttest_check_ignore \"foo\" 128\n+\t) &&\n+\tstderr_contains \"fatal: This operation must be run in a work tree\"\n+'\n+\n+############################################################################\n+#\n+# test standard ignores\n+\n+test_expect_success_multi \"top-level not ignored\" '' '\n+\ttest_check_ignore \"foo\" 1\n+'\n+\n+test_expect_success_multi \"top-level ignored\" \\\n+\t'.gitignore:1:one\tone' '\n+\ttest_check_ignore \"one\"\n+'\n+\n+test_expect_success_multi 'sub-directory ignore from top' \\\n+\t'.gitignore:1:one\ta/one' '\n+\ttest_check_ignore \"a/one\"\n+'\n+\n+test_expect_success 'sub-directory local ignore' '\n+\texpect \"a/3-three\" &&\n+\ttest_check_ignore \"a/3-three a/three-not-this-one\"\n+'\n+\n+test_expect_success 'sub-directory local ignore with --verbose' '\n+\texpect \"a/.gitignore:2:*three\ta/3-three\" &&\n+\ttest_check_ignore \"--verbose a/3-three a/three-not-this-one\"\n+'\n+\n+test_expect_success 'local ignore inside a sub-directory' '\n+\texpect \"3-three\" &&\n+\t(\n+\t\tcd a &&\n+\t\ttest_check_ignore \"3-three three-not-this-one\"\n+\t)\n+'\n+test_expect_success 'local ignore inside a sub-directory with --verbose' '\n+\texpect \"a/.gitignore:2:*three\t3-three\" &&\n+\t(\n+\t\tcd a &&\n+\t\ttest_check_ignore \"--verbose 3-three three-not-this-one\"\n+\t)\n+'\n+\n+test_expect_success_multi 'nested include' \\\n+\t'a/b/.gitignore:8:!on*\ta/b/one' '\n+\ttest_check_ignore \"a/b/one\"\n+'\n+\n+############################################################################\n+#\n+# test ignored sub-directories\n+\n+test_expect_success_multi 'ignored sub-directory' \\\n+\t'a/b/.gitignore:5:ignored-dir/\ta/b/ignored-dir' '\n+\ttest_check_ignore \"a/b/ignored-dir\"\n+'\n+\n+test_expect_success 'multiple files inside ignored sub-directory' '\n+\texpect_from_stdin <<-\\EOF &&\n+\t\ta/b/ignored-dir/foo\n+\t\ta/b/ignored-dir/twoooo\n+\t\ta/b/ignored-dir/seven\n+\tEOF\n+\ttest_check_ignore \"a/b/ignored-dir/foo a/b/ignored-dir/twoooo a/b/ignored-dir/seven\"\n+'\n+\n+test_expect_success 'multiple files inside ignored sub-directory with -v' '\n+\texpect_from_stdin <<-\\EOF &&\n+\t\ta/b/.gitignore:5:ignored-dir/\ta/b/ignored-dir/foo\n+\t\ta/b/.gitignore:5:ignored-dir/\ta/b/ignored-dir/twoooo\n+\t\ta/b/.gitignore:5:ignored-dir/\ta/b/ignored-dir/seven\n+\tEOF\n+\ttest_check_ignore \"-v a/b/ignored-dir/foo a/b/ignored-dir/twoooo a/b/ignored-dir/seven\"\n+'\n+\n+test_expect_success 'cd to ignored sub-directory' '\n+\texpect_from_stdin <<-\\EOF &&\n+\t\tfoo\n+\t\ttwoooo\n+\t\t../one\n+\t\tseven\n+\t\t../../one\n+\tEOF\n+\t(\n+\t\tcd a/b/ignored-dir &&\n+\t\ttest_check_ignore \"foo twoooo ../one seven ../../one\"\n+\t)\n+'\n+\n+test_expect_success 'cd to ignored sub-directory with -v' '\n+\texpect_from_stdin <<-\\EOF &&\n+\t\ta/b/.gitignore:5:ignored-dir/\tfoo\n+\t\ta/b/.gitignore:5:ignored-dir/\ttwoooo\n+\t\ta/b/.gitignore:8:!on*\t../one\n+\t\ta/b/.gitignore:5:ignored-dir/\tseven\n+\t\t.gitignore:1:one\t../../one\n+\tEOF\n+\t(\n+\t\tcd a/b/ignored-dir &&\n+\t\ttest_check_ignore \"-v foo twoooo ../one seven ../../one\"\n+\t)\n+'\n+\n+############################################################################\n+#\n+# test handling of symlinks\n+\n+test_expect_success_multi SYMLINKS 'symlink' '' '\n+\ttest_check_ignore \"a/symlink\" 1\n+'\n+\n+test_expect_success_multi SYMLINKS 'beyond a symlink' '' '\n+\ttest_check_ignore \"a/symlink/foo\" 128 &&\n+\ttest_stderr \"fatal: '\\''a/symlink/foo'\\'' is beyond a symbolic link\"\n+'\n+\n+test_expect_success_multi SYMLINKS 'beyond a symlink from subdirectory' '' '\n+\t(\n+\t\tcd a &&\n+\t\ttest_check_ignore \"symlink/foo\" 128\n+\t) &&\n+\ttest_stderr \"fatal: '\\''symlink/foo'\\'' is beyond a symbolic link\"\n+'\n+\n+############################################################################\n+#\n+# test handling of submodules\n+\n+test_expect_success_multi 'submodule' '' '\n+\ttest_check_ignore \"a/submodule/one\" 128 &&\n+\ttest_stderr \"fatal: Path '\\''a/submodule/one'\\'' is in submodule '\\''a/submodule'\\''\"\n+'\n+\n+test_expect_success_multi 'submodule from subdirectory' '' '\n+\t(\n+\t\tcd a &&\n+\t\ttest_check_ignore \"submodule/one\" 128\n+\t) &&\n+\ttest_stderr \"fatal: Path '\\''a/submodule/one'\\'' is in submodule '\\''a/submodule'\\''\"\n+'\n+\n+############################################################################\n+#\n+# test handling of global ignore files\n+\n+test_expect_success 'global ignore not yet enabled' '\n+\texpect_from_stdin <<-\\EOF &&\n+\t\t.git/info/exclude:7:per-repo\tper-repo\n+\t\ta/.gitignore:2:*three\ta/globalthree\n+\t\t.git/info/exclude:7:per-repo\ta/per-repo\n+\tEOF\n+\ttest_check_ignore \"-v globalone per-repo a/globalthree a/per-repo not-ignored a/globaltwo\"\n+'\n+\n+test_expect_success 'global ignore' '\n+\tenable_global_excludes &&\n+\texpect_from_stdin <<-\\EOF &&\n+\t\tglobalone\n+\t\tper-repo\n+\t\tglobalthree\n+\t\ta/globalthree\n+\t\ta/per-repo\n+\t\tglobaltwo\n+\tEOF\n+\ttest_check_ignore \"globalone per-repo globalthree a/globalthree a/per-repo not-ignored globaltwo\"\n+'\n+\n+test_expect_success 'global ignore with -v' '\n+\tenable_global_excludes &&\n+\texpect_from_stdin <<-EOF &&\n+\t\t$global_excludes:1:globalone\tglobalone\n+\t\t.git/info/exclude:7:per-repo\tper-repo\n+\t\t$global_excludes:3:globalthree\tglobalthree\n+\t\ta/.gitignore:2:*three\ta/globalthree\n+\t\t.git/info/exclude:7:per-repo\ta/per-repo\n+\t\t$global_excludes:2:!globaltwo\tglobaltwo\n+\tEOF\n+\ttest_check_ignore \"-v globalone per-repo globalthree a/globalthree a/per-repo not-ignored globaltwo\"\n+'\n+\n+############################################################################\n+#\n+# test --stdin\n+\n+cat <<-\\EOF >stdin\n+\tone\n+\tnot-ignored\n+\ta/one\n+\ta/not-ignored\n+\ta/b/on\n+\ta/b/one\n+\ta/b/one one\n+\t\"a/b/one two\"\n+\t\"a/b/one\\\"three\"\n+\ta/b/not-ignored\n+\ta/b/two\n+\ta/b/twooo\n+\tglobaltwo\n+\ta/globaltwo\n+\ta/b/globaltwo\n+\tb/globaltwo\n+EOF\n+cat <<-\\EOF >expected-default\n+\tone\n+\ta/one\n+\ta/b/on\n+\ta/b/one\n+\ta/b/one one\n+\ta/b/one two\n+\t\"a/b/one\\\"three\"\n+\ta/b/two\n+\ta/b/twooo\n+\tglobaltwo\n+\ta/globaltwo\n+\ta/b/globaltwo\n+\tb/globaltwo\n+EOF\n+cat <<-EOF >expected-verbose\n+\t.gitignore:1:one\tone\n+\t.gitignore:1:one\ta/one\n+\ta/b/.gitignore:8:!on*\ta/b/on\n+\ta/b/.gitignore:8:!on*\ta/b/one\n+\ta/b/.gitignore:8:!on*\ta/b/one one\n+\ta/b/.gitignore:8:!on*\ta/b/one two\n+\ta/b/.gitignore:8:!on*\t\"a/b/one\\\"three\"\n+\ta/b/.gitignore:9:!two\ta/b/two\n+\ta/.gitignore:1:two*\ta/b/twooo\n+\t$global_excludes:2:!globaltwo\tglobaltwo\n+\t$global_excludes:2:!globaltwo\ta/globaltwo\n+\t$global_excludes:2:!globaltwo\ta/b/globaltwo\n+\t$global_excludes:2:!globaltwo\tb/globaltwo\n+EOF\n+\n+sed -e 's/^\"//' -e 's/\\\\//' -e 's/\"$//' stdin | \\\n+\ttr \"\\n\" \"\\0\" >stdin0\n+sed -e 's/^\"//' -e 's/\\\\//' -e 's/\"$//' expected-default | \\\n+\ttr \"\\n\" \"\\0\" >expected-default0\n+sed -e 's/\t\"/\t/' -e 's/\\\\//' -e 's/\"$//' expected-verbose | \\\n+\ttr \":\\t\\n\" \"\\0\" >expected-verbose0\n+\n+test_expect_success '--stdin' '\n+\texpect_from_stdin stdin\n+\t../one\n+\t../not-ignored\n+\tone\n+\tnot-ignored\n+\tb/on\n+\tb/one\n+\tb/one one\n+\t\"b/one two\"\n+\t\"b/one\\\"three\"\n+\tb/two\n+\tb/not-ignored\n+\tb/twooo\n+\t../globaltwo\n+\tglobaltwo\n+\tb/globaltwo\n+\t../b/globaltwo\n+EOF\n+cat <<-\\EOF >expected-default\n+\t../one\n+\tone\n+\tb/on\n+\tb/one\n+\tb/one one\n+\tb/one two\n+\t\"b/one\\\"three\"\n+\tb/two\n+\tb/twooo\n+\t../globaltwo\n+\tglobaltwo\n+\tb/globaltwo\n+\t../b/globaltwo\n+EOF\n+cat <<-EOF >expected-verbose\n+\t.gitignore:1:one\t../one\n+\t.gitignore:1:one\tone\n+\ta/b/.gitignore:8:!on*\tb/on\n+\ta/b/.gitignore:8:!on*\tb/one\n+\ta/b/.gitignore:8:!on*\tb/one one\n+\ta/b/.gitignore:8:!on*\tb/one two\n+\ta/b/.gitignore:8:!on*\t\"b/one\\\"three\"\n+\ta/b/.gitignore:9:!two\tb/two\n+\ta/.gitignore:1:two*\tb/twooo\n+\t$global_excludes:2:!globaltwo\t../globaltwo\n+\t$global_excludes:2:!globaltwo\tglobaltwo\n+\t$global_excludes:2:!globaltwo\tb/globaltwo\n+\t$global_excludes:2:!globaltwo\t../b/globaltwo\n+EOF\n+\n+sed -e 's/^\"//' -e 's/\\\\//' -e 's/\"$//' stdin | \\\n+\ttr \"\\n\" \"\\0\" >stdin0\n+sed -e 's/^\"//' -e 's/\\\\//' -e 's/\"$//' expected-default | \\\n+\ttr \"\\n\" \"\\0\" >expected-default0\n+sed -e 's/\t\"/\t/' -e 's/\\\\//' -e 's/\"$//' expected-verbose | \\\n+\ttr \":\\t\\n\" \"\\0\" >expected-verbose0\n+\n+test_expect_success '--stdin from subdirectory' '\n+\texpect_from_stdin \nadamspiers.org> writes:\n\n> \n> This v3 re-roll of my check-\nignore series is a reasonably \nsubstantial\n> revamp over v2, and applies on \ntop of Junio's current\n> nd/attr-match-optim-more \nbranch (82dce998c202).\n> \n> All feedback and patches from \nthe v2 series has been \nincorporated, and\n> several other improvements \ntoo, including:\n> \n> - composite exclude_lists \nhave been split up into\n> exclude_list_groups each \ncontaining one exclude_list per \nsource\n> \n> - smaller commits for easier \nreview\n> \n> - minor memory leaks have \nbeen fixed and verified via \nvalgrind\n> \n> - t0007-ignores.sh has been \nrenumbered to t0008-ignores.sh \nto avoid\n> a conflict with t0007-git-\nvar.sh\n> \n> - improvements to \ndocumentation and comments\n> \n> For reference, the v2 series \nwas announced here:\n> \n> http://thread.gmane.org/\ngmane.comp.version-\ncontrol.git/204661/\nfocus=206074\n> \n> All tests pass except for t91*, \nsince there seems to be some\n> pre-existing breakage in \n82dce998c202 relating to git-\nsvn.\n> \n> Adam Spiers (19):\n> api-directory-listing.txt: \nupdate to match code\n> Improve documentation and \ncomments regarding directory \ntraversal API\n> dir.c: rename cryptic 'which' \nvariable to more consistent \nname\n> dir.c: rename path_excluded() \nto is_path_excluded()\n> dir.c: rename \nexcluded_from_list() to \nis_excluded_from_list()\n> dir.c: rename excluded() to \nis_excluded()\n> dir.c: refactor \nis_excluded_from_list()\n> dir.c: refactor is_excluded()\n> dir.c: refactor \nis_path_excluded()\n> dir.c: rename free_excludes() \nto clear_exclude_list()\n> dir.c: use a single struct \nexclude_list per source of \nexcludes\n> dir.c: keep track of where \npatterns came from\n> dir.c: provide clear_directory() \nfor reclaiming dir_struct memory\n> add.c: refactor treat_gitlinks()\n> add.c: remove unused \nargument from \nvalidate_pathspec()\n> pathspec.c: move reusable \ncode from builtin/add.c\n> pathspec.c: extract new \nvalidate_path() for reuse\n> setup.c: document \nget_pathspec()\n> Add git-check-ignore sub-\ncommand\n> \n> .gitignore \n| 1 +\n> Documentation/git-check-\nignore.txt | 89 ++++\n> Documentation/gitignore.txt \n| 6 +-\n> Documentation/technical/api-\ndirectory-listing.txt | 35 +-\n> Makefile \n| 3 +\n> attr.c | \n2 +-\n> builtin.h \n| 1 +\n> builtin/add.c \n| 84 +--\n> builtin/check-ignore.c \n| 170 +++++++\n> builtin/clean.c \n| 3 +-\n> builtin/ls-files.c \n| 11 +-\n> command-list.txt \n| 1 +\n> contrib/completion/git-\ncompletion.bash | 1 +\n> dir.c | \n243 +++++++--\n> dir.h | \n87 +++-\n> git.c | \n1 +\n> pathspec.c \n| 107 ++++\n> pathspec.h \n| 6 +\n> setup.c \n| 15 +\n> t/t0008-ignores.sh \n| 595 \n++++++++++++++++++++++\n> unpack-trees.c \n| 14 +-\n> 21 files changed, 1305 \ninsertions(+), 170 deletions(-)\n> create mode 100644 \nDocumentation/git-check-\nignore.txt\n> create mode 100644 builtin/\ncheck-ignore.c\n> create mode 100644 \npathspec.c\n> create mode 100644 \npathspec.h\n> create mode 100755 t/t0008-\nignores.sh\n> \n"},{"id":"205597","messageId":"7v38yqj9ix.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 00/19] new git check-ignore sub-command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-28T18:50:46Z","receivedAt":"2012-12-28T18:50:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> This v3 re-roll of my check-ignore series is a reasonably substantial\n> revamp over v2, and applies on top of Junio's current\n> nd/attr-match-optim-more branch (82dce998c202).\n\nThanks.\n\nDoes this (and should this, if it doesn't) interact with the more\nrecent discussion around \"git status --untracked/--ignored\" [*1*],\nwhich also wants to touch the recursive directory traversal logic in\n\"dir.c\"?\n\n\n\n[Reference]\n\n*1* http://thread.gmane.org/gmane.comp.version-control.git/212127/focus=212136\n"},{"id":"205600","messageId":"CAOkDyE8gfW9TvyP=iE7gVEXOqCpOqMRjpr=Vnyd_pnummy4Qsg@mail.gmail.com","threadId":"32464","inReplyTo":"7v38yqj9ix.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 00/19] new git check-ignore sub-command","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-28T19:39:46Z","receivedAt":"2012-12-28T19:39:46Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 28, 2012 at 6:50 PM, Junio C Hamano wrote:\n> Adam Spiers writes:\n>\n>> This v3 re-roll of my check-ignore series is a reasonably substantial\n>> revamp over v2, and applies on top of Junio's current\n>> nd/attr-match-optim-more branch (82dce998c202).\n>\n> Thanks.\n>\n> Does this (and should this, if it doesn't) interact with the more\n> recent discussion around \"git status --untracked/--ignored\" [*1*],\n> which also wants to touch the recursive directory traversal logic in\n> \"dir.c\"?\n\nI cannot think of a reason why they would or should interact. If I'm\nwrong, I expect that either set of unit tests would show me up :-)\n"},{"id":"205602","messageId":"CALWbr2wEVzjJ_Y+W9BmakvXCwdFR3OjVH+15tPaDeXsrwaO86w@mail.gmail.com","threadId":"32464","inReplyTo":"CAOkDyE8gfW9TvyP=iE7gVEXOqCpOqMRjpr=Vnyd_pnummy4Qsg@mail.gmail.com","subject":"Re: [PATCH v3 00/19] new git check-ignore sub-command","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2012-12-28T20:15:59Z","receivedAt":"2012-12-28T20:15:59Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"I think they will interact, but I need to have a deeper look to Adam's series.\nIf it does, do you want me to base my work on the top of his branch ?\n\nOn Fri, Dec 28, 2012 at 8:39 PM, Adam Spiers wrote:\n> On Fri, Dec 28, 2012 at 6:50 PM, Junio C Hamano wrote:\n>> Adam Spiers writes:\n>>\n>>> This v3 re-roll of my check-ignore series is a reasonably substantial\n>>> revamp over v2, and applies on top of Junio's current\n>>> nd/attr-match-optim-more branch (82dce998c202).\n>>\n>> Thanks.\n>>\n>> Does this (and should this, if it doesn't) interact with the more\n>> recent discussion around \"git status --untracked/--ignored\" [*1*],\n>> which also wants to touch the recursive directory traversal logic in\n>> \"dir.c\"?\n>\n> I cannot think of a reason why they would or should interact. If I'm\n> wrong, I expect that either set of unit tests would show me up :-)\n"},{"id":"205603","messageId":"7v8v8hj4t0.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-17-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-28T20:32:43Z","receivedAt":"2012-12-28T20:32:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> diff --git a/pathspec.h b/pathspec.h\n> new file mode 100644\n> index 0000000..8bb670b\n> --- /dev/null\n> +++ b/pathspec.h\n> @@ -0,0 +1,5 @@\n> +extern char *find_used_pathspec(const char **pathspec);\n> +extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);\n> +extern const char *treat_gitlink(const char *path);\n> +extern void treat_gitlinks(const char **pathspec);\n> +extern const char **validate_pathspec(const char **argv, const char *prefix);\n\nProtect this against multiple inclusion with \"#ifndef PATHSPEC_H\".\n"},{"id":"205605","messageId":"7v7go1j4ml.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-19-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 18/19] setup.c: document get_pathspec()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-28T20:36:34Z","receivedAt":"2012-12-28T20:36:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> Since we have just created a new pathspec-handling library, now is a\n> good time to add some comments explaining get_pathspec().\n>\n> Signed-off-by: Adam Spiers \n> ---\n\nYes, but we would rather not to see new users of this function added\nto our codebase in its current form, as explained in the nearby\ncomment. We would want to migrate everybody to \"struct pathspec\"\nbased interface to support magic pathspecs in the longer term.\n\n> setup.c | 15 +++++++++++++++\n> 1 file changed, 15 insertions(+)\n>\n> diff --git a/setup.c b/setup.c\n> index 7663a4c..03d6d5c 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -249,6 +249,21 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char\n> \t\treturn prefix_path(prefix, prefixlen, copyfrom);\n> }\n> \n> +/*\n> + * prefix - a path relative to the root of the working tree\n> + * pathspec - a list of paths underneath the prefix path\n> + *\n> + * Iterates over pathspec, prepending each path with prefix,\n> + * and return the resulting list.\n> + *\n> + * If pathspec is empty, return a singleton list containing prefix.\n> + *\n> + * If pathspec and prefix are both empty, return an empty list.\n> + *\n> + * This is typically used by built-in commands such as add.c, in order\n> + * to normalize argv arguments provided to the built-in into a list of\n> + * paths to process, all relative to the root of the working tree.\n> + */\n> const char **get_pathspec(const char *prefix, const char **pathspec)\n> {\n> \tconst char *entry = *pathspec;\n"},{"id":"205607","messageId":"CAOkDyE-UXGhe1aiWy_1_y7cvrmfvivSBxY9LHudOmeZD=M-12A@mail.gmail.com","threadId":"32464","inReplyTo":"7v7go1j4ml.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 18/19] setup.c: document get_pathspec()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-28T20:40:41Z","receivedAt":"2012-12-28T20:40:41Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 28, 2012 at 8:36 PM, Junio C Hamano wrote:\n> Adam Spiers writes:\n>\n>> Since we have just created a new pathspec-handling library, now is a\n>> good time to add some comments explaining get_pathspec().\n>>\n>> Signed-off-by: Adam Spiers \n>> ---\n>\n> Yes, but we would rather not to see new users of this function added\n> to our codebase in its current form, as explained in the nearby\n> comment. We would want to migrate everybody to \"struct pathspec\"\n> based interface to support magic pathspecs in the longer term.\n\nI see. Please feel free to drop that patch from the series or amend\nas you see fit.\n"},{"id":"205608","messageId":"7v38ypj490.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-18-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 17/19] pathspec.c: extract new validate_path() for reuse","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-28T20:44:43Z","receivedAt":"2012-12-28T20:44:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> This will be reused by a new git check-ignore command.\n>\n> Signed-off-by: Adam Spiers \n> ---\n> pathspec.c | 20 ++++++++++++++------\n> pathspec.h | 1 +\n> 2 files changed, 15 insertions(+), 6 deletions(-)\n>\n> diff --git a/pathspec.c b/pathspec.c\n> index 8aea0d2..6724121 100644\n> --- a/pathspec.c\n> +++ b/pathspec.c\n> @@ -77,9 +77,20 @@ void treat_gitlinks(const char **pathspec)\n> }\n> \n> /*\n> + * Dies if the given path refers to a file inside a symlinked\n> + * directory.\n> + */\n> +void validate_path(const char *path, const char *prefix)\n\nThe name needs to be a lot more specific.\n\nThere may be 47 different kinds of \"validations\" various callers may\nwant to do on a path, but this function only caters to one kind of\ncallers that want to make sure that the path refers to something\nthat we would directly add to our index.\n\n> +{\n> +\tif (has_symlink_leading_path(path, strlen(path))) {\n> +\t\tint len = prefix ? strlen(prefix) : 0;\n> +\t\tdie(_(\"'%s' is beyond a symbolic link\"), path + len);\n> +\t}\n> +}\n"},{"id":"205609","messageId":"CAOkDyE8vSyT=eJ4FxRwYgz7Jzqu1+0LSzxtq9iSSzJEgTD1M0g@mail.gmail.com","threadId":"32464","inReplyTo":"7v8v8hj4t0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-28T20:45:00Z","receivedAt":"2012-12-28T20:45:00Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 28, 2012 at 8:32 PM, Junio C Hamano wrote:\n> Adam Spiers writes:\n>\n>> diff --git a/pathspec.h b/pathspec.h\n>> new file mode 100644\n>> index 0000000..8bb670b\n>> --- /dev/null\n>> +++ b/pathspec.h\n>> @@ -0,0 +1,5 @@\n>> +extern char *find_used_pathspec(const char **pathspec);\n>> +extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);\n>> +extern const char *treat_gitlink(const char *path);\n>> +extern void treat_gitlinks(const char **pathspec);\n>> +extern const char **validate_pathspec(const char **argv, const char *prefix);\n>\n> Protect this against multiple inclusion with \"#ifndef PATHSPEC_H\".\n\nYep good idea, how should I submit this? It will cause a trivially\nresolvable conflict with the next patch in the series (17/19):\n\n pathspec.c: extract new validate_path() for reuse\n\nbut I'd prefer not to re-roll 16--19 when just 16 and 17 are sufficient.\n"},{"id":"205610","messageId":"7vy5ghhpi0.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-17-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-28T20:48:39Z","receivedAt":"2012-12-28T20:48:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> diff --git a/pathspec.c b/pathspec.c\n> new file mode 100644\n> index 0000000..8aea0d2\n> --- /dev/null\n> +++ b/pathspec.c\n> @@ -0,0 +1,99 @@\n> +#include \"cache.h\"\n> +#include \"dir.h\"\n> +#include \"pathspec.h\"\n> +\n> +void fill_pathspec_matches(const char **pathspec, char *seen, int specs)\n> +{\n\nIt did not matter while it was an implementation detail of \"git\nadd\", but as a public function, something needs to clarify that this\n\"fill\"s matches *against the index*, not against a tree or the files\nin the current directory. The same comment applies to all the\ninternal functions this patch exposes to the outside world.\n"},{"id":"205611","messageId":"CAOkDyE_XBcYxisnkneY2jV0Fiv4r=e_P45hYdSo9ru0Qq7TFCA@mail.gmail.com","threadId":"32464","inReplyTo":"7v38ypj490.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 17/19] pathspec.c: extract new validate_path() for reuse","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-28T21:08:57Z","receivedAt":"2012-12-28T21:08:57Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 28, 2012 at 8:44 PM, Junio C Hamano wrote:\n> Adam Spiers writes:\n>\n>> This will be reused by a new git check-ignore command.\n>>\n>> Signed-off-by: Adam Spiers \n>> ---\n>> pathspec.c | 20 ++++++++++++++------\n>> pathspec.h | 1 +\n>> 2 files changed, 15 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/pathspec.c b/pathspec.c\n>> index 8aea0d2..6724121 100644\n>> --- a/pathspec.c\n>> +++ b/pathspec.c\n>> @@ -77,9 +77,20 @@ void treat_gitlinks(const char **pathspec)\n>> }\n>>\n>> /*\n>> + * Dies if the given path refers to a file inside a symlinked\n>> + * directory.\n>> + */\n>> +void validate_path(const char *path, const char *prefix)\n>\n> The name needs to be a lot more specific.\n>\n> There may be 47 different kinds of \"validations\" various callers may\n> want to do on a path, but this function only caters to one kind of\n> callers that want to make sure that the path refers to something\n> that we would directly add to our index.\n>\n>> +{\n>> + if (has_symlink_leading_path(path, strlen(path))) {\n>> + int len = prefix ? strlen(prefix) : 0;\n>> + die(_(\"'%s' is beyond a symbolic link\"), path + len);\n>> + }\n>> +}\n\nGood point. Which do you prefer of these suggested names?\n\n- die_if_path_beyond_symlink()\n- validate_path_not_beyond_symlink()\n- die_if_symlink_leading_path()\n- validate_no_symlink_leading_path()\n- validate_path_addable_to_index()\n\nOr something else?\n"},{"id":"205612","messageId":"CAOkDyE_706JwjY24xHvyMb1YGNhJOo_L3T8+V4hHgDJYxdLeiA@mail.gmail.com","threadId":"32464","inReplyTo":"7vy5ghhpi0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-28T21:15:40Z","receivedAt":"2012-12-28T21:15:40Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 28, 2012 at 8:48 PM, Junio C Hamano wrote:\n> Adam Spiers writes:\n>\n>> diff --git a/pathspec.c b/pathspec.c\n>> new file mode 100644\n>> index 0000000..8aea0d2\n>> --- /dev/null\n>> +++ b/pathspec.c\n>> @@ -0,0 +1,99 @@\n>> +#include \"cache.h\"\n>> +#include \"dir.h\"\n>> +#include \"pathspec.h\"\n>> +\n>> +void fill_pathspec_matches(const char **pathspec, char *seen, int specs)\n>> +{\n>\n> It did not matter while it was an implementation detail of \"git\n> add\", but as a public function, something needs to clarify that this\n> \"fill\"s matches *against the index*, not against a tree or the files\n> in the current directory. The same comment applies to all the\n> internal functions this patch exposes to the outside world.\n\nPrior to submitting the v3 series, I attempted to understand\nfill_pathspec_matches() and find_used_pathspec() well enough to\ndocument them all, but I failed. Perhaps some kind soul could explain\nwhat is the exact purpose of these functions, and in particular the\nrole of char *seen in both?\n"},{"id":"205613","messageId":"7vtxr5ho01.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-20-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 19/19] Add git-check-ignore sub-command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-28T21:21:02Z","receivedAt":"2012-12-28T21:21:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\n> new file mode 100644\n> index 0000000..c825736\n> --- /dev/null\n> +++ b/builtin/check-ignore.c\n> @@ -0,0 +1,170 @@\n> +#include \"builtin.h\"\n> +#include \"cache.h\"\n> +#include \"dir.h\"\n> +#include \"quote.h\"\n> +#include \"pathspec.h\"\n> +#include \"parse-options.h\"\n> +\n> +static int quiet, verbose, stdin_paths;\n> +static const char * const check_ignore_usage[] = {\n> +\"git check-ignore [options] pathname...\",\n> +\"git check-ignore [options] --stdin < \",\n> +NULL\n> +};\n> +\n> +static int null_term_line;\n> +\n> +static const struct option check_ignore_options[] = {\n> +\tOPT__QUIET(&quiet, N_(\"suppress progress reporting\")),\n> +\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n> +\tOPT_GROUP(\"\"),\n> +\tOPT_BOOLEAN(0, \"stdin\", &stdin_paths,\n> +\t\t N_(\"read file names from stdin\")),\n> +\tOPT_BOOLEAN('z', NULL, &null_term_line,\n> +\t\t N_(\"input paths are terminated by a null character\")),\n> +\tOPT_END()\n> +};\n> +\n> +static void output_exclude(const char *path, struct exclude *exclude)\n> +{\n> +\tchar *bang = exclude->flags & EXC_FLAG_NEGATIVE ? \"!\" : \"\";\n> +\tchar *dir = (exclude->flags & EXC_FLAG_MUSTBEDIR) ? \"/\" : \"\";\n\nThat's inconsistent. Either s/bang/negated/ or s/dir/slash/ (but\nobviously not both).\n\n> +static int check_ignore(const char *prefix, const char **pathspec)\n> +{\n> +\tstruct dir_struct dir;\n> +...\n> +\tif (pathspec) {\n> +...\n> +\t} else {\n> +\t\tprintf(\"no pathspec\\n\");\n> +\t}\n\nIs this an error message, an informative warning, or what? The\ncommand is presumably for script consumption downstream of a pipe.\nDoes it make sense to emit this message to their standard input?\nDoes it even have to be output? Especially what should happen when\nthe user says \"--quiet\"?\n\nPerhaps\n\n\tif (!quiet)\n\t\tfprintf(stderr, \"no pathspec given.\\n\");\n\nor something?\n\n> +int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n> +{\n> +\tint num_ignored = 0;\n\nI do not think you need to initialize this one.\n\n> +\tif (stdin_paths) {\n> +\t\tnum_ignored = check_ignore_stdin_paths(prefix);\n> +\t} else {\n> +\t\tnum_ignored = check_ignore(prefix, argv);\n> +\t\tmaybe_flush_or_die(stdout, \"ignore to stdout\");\n> +\t}\n> +\n> +\treturn num_ignored > 0 ? 0 : 1;\n\nGiven that num_ignored will always be >=0, that is a funny way to\nsay\n\n\treturn !num_ignored;\n\nor if you plan to report a non-fatal error as negative return from\nthe two check_ignore* functions, perhaps:\n\n\treturn num_ignored <= 0;\n\n> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\n> new file mode 100755\n> index 0000000..3937ca4\n> --- /dev/null\n> +++ b/t/t0008-ignores.sh\n> @@ -0,0 +1,595 @@\n> +#!/bin/sh\n> +\n> +test_description=check-ignore\n> +\n> +. ./test-lib.sh\n> +\n> +init_vars () {\n> +\tglobal_excludes=\"$(pwd)/global-excludes\"\n> +}\n> +\n> +enable_global_excludes () {\n> +\tinit_vars\n> +\tgit config core.excludesfile \"$global_excludes\"\n> +}\n> +\n> +expect_in () {\n> +\tdest=\"$HOME/expected-$1\" text=\"$2\"\n> +\tif test -z \"$text\"\n> +\tthen\n> +\t\t>\"$dest\" # avoid newline\n> +\telse\n> +\t\techo \"$text\" >\"$dest\"\n> +\tfi\n> +}\n> +\n> +expect () {\n> +\texpect_in stdout \"$1\"\n> +}\n> +\n> +expect_from_stdin () {\n> +\tcat >\"$HOME/expected-stdout\"\n> +}\n> +\n> +test_stderr () {\n> +\texpected=\"$1\"\n> +\texpect_in stderr \"$1\" &&\n> +\ttest_cmp \"$HOME/expected-stderr\" \"$HOME/stderr\"\n> +}\n> +\n> +stderr_contains () {\n> +\tregexp=\"$1\"\n> +\tif grep -q \"$regexp\" \"$HOME/stderr\"\n\nPlease do not use \"grep -q\"; the output from commands in test\nharness is normally hidden already. This only makes script more\ncluttered and robs debuggers of potentially useful output when the\ntest script is run with \"-v\".\n\n> +test_check_ignore () {\n> +\targs=\"$1\" expect_code=\"${2:-0}\" global_args=\"$3\"\n> +\n> +\tinit_vars &&\n> +\trm -f \"$HOME/stdout\" \"$HOME/stderr\" \"$HOME/cmd\" &&\n> +\techo git $global_args check-ignore $quiet_opt $verbose_opt $args \\\n> +\t\t>\"$HOME/cmd\" &&\n\nDoes a debugger needs this \"cmd\" file when we already have \"-v\" option?\n\n> +test_expect_success_multi () {\n> +\tprereq=\n> +\tif test $# -eq 4\n> +\tthen\n> +\t\tprereq=$1\n> +\t\tshift\n> +\tfi\n> +\ttestname=\"$1\" expect_verbose=\"$2\" code=\"$3\"\n> +\n> +\texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n> +\n> +\ttest_expect_success $prereq \"$testname\" \"\n> +\t\texpect '$expect' &&\n> +\t\t$code\n> +\t\"\n\nThis is brittle when $expect may have single quotes, isn't it?\nPerhaps\n\n\ttest_expect_success $prereq \"$testname\" '\n\t\texpect \"$expect\" && eval \"$code\"\n '\n\nmay fix it, but in general I hate to see each test script trying to\ninvent their own mini-harness like this (and getting them wrong).\n"},{"id":"205614","messageId":"7vpq1thnvw.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 00/19] new git check-ignore sub-command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-28T21:23:31Z","receivedAt":"2012-12-28T21:23:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"After skimming the series twice quickly, I found that the early part\nof refactorings are excellently done. Making existing private\nfunctions into public needs a lot more careful thought on namings, I\nthink, though.\n\nThe end result looks promising. Thanks for a pleasant read.\n"},{"id":"205615","messageId":"7vlichhnj4.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"CALWbr2wEVzjJ_Y+W9BmakvXCwdFR3OjVH+15tPaDeXsrwaO86w@mail.gmail.com","subject":"Re: [PATCH v3 00/19] new git check-ignore sub-command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-12-28T21:31:11Z","receivedAt":"2012-12-28T21:31:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse writes:\n\n> I think they will interact, but I need to have a deeper look to Adam's series.\n> If it does, do you want me to base my work on the top of his branch ?\n\nNot necessarily. If it becomes absolutely necessary to introduce\npatch dependencies, I would rather see an addition of new command\nrebased on a fix.\n\nI just wanted to make sure that parties touching the same area of\nthe codebase (and me who will be merging their efforts) are aware of\nwhat is going on.\n"},{"id":"205624","messageId":"CAOkDyE_XiMV+apb4mo1T6Ukp72YSdh6LgbnXsxQb-i19Lu+kOg@mail.gmail.com","threadId":"32464","inReplyTo":"CAOkDyE8vSyT=eJ4FxRwYgz7Jzqu1+0LSzxtq9iSSzJEgTD1M0g@mail.gmail.com","subject":"Re: [PATCH v3 16/19] pathspec.c: move reusable code from builtin/add.c","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-29T00:40:12Z","receivedAt":"2012-12-29T00:40:12Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 28, 2012 at 8:45 PM, Adam Spiers wrote:\n> On Fri, Dec 28, 2012 at 8:32 PM, Junio C Hamano wrote:\n>> Adam Spiers writes:\n>>\n>>> diff --git a/pathspec.h b/pathspec.h\n>>> new file mode 100644\n>>> index 0000000..8bb670b\n>>> --- /dev/null\n>>> +++ b/pathspec.h\n>>> @@ -0,0 +1,5 @@\n>>> +extern char *find_used_pathspec(const char **pathspec);\n>>> +extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);\n>>> +extern const char *treat_gitlink(const char *path);\n>>> +extern void treat_gitlinks(const char **pathspec);\n>>> +extern const char **validate_pathspec(const char **argv, const char *prefix);\n>>\n>> Protect this against multiple inclusion with \"#ifndef PATHSPEC_H\".\n>\n> Yep good idea, how should I submit this? It will cause a trivially\n> resolvable conflict with the next patch in the series (17/19):\n>\n> pathspec.c: extract new validate_path() for reuse\n\nI was wrong about that - it didn't cause a conflict, although it does\nmarginally change the context at the end of the pathspec.h hunk in the\nabove patch.\n\n> but I'd prefer not to re-roll 16--19 when just 16 and 17 are sufficient.\n\nBased on your other feedback, all of 16--19 require changes, and as\nthings stand, conveniently nothing earlier in the series does, so I'll\nre-roll those four once the outstanding issues are all resolved.\n"},{"id":"205626","messageId":"CAOkDyE89fm5_z2V=VW5n+8XAvB6tE+DqciXttbhX29X8mWjXTQ@mail.gmail.com","threadId":"32464","inReplyTo":"CAOkDyE-UXGhe1aiWy_1_y7cvrmfvivSBxY9LHudOmeZD=M-12A@mail.gmail.com","subject":"Re: [PATCH v3 18/19] setup.c: document get_pathspec()","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-29T00:52:25Z","receivedAt":"2012-12-29T00:52:25Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 28, 2012 at 8:40 PM, Adam Spiers wrote:\n> On Fri, Dec 28, 2012 at 8:36 PM, Junio C Hamano wrote:\n>> Adam Spiers writes:\n>>\n>>> Since we have just created a new pathspec-handling library, now is a\n>>> good time to add some comments explaining get_pathspec().\n>>>\n>>> Signed-off-by: Adam Spiers \n>>> ---\n>>\n>> Yes, but we would rather not to see new users of this function added\n>> to our codebase in its current form, as explained in the nearby\n>> comment. We would want to migrate everybody to \"struct pathspec\"\n>> based interface to support magic pathspecs in the longer term.\n>\n> I see. Please feel free to drop that patch from the series or amend\n> as you see fit.\n\nI've added this sentence to the top of the comments above\nget_pathspec():\n\n /*\n * N.B. get_pathspec() is deprecated in favor of the \"struct pathspec\"\n * based interface - see pathspec_magic above.\n *\n [...]\n\nThat should be sufficient to discourage people from adding new users\nof get_pathspec().\n"},{"id":"205628","messageId":"20121229012352.GA20379@pacific.linksys.moosehall","threadId":"32464","inReplyTo":"7vtxr5ho01.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 19/19] Add git-check-ignore sub-command","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-29T01:23:52Z","receivedAt":"2012-12-29T01:23:52Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Dec 28, 2012 at 01:21:02PM -0800, Junio C Hamano wrote:\n> Adam Spiers writes:\n> > +static void output_exclude(const char *path, struct exclude *exclude)\n> > +{\n> > +\tchar *bang = exclude->flags & EXC_FLAG_NEGATIVE ? \"!\" : \"\";\n> > +\tchar *dir = (exclude->flags & EXC_FLAG_MUSTBEDIR) ? \"/\" : \"\";\n> \n> That's inconsistent. Either s/bang/negated/ or s/dir/slash/ (but\n> obviously not both).\n\nOK. I'll make the use of parentheses there consistent too.\n\n> > +static int check_ignore(const char *prefix, const char **pathspec)\n> > +{\n> > +\tstruct dir_struct dir;\n> > +...\n> > +\tif (pathspec) {\n> > +...\n> > +\t} else {\n> > +\t\tprintf(\"no pathspec\\n\");\n> > +\t}\n> \n> Is this an error message, an informative warning, or what? The\n> command is presumably for script consumption downstream of a pipe.\n> Does it make sense to emit this message to their standard input?\n> Does it even have to be output? Especially what should happen when\n> the user says \"--quiet\"?\n>\n> Perhaps\n> \n> \tif (!quiet)\n> \t\tfprintf(stderr, \"no pathspec given.\\n\");\n> \n> or something?\n\nHmm. I suspect this was an unfinished remnant of one of my early\nprototypes, because this code path never gets hit. There's even a\ntest which checks that a fatal error is generated when no pathspecs\nare given via argv. \n\nHowever, the test for behaviour when no pathspecs are given via\n--stdin *is* missing. In this case, I think the code you suggest\nabove makes sense for generating a warning, and the existing behaviour\nof returning an exit code of 1 also seems correct. I have added your\ncode and a test to cover it.\n\n> > +int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n> > +{\n> > +\tint num_ignored = 0;\n> \n> I do not think you need to initialize this one.\n\nFixed.\n\n> > +\tif (stdin_paths) {\n> > +\t\tnum_ignored = check_ignore_stdin_paths(prefix);\n> > +\t} else {\n> > +\t\tnum_ignored = check_ignore(prefix, argv);\n> > +\t\tmaybe_flush_or_die(stdout, \"ignore to stdout\");\n> > +\t}\n> > +\n> > +\treturn num_ignored > 0 ? 0 : 1;\n> \n> Given that num_ignored will always be >=0, that is a funny way to\n> say\n> \n> \treturn !num_ignored;\n\nPersonally I find that harder to read, but I'm not a regular C\nprogrammer so I'll defer to your sense of style.\n\n> or if you plan to report a non-fatal error as negative return from\n> the two check_ignore* functions,\n\nI don't think that's necessary here, but I'll bear it in mind.\n\n> > +stderr_contains () {\n> > +\tregexp=\"$1\"\n> > +\tif grep -q \"$regexp\" \"$HOME/stderr\"\n> \n> Please do not use \"grep -q\"; the output from commands in test\n> harness is normally hidden already. This only makes script more\n> cluttered and robs debuggers of potentially useful output when the\n> test script is run with \"-v\".\n\nFixed.\n\n> > +test_check_ignore () {\n> > +\targs=\"$1\" expect_code=\"${2:-0}\" global_args=\"$3\"\n> > +\n> > +\tinit_vars &&\n> > +\trm -f \"$HOME/stdout\" \"$HOME/stderr\" \"$HOME/cmd\" &&\n> > +\techo git $global_args check-ignore $quiet_opt $verbose_opt $args \\\n> > +\t\t>\"$HOME/cmd\" &&\n> \n> Does a debugger needs this \"cmd\" file when we already have \"-v\" option?\n\nYes, I think it's useful; for example:\n\n $ ./t0008-ignores.sh -i -v\n\n [... test fails ...]\n\n $ cd trash\\ directory.t0008-ignores\n $ gdb --args ../../$( > +test_expect_success_multi () {\n> > +\tprereq=\n> > +\tif test $# -eq 4\n> > +\tthen\n> > +\t\tprereq=$1\n> > +\t\tshift\n> > +\tfi\n> > +\ttestname=\"$1\" expect_verbose=\"$2\" code=\"$3\"\n> > +\n> > +\texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n> > +\n> > +\ttest_expect_success $prereq \"$testname\" \"\n> > +\t\texpect '$expect' &&\n> > +\t\t$code\n> > +\t\"\n> \n> This is brittle when $expect may have single quotes, isn't it?\n\nCurrently $expect will never have single quotes, but I grant this may\nchange in the future.\n\n> Perhaps\n> \n> \ttest_expect_success $prereq \"$testname\" '\n> \t\texpect \"$expect\" && eval \"$code\"\n> '\n> \n> may fix it,\n\nYes that works, thanks.\n\n> but in general I hate to see each test script trying to\n> invent their own mini-harness like this (and getting them wrong).\n\ntest_expect_success_multi is specific to check-ignore and avoids a\nhuge amount of code duplication. I can't think of a better approach\nbut if you can then I'm all ears.\n\nI believe this only leaves two items from your review of v3 which are\nyet to be resolved:\n\n1. What should validate_path() should be renamed to in order to avoid\n ambiguity with other path validation functions? Currently my\n preferred choice is die_if_path_beyond_symlink() but I don't really\n mind - I'll go with that unless I hear someone prefers another\n name.\n\n2. The now-public functions fill_pathspec_matches() and\n find_used_pathspec(), need to be documented, and in particular need\n to mention that they traverse the index not a tree or the working\n directory. I'll work on improving my understanding of these\n functions to the point where I can document them accurately (but\n hints are still welcome).\n\nOnce these are both resolved, I'll re-roll patches 16--19 only of the\nv3 series, labelled as v4.\n\nFYI, attached is the diff between check-ignore-v3 and my current\ncheck-ignore, which is available at github:\n\n https://github.com/aspiers/git/commits/check-ignore\n\ndiff --git a/builtin/check-ignore.c b/builtin/check-ignore.c\nindex c825736..06e250e 100644\n--- a/builtin/check-ignore.c\n+++ b/builtin/check-ignore.c\n@@ -27,8 +27,8 @@ static const struct option check_ignore_options[] = {\n \n static void output_exclude(const char *path, struct exclude *exclude)\n {\n-\tchar *bang = exclude->flags & EXC_FLAG_NEGATIVE ? \"!\" : \"\";\n-\tchar *dir = (exclude->flags & EXC_FLAG_MUSTBEDIR) ? \"/\" : \"\";\n+\tchar *bang = exclude->flags & EXC_FLAG_NEGATIVE ? \"!\" : \"\";\n+\tchar *slash = exclude->flags & EXC_FLAG_MUSTBEDIR ? \"/\" : \"\";\n \tif (!null_term_line) {\n \t\tif (!verbose) {\n \t\t\twrite_name_quoted(path, stdout, '\\n');\n@@ -36,7 +36,7 @@ static void output_exclude(const char *path, struct exclude *exclude)\n \t\t\tquote_c_style(exclude->el->src, NULL, stdout, 0);\n \t\t\tprintf(\":%d:%s%s%s\\t\",\n \t\t\t exclude->srcpos,\n-\t\t\t bang, exclude->pattern, dir);\n+\t\t\t bang, exclude->pattern, slash);\n \t\t\tquote_c_style(path, NULL, stdout, 0);\n \t\t\tfputc('\\n', stdout);\n \t\t}\n@@ -47,7 +47,7 @@ static void output_exclude(const char *path, struct exclude *exclude)\n \t\t\tprintf(\"%s%c%d%c%s%s%s%c%s%c\",\n \t\t\t exclude->el->src, '\\0',\n \t\t\t exclude->srcpos, '\\0',\n-\t\t\t bang, exclude->pattern, dir, '\\0',\n+\t\t\t bang, exclude->pattern, slash, '\\0',\n \t\t\t path, '\\0');\n \t\t}\n \t}\n@@ -57,8 +57,10 @@ static int check_ignore(const char *prefix, const char **pathspec)\n {\n \tstruct dir_struct dir;\n \tconst char *path;\n-\tchar *seen = NULL;\n-\tint num_ignored = 0;\n+\tchar *seen;\n+\tint num_ignored = 0, i;\n+\tstruct path_exclude_check check;\n+\tstruct exclude *exclude;\n \n \t/* read_cache() is only necessary so we can watch out for submodules. */\n \tif (read_cache() < 0)\n@@ -68,38 +70,36 @@ static int check_ignore(const char *prefix, const char **pathspec)\n \tdir.flags |= DIR_COLLECT_IGNORED;\n \tsetup_standard_excludes(&dir);\n \n-\tif (pathspec) {\n-\t\tint i;\n-\t\tstruct path_exclude_check check;\n-\t\tstruct exclude *exclude;\n-\n-\t\tpath_exclude_check_init(&check, &dir);\n-\t\tif (!seen)\n-\t\t\tseen = find_used_pathspec(pathspec);\n-\t\tfor (i = 0; pathspec[i]; i++) {\n-\t\t\tconst char *full_path;\n-\t\t\tpath = pathspec[i];\n-\t\t\tfull_path = prefix_path(prefix, prefix\n-\t\t\t\t\t\t? strlen(prefix) : 0, path);\n-\t\t\tfull_path = treat_gitlink(full_path);\n-\t\t\tvalidate_path(full_path, prefix);\n-\t\t\tif (!seen[i] && path[0]) {\n-\t\t\t\tint dtype = DT_UNKNOWN;\n-\t\t\t\texclude = last_exclude_matching_path(&check, full_path,\n-\t\t\t\t\t\t\t\t -1, &dtype);\n-\t\t\t\tif (exclude) {\n-\t\t\t\t\tif (!quiet)\n-\t\t\t\t\t\toutput_exclude(path, exclude);\n-\t\t\t\t\tnum_ignored++;\n-\t\t\t\t}\n+\tif (!pathspec || !*pathspec) {\n+\t\tif (!quiet)\n+\t\t\tfprintf(stderr, \"no pathspec given.\\n\");\n+\t\treturn 0;\n+\t}\n+\n+\tpath_exclude_check_init(&check, &dir);\n+\tseen = find_used_pathspec(pathspec);\n+\tfor (i = 0; pathspec[i]; i++) {\n+\t\tconst char *full_path;\n+\t\tpath = pathspec[i];\n+\t\tfull_path = prefix_path(prefix, prefix\n+\t\t\t\t\t? strlen(prefix) : 0, path);\n+\t\tfull_path = treat_gitlink(full_path);\n+\t\tdie_if_path_beyond_symlink(full_path, prefix);\n+\t\tif (!seen[i] && path[0]) {\n+\t\t\tint dtype = DT_UNKNOWN;\n+\t\t\texclude = last_exclude_matching_path(&check, full_path,\n+\t\t\t\t\t\t\t -1, &dtype);\n+\t\t\tif (exclude) {\n+\t\t\t\tif (!quiet)\n+\t\t\t\t\toutput_exclude(path, exclude);\n+\t\t\t\tnum_ignored++;\n \t\t\t}\n \t\t}\n-\t\tfree(seen);\n-\t\tclear_directory(&dir);\n-\t\tpath_exclude_check_clear(&check);\n-\t} else {\n-\t\tprintf(\"no pathspec\\n\");\n \t}\n+\tfree(seen);\n+\tclear_directory(&dir);\n+\tpath_exclude_check_clear(&check);\n+\n \treturn num_ignored;\n }\n \n@@ -136,7 +136,7 @@ static int check_ignore_stdin_paths(const char *prefix)\n \n int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n {\n-\tint num_ignored = 0;\n+\tint num_ignored;\n \n \tgit_config(git_default_config, NULL);\n \n@@ -144,7 +144,7 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \t\t\t check_ignore_usage, 0);\n \n \tif (stdin_paths) {\n-\t\tif (0 < argc)\n+\t\tif (argc > 0)\n \t\t\tdie(_(\"cannot specify pathnames with --stdin\"));\n \t} else {\n \t\tif (null_term_line)\n@@ -166,5 +166,5 @@ int cmd_check_ignore(int argc, const char **argv, const char *prefix)\n \t\tmaybe_flush_or_die(stdout, \"ignore to stdout\");\n \t}\n \n-\treturn num_ignored > 0 ? 0 : 1;\n+\treturn !num_ignored;\n }\ndiff --git a/pathspec.c b/pathspec.c\nindex 6724121..3789b14 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -36,9 +36,10 @@ char *find_used_pathspec(const char **pathspec)\n }\n \n /*\n- * Check whether path refers to a submodule, or something inside a\n- * submodule. If the former, returns the path with any trailing slash\n- * stripped. If the latter, dies with an error message.\n+ * Check the index to see whether path refers to a submodule, or\n+ * something inside a submodule. If the former, returns the path with\n+ * any trailing slash stripped. If the latter, dies with an error\n+ * message.\n */\n const char *treat_gitlink(const char *path)\n {\n@@ -78,9 +79,9 @@ void treat_gitlinks(const char **pathspec)\n \n /*\n * Dies if the given path refers to a file inside a symlinked\n- * directory.\n+ * directory in the index.\n */\n-void validate_path(const char *path, const char *prefix)\n+void die_if_path_beyond_symlink(const char *path, const char *prefix)\n {\n \tif (has_symlink_leading_path(path, strlen(path))) {\n \t\tint len = prefix ? strlen(prefix) : 0;\n@@ -90,7 +91,8 @@ void validate_path(const char *path, const char *prefix)\n \n /*\n * Normalizes argv relative to prefix, via get_pathspec(), and then\n- * runs validate_path() on each path in the normalized list.\n+ * runs die_if_path_beyond_symlink() on each path in the normalized\n+ * list.\n */\n const char **validate_pathspec(const char **argv, const char *prefix)\n {\n@@ -99,7 +101,7 @@ const char **validate_pathspec(const char **argv, const char *prefix)\n \tif (pathspec) {\n \t\tconst char **p;\n \t\tfor (p = pathspec; *p; p++) {\n-\t\t\tvalidate_path(*p, prefix);\n+\t\t\tdie_if_path_beyond_symlink(*p, prefix);\n \t\t}\n \t}\n \ndiff --git a/pathspec.h b/pathspec.h\nindex c251441..1961b19 100644\n--- a/pathspec.h\n+++ b/pathspec.h\n@@ -1,6 +1,11 @@\n+#ifndef PATHSPEC_H\n+#define PATHSPEC_H\n+\n extern char *find_used_pathspec(const char **pathspec);\n extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);\n extern const char *treat_gitlink(const char *path);\n extern void treat_gitlinks(const char **pathspec);\n extern void validate_path(const char *path, const char *prefix);\n extern const char **validate_pathspec(const char **argv, const char *prefix);\n+\n+#endif /* PATHSPEC_H */\ndiff --git a/setup.c b/setup.c\nindex 03d6d5c..9570147 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -250,8 +250,12 @@ static const char *prefix_pathspec(const char *prefix, int prefixlen, const char\n }\n \n /*\n- * prefix - a path relative to the root of the working tree\n- * pathspec - a list of paths underneath the prefix path\n+ * N.B. get_pathspec() is deprecated in favor of the \"struct pathspec\"\n+ * based interface - see pathspec_magic above.\n+ *\n+ * Arguments:\n+ * - prefix - a path relative to the root of the working tree\n+ * - pathspec - a list of paths underneath the prefix path\n *\n * Iterates over pathspec, prepending each path with prefix,\n * and return the resulting list.\ndiff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh\nindex 3937ca4..8780b1e 100755\n--- a/t/t0008-ignores.sh\n+++ b/t/t0008-ignores.sh\n@@ -39,7 +39,7 @@ test_stderr () {\n \n stderr_contains () {\n \tregexp=\"$1\"\n-\tif grep -q \"$regexp\" \"$HOME/stderr\"\n+\tif grep \"$regexp\" \"$HOME/stderr\"\n \tthen\n \t\treturn 0\n \telse\n@@ -86,10 +86,10 @@ test_expect_success_multi () {\n \n \texpect=$( echo \"$expect_verbose\" | sed -e 's/.*\t//' )\n \n-\ttest_expect_success $prereq \"$testname\" \"\n-\t\texpect '$expect' &&\n-\t\t$code\n-\t\"\n+\ttest_expect_success $prereq \"$testname\" '\n+\t\texpect \"$expect\" &&\n+\t\teval \"$code\"\n+\t'\n \n \tfor quiet_opt in '-q' '--quiet'\n \tdo\n@@ -164,6 +164,15 @@ test_expect_success_multi 'empty command line' '' '\n \tstderr_contains \"fatal: no path specified\"\n '\n \n+test_expect_success_multi '--stdin with empty STDIN' '' '\n+\ttest_check_ignore \"--stdin\" 1 writes:\n\n> I've added this sentence to the top of the comments above\n> get_pathspec():\n>\n> /*\n> * N.B. get_pathspec() is deprecated in favor of the \"struct pathspec\"\n> * based interface - see pathspec_magic above.\n> *\n> [...]\n>\n> That should be sufficient to discourage people from adding new users\n> of get_pathspec().\n\nYeah, that sounds sensible.\n\nThanks, and happy new year to you ;-)\n"},{"id":"205632","messageId":"20121229033227.GA13848@gmail.com","threadId":"32464","inReplyTo":"20121229012352.GA20379@pacific.linksys.moosehall","subject":"Re: [PATCH v3 19/19] Add git-check-ignore sub-command","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2012-12-29T03:32:27Z","receivedAt":"2012-12-29T03:32:27Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Sat, Dec 29, 2012 at 01:23:52AM +0000, Adam Spiers wrote:\n> FYI, attached is the diff between check-ignore-v3 and my current\n> check-ignore, which is available at github:\n> \n> https://github.com/aspiers/git/commits/check-ignore\n\n[snipped]\n\n> diff --git a/pathspec.c b/pathspec.c\n> index 6724121..3789b14 100644\n> --- a/pathspec.c\n> +++ b/pathspec.c\n\n[snipped]\n\n> -void validate_path(const char *path, const char *prefix)\n> +void die_if_path_beyond_symlink(const char *path, const char *prefix)\n\n[snipped]\n\n> diff --git a/pathspec.h b/pathspec.h\n> index c251441..1961b19 100644\n> --- a/pathspec.h\n> +++ b/pathspec.h\n> @@ -1,6 +1,11 @@\n> +#ifndef PATHSPEC_H\n> +#define PATHSPEC_H\n> +\n> extern char *find_used_pathspec(const char **pathspec);\n> extern void fill_pathspec_matches(const char **pathspec, char *seen, int specs);\n> extern const char *treat_gitlink(const char *path);\n> extern void treat_gitlinks(const char **pathspec);\n> extern void validate_path(const char *path, const char *prefix);\n ^^^^^^^^^^^^^\nI forgot to rename this one. Will be fixed in v4.\n"},{"id":"205780","messageId":"7vobh8aans.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-3-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 02/19] Improve documentation and comments regarding directory traversal API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-01T20:52:23Z","receivedAt":"2013-01-01T20:52:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\n> index 0356d25..944fc39 100644\n> --- a/Documentation/technical/api-directory-listing.txt\n> +++ b/Documentation/technical/api-directory-listing.txt\n> @@ -9,8 +9,11 @@ Data structure\n> --------------\n> \n> `struct dir_struct` structure is used to pass directory traversal\n> -options to the library and to record the paths discovered. The notable\n> -options are:\n> +options to the library and to record the paths discovered. A single\n> +`struct dir_struct` is used regardless of whether or not the traversal\n> +recursively descends into subdirectories.\n\nI am somewhat lukewarm on this part of the change.\n\nThe added \"regardless of...\" does not seem to add as much value as\nthe two extra lines the patch spends. If we say something like:\n\n\tA `struct dir_struct` structure is used to pass options to\n traverse directories recursively, and to record all the\n paths discovered by the traversal.\n\nit might be much more direct and informative, I suspect, though.\n\nAfter all, the word \"traversal\" pretty much implies that the library\ngoes in and out of the directories recursively.\n\n> @@ -39,7 +42,7 @@ options are:\n> \tIf set, recurse into a directory that looks like a git\n> \tdirectory. Otherwise it is shown as a directory.\n> \n> -The result of the enumeration is left in these fields::\n> +The result of the enumeration is left in these fields:\n\nGood eyes.\n\n> diff --git a/dir.c b/dir.c\n> index ee8e711..89e27a6 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -2,6 +2,8 @@\n> * This handles recursive filename detection with exclude\n> * files, index knowledge etc..\n> *\n> + * See Documentation/technical/api-directory-listing.txt\n> + *\n> * Copyright (C) Linus Torvalds, 2005-2006\n> *\t\t Junio Hamano, 2005-2006\n> */\n> @@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)\n> \t\tdie(\"cannot use %s as an exclude file\", fname);\n> }\n> \n> +/*\n> + * Loads the per-directory exclude list for the substring of base\n> + * which has a char length of baselen.\n> + */\n> static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n> {\n> \tstruct exclude_list *el;\n> @@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n> \t (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))\n> \t\treturn; /* too long a path -- ignore */\n> \n> -\t/* Pop the ones that are not the prefix of the path being checked. */\n> +\t/* Pop the directories that are not the prefix of the path being checked. */\n\nThe \"one\" does not refer to a \"directory\", but to an \"exclude-list\".\n\n\tPop the ones that are not for parent directories of the path\n\tbeing checked\n\nperhaps?\n"},{"id":"205854","messageId":"CAOkDyE_DX8iAAd5ubJaQ_guPQ-PSz4-sFETZoRf7JRTrH6Qcpw@mail.gmail.com","threadId":"32464","inReplyTo":"7vobh8aans.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 02/19] Improve documentation and comments regarding directory traversal API","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-01-02T12:54:19Z","receivedAt":"2013-01-02T12:54:19Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Tue, Jan 1, 2013 at 8:52 PM, Junio C Hamano wrote:\n> Adam Spiers writes:\n>\n>> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt\n>> index 0356d25..944fc39 100644\n>> --- a/Documentation/technical/api-directory-listing.txt\n>> +++ b/Documentation/technical/api-directory-listing.txt\n>> @@ -9,8 +9,11 @@ Data structure\n>> --------------\n>>\n>> `struct dir_struct` structure is used to pass directory traversal\n>> -options to the library and to record the paths discovered. The notable\n>> -options are:\n>> +options to the library and to record the paths discovered. A single\n>> +`struct dir_struct` is used regardless of whether or not the traversal\n>> +recursively descends into subdirectories.\n>\n> I am somewhat lukewarm on this part of the change.\n>\n> The added \"regardless of...\" does not seem to add as much value as\n> the two extra lines the patch spends. If we say something like:\n>\n> A `struct dir_struct` structure is used to pass options to\n> traverse directories recursively, and to record all the\n> paths discovered by the traversal.\n>\n> it might be much more direct and informative, I suspect, though.\n\nI somewhat disagree ;) When I first encountered this code, I naturally\nassumed that one struct would be created per sub-directory traversed.\nThis is after all a natural and very common design pattern. The point\nof this hunk was to make it explicitly clear that this is *not* how it\nworks in dir.c. IMHO your rewording still contains a certain amount of\nambiguity in this regard. For example, it could mean that each\ndir_struct records all the paths discovered underneath the sub-directory\nit represents, and that these recursively bubble up to a top-level\ndir_struct.\n\n>> diff --git a/dir.c b/dir.c\n>> index ee8e711..89e27a6 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -2,6 +2,8 @@\n>> * This handles recursive filename detection with exclude\n>> * files, index knowledge etc..\n>> *\n>> + * See Documentation/technical/api-directory-listing.txt\n>> + *\n>> * Copyright (C) Linus Torvalds, 2005-2006\n>> * Junio Hamano, 2005-2006\n>> */\n>> @@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)\n>> die(\"cannot use %s as an exclude file\", fname);\n>> }\n>>\n>> +/*\n>> + * Loads the per-directory exclude list for the substring of base\n>> + * which has a char length of baselen.\n>> + */\n>> static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n>> {\n>> struct exclude_list *el;\n>> @@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n>> (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))\n>> return; /* too long a path -- ignore */\n>>\n>> - /* Pop the ones that are not the prefix of the path being checked. */\n>> + /* Pop the directories that are not the prefix of the path being checked. */\n>\n> The \"one\" does not refer to a \"directory\", but to an \"exclude-list\".\n\nNo, if that was the case, it would mean that multiple exclude lists\nwould be popped, but that is not the case here (prior to v4).\n\n> Pop the ones that are not for parent directories of the path\n> being checked\n\nBetter would be:\n\n Pop the entries within the EXCL_DIRS exclude list which originate\n from directories not in the prefix of the path being checked.\n\nalthough as previously stated, the v4 series I have been holding off\nfrom submitting (in order not to distract you from a maint release)\nactually changes this behaviour so EXCL_DIRS becomes an exclude_group of\nmultiple exclude_lists, one per directory. So in v4, multiple\nexclude_lists *will* be popped. I'll tweak the comment in v4 to make\nthis clear.\n"},{"id":"205977","messageId":"7v1ue0veww.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1356575558-2674-12-git-send-email-git@adamspiers.org","subject":"Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-04T21:03:59Z","receivedAt":"2013-01-04T21:03:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> diff --git a/builtin/clean.c b/builtin/clean.c\n> index 0c7b3d0..bd18b88 100644\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> \tif (!ignored)\n> \t\tsetup_standard_excludes(&dir);\n> \n> +\tadd_exclude_list(&dir, EXC_CMDL);\n> \tfor (i = 0; i < exclude_list.nr; i++)\n> \t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n> -\t\t\t &dir.exclude_list[EXC_CMDL]);\n> +\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0]);\n\nThis looks somewhat ugly for two reasons.\n\n * The abstraction add_exclude() offers to its callers is just to\n let them add one pattern to the list of patterns for the kind\n (here, EXC_CMDL); why should they care about .ary[0] part? Are\n there cases any sane caller (not the implementation of the\n exclude_list_group machinery e.g. add_excludes_from_... function)\n may want to call it with .ary[1]? I do not think of any.\n Shouldn't the public API function add_exclude() take a pointer to\n the list group itself?\n\n * When naming an array of things, we tend to prefer naming it\n\n type thing[count]\n\n so that the second element can be called \"thing[2]\" and not\n \"things[2]\". dir.exclude_list_group[EXC_CMDL] reads beter.\n\n> diff --git a/builtin/ls-files.c b/builtin/ls-files.c\n> index ef7f99a..c448e06 100644\n> --- a/builtin/ls-files.c\n> +++ b/builtin/ls-files.c\n> @@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,\n> static int option_parse_exclude(const struct option *opt,\n> \t\t\t\tconst char *arg, int unset)\n> {\n> -\tstruct exclude_list *list = opt->value;\n> +\tstruct exclude_list_group *group = opt->value;\n> \n> \texc_given = 1;\n> -\tadd_exclude(arg, \"\", 0, list);\n> +\tadd_exclude(arg, \"\", 0, &group->ary[0]);\n\nThis is another example where the caller would wish to be able to say\n\n\tadd_exclude(arg, \"\", 0, group);\n\ninstead.\n"},{"id":"206014","messageId":"7v623cqd39.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"7v1ue0veww.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-05T07:54:34Z","receivedAt":"2013-01-05T07:54:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Adam Spiers writes:\n>\n>> diff --git a/builtin/clean.c b/builtin/clean.c\n>> index 0c7b3d0..bd18b88 100644\n>> --- a/builtin/clean.c\n>> +++ b/builtin/clean.c\n>> @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n>> \tif (!ignored)\n>> \t\tsetup_standard_excludes(&dir);\n>> \n>> +\tadd_exclude_list(&dir, EXC_CMDL);\n>> \tfor (i = 0; i < exclude_list.nr; i++)\n>> \t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n>> -\t\t\t &dir.exclude_list[EXC_CMDL]);\n>> +\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0]);\n>\n> This looks somewhat ugly for two reasons.\n>\n> * The abstraction add_exclude() offers to its callers is just to\n> let them add one pattern to the list of patterns for the kind\n> (here, EXC_CMDL); why should they care about .ary[0] part? Are\n> there cases any sane caller (not the implementation of the\n> exclude_list_group machinery e.g. add_excludes_from_... function)\n> may want to call it with .ary[1]? I do not think of any.\n> Shouldn't the public API function add_exclude() take a pointer to\n> the list group itself?\n>\n> * When naming an array of things, we tend to prefer naming it\n>\n> type thing[count]\n>\n> so that the second element can be called \"thing[2]\" and not\n> \"things[2]\". dir.exclude_list_group[EXC_CMDL] reads better.\n\nAlso, \"ary[]\" is a bad name, even as an implementation detail, for\ntwo reasons: it is naming it after its type (being an \"array\") not\nafter what it is (if it holds the patterns from the same information\nsource, e.g. file, togeter, \"src\" might be a better name), and it\nuses rather unusual abbreviation (I haven't seen \"array\" shortened\nto \"ary\" anywhere else).\n\n>\n>> diff --git a/builtin/ls-files.c b/builtin/ls-files.c\n>> index ef7f99a..c448e06 100644\n>> --- a/builtin/ls-files.c\n>> +++ b/builtin/ls-files.c\n>> @@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,\n>> static int option_parse_exclude(const struct option *opt,\n>> \t\t\t\tconst char *arg, int unset)\n>> {\n>> -\tstruct exclude_list *list = opt->value;\n>> +\tstruct exclude_list_group *group = opt->value;\n>> \n>> \texc_given = 1;\n>> -\tadd_exclude(arg, \"\", 0, list);\n>> +\tadd_exclude(arg, \"\", 0, &group->ary[0]);\n>\n> This is another example where the caller would wish to be able to say\n>\n> \tadd_exclude(arg, \"\", 0, group);\n>\n> instead.\n"},{"id":"206129","messageId":"20130106120201.GH28089@pacific.linksys.moosehall","threadId":"32464","inReplyTo":"CAOkDyE_DX8iAAd5ubJaQ_guPQ-PSz4-sFETZoRf7JRTrH6Qcpw@mail.gmail.com","subject":"Re: [PATCH v3 02/19] Improve documentation and comments regarding directory traversal API","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-01-06T12:02:01Z","receivedAt":"2013-01-06T12:02:01Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Wed, Jan 02, 2013 at 12:54:19PM +0000, Adam Spiers wrote:\n> On Tue, Jan 1, 2013 at 8:52 PM, Junio C Hamano wrote:\n> > Adam Spiers writes:\n> >> diff --git a/dir.c b/dir.c\n> >> index ee8e711..89e27a6 100644\n> >> --- a/dir.c\n> >> +++ b/dir.c\n> >> @@ -2,6 +2,8 @@\n> >> * This handles recursive filename detection with exclude\n> >> * files, index knowledge etc..\n> >> *\n> >> + * See Documentation/technical/api-directory-listing.txt\n> >> + *\n> >> * Copyright (C) Linus Torvalds, 2005-2006\n> >> * Junio Hamano, 2005-2006\n> >> */\n> >> @@ -476,6 +478,10 @@ void add_excludes_from_file(struct dir_struct *dir, const char *fname)\n> >> die(\"cannot use %s as an exclude file\", fname);\n> >> }\n> >>\n> >> +/*\n> >> + * Loads the per-directory exclude list for the substring of base\n> >> + * which has a char length of baselen.\n> >> + */\n> >> static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n> >> {\n> >> struct exclude_list *el;\n> >> @@ -486,7 +492,7 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen)\n> >> (baselen + strlen(dir->exclude_per_dir) >= PATH_MAX))\n> >> return; /* too long a path -- ignore */\n> >>\n> >> - /* Pop the ones that are not the prefix of the path being checked. */\n> >> + /* Pop the directories that are not the prefix of the path being checked. */\n> >\n> > The \"one\" does not refer to a \"directory\", but to an \"exclude-list\".\n> \n> No, if that was the case, it would mean that multiple exclude lists\n> would be popped, but that is not the case here (prior to v4).\n\nSorry, I meant prior to v3 11/19.\n\n> > Pop the ones that are not for parent directories of the path\n> > being checked\n> \n> Better would be:\n> \n> Pop the entries within the EXCL_DIRS exclude list which originate\n> from directories not in the prefix of the path being checked.\n> \n> although as previously stated, the v4 series I have been holding off\n> from submitting (in order not to distract you from a maint release)\n> actually changes this behaviour so EXCL_DIRS becomes an exclude_group of\n> multiple exclude_lists, one per directory. So in v4, multiple\n> exclude_lists *will* be popped. I'll tweak the comment in v4 to make\n> this clear.\n\nAgain, I got confused and forgot that I already included the switch to\nexclude_list_groups as v3 11/19. But since the patch being discussed\nhere is v3 02/19 which precedes it, everything I said still applies.\n"},{"id":"206139","messageId":"20130106152039.GA2396@pacific.linksys.moosehall","threadId":"32464","inReplyTo":"7v1ue0veww.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-01-06T15:20:39Z","receivedAt":"2013-01-06T15:20:39Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Jan 04, 2013 at 01:03:59PM -0800, Junio C Hamano wrote:\n> Adam Spiers writes:\n> \n> > diff --git a/builtin/clean.c b/builtin/clean.c\n> > index 0c7b3d0..bd18b88 100644\n> > --- a/builtin/clean.c\n> > +++ b/builtin/clean.c\n> > @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> > \tif (!ignored)\n> > \t\tsetup_standard_excludes(&dir);\n> > \n> > +\tadd_exclude_list(&dir, EXC_CMDL);\n> > \tfor (i = 0; i < exclude_list.nr; i++)\n> > \t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n> > -\t\t\t &dir.exclude_list[EXC_CMDL]);\n> > +\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0]);\n> \n> This looks somewhat ugly for two reasons.\n> \n> * The abstraction add_exclude() offers to its callers is just to\n> let them add one pattern to the list of patterns for the kind\n> (here, EXC_CMDL); why should they care about .ary[0] part?\n\nBecause the caller has to decide which exclude list the new exclude\nshould be added to; that is how it has been for a long time, and I am\nnot proposing we change that.\n\nThere are currently three callers:\n\n builtin/clean.c: cmd_clean()\n builtin/ls-files.c: option_parse_exclude()\n dir.c: add_excludes_from_file_to_list()\n\nThe first two add to EXC_CMDL, but the latter could be adding to\nnumerous different possible lists, e.g.\n\n - .git/info/exclude (EXC_FILE)\n - core.excludesfile (EXC_FILE)\n - any of the per-directory .gitignore lists (EXC_DIRS)\n\n> Are\n> there cases any sane caller (not the implementation of the\n> exclude_list_group machinery e.g. add_excludes_from_... function)\n> may want to call it with .ary[1]?\n\nEffectively yes, although it is not written like \".ary[1]\". For\nexample prep_exclude() calls add_excludes_from_file_to_list() for each\nnew .gitignore file\n\n> Shouldn't the public API function add_exclude() take a pointer to\n> the list group itself?\n\nTypically EXC_DIRS and EXC_FILE will contain excludes from multiple\nsources, whereas EXC_CMDL will contain excludes from a single source.\n\nTherefore when transitioning to exclude_list_groups, I had to make a\nchoice whether to keep EXC_CMDL as a singleton list, or split it out\ninto a separate field in struct dir_struct, e.g.:\n\n\tstruct exclude_list exclude_list_cmdl;\n\tstruct exclude_list exclude_list[2];\n#define EXC_DIRS 0\n#define EXC_FILE 1\n\nI decided it was cleaner to use a singleton list, because this\npreserved the design that excludes in earlier members of\nexclude_list[3] take priority over excludes in later members of\nexclude_list[3]. That way, this loop in last_exclude_matching():\n\n\tfor (i = EXC_CMDL; i <= EXC_FILE; i++) {\n\nstill makes sense.\n\n> * When naming an array of things, we tend to prefer naming it\n> \n> type thing[count]\n> \n> so that the second element can be called \"thing[2]\" and not\n> \"things[2]\". dir.exclude_list_group[EXC_CMDL] reads beter.\n\nOK, I will change that.\n\n> > diff --git a/builtin/ls-files.c b/builtin/ls-files.c\n> > index ef7f99a..c448e06 100644\n> > --- a/builtin/ls-files.c\n> > +++ b/builtin/ls-files.c\n> > @@ -420,10 +420,10 @@ static int option_parse_z(const struct option *opt,\n> > static int option_parse_exclude(const struct option *opt,\n> > \t\t\t\tconst char *arg, int unset)\n> > {\n> > -\tstruct exclude_list *list = opt->value;\n> > +\tstruct exclude_list_group *group = opt->value;\n> > \n> > \texc_given = 1;\n> > -\tadd_exclude(arg, \"\", 0, list);\n> > +\tadd_exclude(arg, \"\", 0, &group->ary[0]);\n> \n> This is another example where the caller would wish to be able to say\n> \n> \tadd_exclude(arg, \"\", 0, group);\n> \n> instead.\n\nWhy? The caller needs to decide which exclude list the exclude should\ngo on, because that determines matching priority. Specifying an\nexclude_list_group is insufficient information to determine that.\n"},{"id":"206141","messageId":"20130106152716.GB2396@pacific.linksys.moosehall","threadId":"32464","inReplyTo":"7v623cqd39.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-01-06T15:27:16Z","receivedAt":"2013-01-06T15:27:16Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Fri, Jan 04, 2013 at 11:54:34PM -0800, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> > Adam Spiers writes:\n> >\n> >> diff --git a/builtin/clean.c b/builtin/clean.c\n> >> index 0c7b3d0..bd18b88 100644\n> >> --- a/builtin/clean.c\n> >> +++ b/builtin/clean.c\n> >> @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> >> \tif (!ignored)\n> >> \t\tsetup_standard_excludes(&dir);\n> >> \n> >> +\tadd_exclude_list(&dir, EXC_CMDL);\n> >> \tfor (i = 0; i < exclude_list.nr; i++)\n> >> \t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n> >> -\t\t\t &dir.exclude_list[EXC_CMDL]);\n> >> +\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0]);\n> >\n> > This looks somewhat ugly for two reasons.\n> >\n> > * The abstraction add_exclude() offers to its callers is just to\n> > let them add one pattern to the list of patterns for the kind\n> > (here, EXC_CMDL); why should they care about .ary[0] part? Are\n> > there cases any sane caller (not the implementation of the\n> > exclude_list_group machinery e.g. add_excludes_from_... function)\n> > may want to call it with .ary[1]? I do not think of any.\n> > Shouldn't the public API function add_exclude() take a pointer to\n> > the list group itself?\n> >\n> > * When naming an array of things, we tend to prefer naming it\n> >\n> > type thing[count]\n> >\n> > so that the second element can be called \"thing[2]\" and not\n> > \"things[2]\". dir.exclude_list_group[EXC_CMDL] reads better.\n> \n> Also, \"ary[]\" is a bad name, even as an implementation detail, for\n> two reasons: it is naming it after its type (being an \"array\") not\n> after what it is (if it holds the patterns from the same information\n> source, e.g. file, togeter, \"src\" might be a better name), and it\n> uses rather unusual abbreviation (I haven't seen \"array\" shortened\n> to \"ary\" anywhere else).\n\nOK, well in that case Documentation/technical/api-allocation-growing.txt\nneeds to be fixed, because I copied it from that. I would never normally\nshorten \"array\" to \"ary\" either, but I did it in an attempt to conform\nto the stated guidelines.\n"},{"id":"206142","messageId":"1357486505-21357-1-git-send-email-git@adamspiers.org","threadId":"32464","inReplyTo":"20130106152716.GB2396@pacific.linksys.moosehall","subject":"[PATCH] api-allocation-growing.txt: encourage better variable naming","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-01-06T15:35:05Z","receivedAt":"2013-01-06T15:35:05Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"The documentation for the ALLOC_GROW API implicitly encouraged\ndevelopers to use \"ary\" as the variable name for the array which is\ndynamically grown. However \"ary\" is an unusual abbreviation hardly\nused anywhere else in the source tree, and it is also better to name\nvariables based on their contents not on their type.\n\nSigned-off-by: Adam Spiers \n---\n Documentation/technical/api-allocation-growing.txt | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/technical/api-allocation-growing.txt b/Documentation/technical/api-allocation-growing.txt\nindex 43dbe09..3894815 100644\n--- a/Documentation/technical/api-allocation-growing.txt\n+++ b/Documentation/technical/api-allocation-growing.txt\n@@ -5,7 +5,9 @@ Dynamically growing an array using realloc() is error prone and boring.\n \n Define your array with:\n \n-* a pointer (`ary`) that points at the array, initialized to `NULL`;\n+* a pointer (`array`) that points at the array, initialized to `NULL`\n+ (although please name the variable based on its contents, not on its\n+ type);\n \n * an integer variable (`alloc`) that keeps track of how big the current\n allocation is, initialized to `0`;\n@@ -13,22 +15,22 @@ Define your array with:\n * another integer variable (`nr`) to keep track of how many elements the\n array currently has, initialized to `0`.\n \n-Then before adding `n`th element to the array, call `ALLOC_GROW(ary, n,\n+Then before adding `n`th element to the array, call `ALLOC_GROW(array, n,\n alloc)`. This ensures that the array can hold at least `n` elements by\n calling `realloc(3)` and adjusting `alloc` variable.\n \n ------------\n-sometype *ary;\n+sometype *array;\n size_t nr;\n size_t alloc\n \n for (i = 0; i < nr; i++)\n-\tif (we like ary[i] already)\n+\tif (we like array[i] already)\n \t\treturn;\n \n /* we did not like any existing one, so add one */\n-ALLOC_GROW(ary, nr + 1, alloc);\n-ary[nr++] = value you like;\n+ALLOC_GROW(array, nr + 1, alloc);\n+array[nr++] = value you like;\n ------------\n \n You are responsible for updating the `nr` variable.\n-- \n1.7.11.7.33.gb8feba5\n"},{"id":"206168","messageId":"7v7gnqnjn7.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"20130106152039.GA2396@pacific.linksys.moosehall","subject":"Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-06T20:25:48Z","receivedAt":"2013-01-06T20:25:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> On Fri, Jan 04, 2013 at 01:03:59PM -0800, Junio C Hamano wrote:\n>> Adam Spiers writes:\n>> \n>> > diff --git a/builtin/clean.c b/builtin/clean.c\n>> > index 0c7b3d0..bd18b88 100644\n>> > --- a/builtin/clean.c\n>> > +++ b/builtin/clean.c\n>> > @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n>> > \tif (!ignored)\n>> > \t\tsetup_standard_excludes(&dir);\n>> > \n>> > +\tadd_exclude_list(&dir, EXC_CMDL);\n>> > \tfor (i = 0; i < exclude_list.nr; i++)\n>> > \t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n>> > -\t\t\t &dir.exclude_list[EXC_CMDL]);\n>> > +\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0]);\n>> \n>> This looks somewhat ugly for two reasons.\n>> \n>> * The abstraction add_exclude() offers to its callers is just to\n>> let them add one pattern to the list of patterns for the kind\n>> (here, EXC_CMDL); why should they care about .ary[0] part?\n>\n> Because the caller has to decide which exclude list the new exclude\n> should be added to; that is how it has been for a long time, and I am\n> not proposing we change that.\n\nUnless I was mistaken, I never objected to the EXC_CMDL, etc\nappearing in the text of the calling site of add_exclude().\n\nThe objection was about the .ary[0] bit. From the point of view of\na caller of the API, it:\n\n - calls add_exclude_list() to declare \"I now start adding new\n patterns that come from a new source of patterns\"; then\n\n - calls add_exclude() repeatedly to add the patterns that come\n from that source.\n\nno? Why does the latter has to keep repeating \"Here is the new\npattern for the EXC_CMDL group; it comes from the latest source I\nearlier declared, by the way\", instead of just \"Here is the new\npattern for the EXC_CMDL group\"? The ary[0] part always using \"0\"\n(not \"4\" or \"ix\") is what repeats that \"by the way\".\n\n>> Are\n>> there cases any sane caller (not the implementation of the\n>> exclude_list_group machinery e.g. add_excludes_from_... function)\n>> may want to call it with .ary[1]?\n>\n> Effectively yes, although it is not written like \".ary[1]\". For\n> example prep_exclude() calls add_excludes_from_file_to_list() for each\n> new .gitignore file\n\nThat is part of the \"implementation of the machinery\". If the API\nfor the outside callers are to call add_exclude_list() to declare\nthat patterns added by subsequent calls to add_exclude() are from\none new source of the patterns (e.g. .gitignore file in a new\ndirectory level), and then call add_exclude() to add each pattern,\nthen the callers to add_exclude() shouldn't have to care about the\nimplementation detail that individual sources in exclude_list_group\nis implemented as an array in that sructure, and the latest ones\nshould go to its ->array[0].\n\nThe implementation of the machinery may find it more convenient if\nthey can add one or more \"sources\" to an exclude_list_group before\nstarting to add patterns to ->array[0] or ->array[1] or ->array[2],\nand a finer grained internal API that lets the caller pass an\ninstance of \"struct exclude_list\" regardless of where in an\nexclude_list_group's ary[] that instance sits may be necessary to do\nso.\n\nBut that does not mean other existing callers has to be aware of\nsuch inner detail. If the implementation of the machinery needs a\nhelper function that adds an element to any struct exclude_list, not\nnecessarily the one at the tip of an exclude_list_group, we can\nstill do that by having the bulk of the logic in the internal, finer\ngrained helper, say, add_pattern_to_exclude_list(), and keep the\nexternal API simpler by making it a thin wrapper around it, perhaps\nlike:\n\n static void add_pattern_to_exclude_list(const char *pattern,\n \t\t const char *base, int baselen,\n struct exclude_list *el);\n\n void add_exclude(const char *pattern,\n \t\t const char *base, int baselen,\n struct exclude_list_group *group) {\n\tadd_pattern_to_exclude_list(pattern, base, baselen, &group->ary[0]);\n } \n\nno?\n"},{"id":"206169","messageId":"7v38yenjgy.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"1357486505-21357-1-git-send-email-git@adamspiers.org","subject":"Re: [PATCH] api-allocation-growing.txt: encourage better variable naming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-06T20:29:33Z","receivedAt":"2013-01-06T20:29:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> The documentation for the ALLOC_GROW API implicitly encouraged\n> developers to use \"ary\" as the variable name for the array which is\n> dynamically grown. However \"ary\" is an unusual abbreviation hardly\n> used anywhere else in the source tree, and it is also better to name\n> variables based on their contents not on their type.\n\nSounds good. To follow \"not type but contents\", a further rewrite\nwith s/array/item/ is even better, no?\n\nI can obviously squash it in without resending, if you agree, or you\ncan point out why item[] is not a good idea and array[] is better.\n\n>\n> Signed-off-by: Adam Spiers \n> ---\n> Documentation/technical/api-allocation-growing.txt | 14 ++++++++------\n> 1 file changed, 8 insertions(+), 6 deletions(-)\n>\n> diff --git a/Documentation/technical/api-allocation-growing.txt b/Documentation/technical/api-allocation-growing.txt\n> index 43dbe09..3894815 100644\n> --- a/Documentation/technical/api-allocation-growing.txt\n> +++ b/Documentation/technical/api-allocation-growing.txt\n> @@ -5,7 +5,9 @@ Dynamically growing an array using realloc() is error prone and boring.\n> \n> Define your array with:\n> \n> -* a pointer (`ary`) that points at the array, initialized to `NULL`;\n> +* a pointer (`array`) that points at the array, initialized to `NULL`\n> + (although please name the variable based on its contents, not on its\n> + type);\n> \n> * an integer variable (`alloc`) that keeps track of how big the current\n> allocation is, initialized to `0`;\n> @@ -13,22 +15,22 @@ Define your array with:\n> * another integer variable (`nr`) to keep track of how many elements the\n> array currently has, initialized to `0`.\n> \n> -Then before adding `n`th element to the array, call `ALLOC_GROW(ary, n,\n> +Then before adding `n`th element to the array, call `ALLOC_GROW(array, n,\n> alloc)`. This ensures that the array can hold at least `n` elements by\n> calling `realloc(3)` and adjusting `alloc` variable.\n> \n> ------------\n> -sometype *ary;\n> +sometype *array;\n> size_t nr;\n> size_t alloc\n> \n> for (i = 0; i < nr; i++)\n> -\tif (we like ary[i] already)\n> +\tif (we like array[i] already)\n> \t\treturn;\n> \n> /* we did not like any existing one, so add one */\n> -ALLOC_GROW(ary, nr + 1, alloc);\n> -ary[nr++] = value you like;\n> +ALLOC_GROW(array, nr + 1, alloc);\n> +array[nr++] = value you like;\n> ------------\n> \n> You are responsible for updating the `nr` variable.\n"},{"id":"206172","messageId":"20130106205207.GA6552@pacific.linksys.moosehall","threadId":"32464","inReplyTo":"7v38yenjgy.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] api-allocation-growing.txt: encourage better variable naming","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-01-06T20:52:08Z","receivedAt":"2013-01-06T20:52:08Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Sun, Jan 06, 2013 at 12:29:33PM -0800, Junio C Hamano wrote:\n> Adam Spiers writes:\n> \n> > The documentation for the ALLOC_GROW API implicitly encouraged\n> > developers to use \"ary\" as the variable name for the array which is\n> > dynamically grown. However \"ary\" is an unusual abbreviation hardly\n> > used anywhere else in the source tree, and it is also better to name\n> > variables based on their contents not on their type.\n> \n> Sounds good. To follow \"not type but contents\", a further rewrite\n> with s/array/item/ is even better, no?\n> \n> I can obviously squash it in without resending, if you agree, or you\n> can point out why item[] is not a good idea and array[] is better.\n\nI agree.\n"},{"id":"206173","messageId":"7vy5g6m3jl.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"20130106205207.GA6552@pacific.linksys.moosehall","subject":"Re: [PATCH] api-allocation-growing.txt: encourage better variable naming","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-06T20:58:54Z","receivedAt":"2013-01-06T20:58:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n>> Sounds good. To follow \"not type but contents\", a further rewrite\n>> with s/array/item/ is even better, no?\n>\n> I agree.\n\nThanks for a quick response; let's do this then.\n\n-- >8 --\nFrom: Adam Spiers \n\nThe documentation for the ALLOC_GROW API implicitly encouraged\ndevelopers to use \"ary\" as the variable name for the array which is\ndynamically grown. However \"ary\" is an unusual abbreviation hardly\nused anywhere else in the source tree, and it is also better to name\nvariables based on their contents not on their type.\n\nSigned-off-by: Adam Spiers \nSigned-off-by: Junio C Hamano \n---\n Documentation/technical/api-allocation-growing.txt | 14 ++++++++------\n 1 file changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/technical/api-allocation-growing.txt b/Documentation/technical/api-allocation-growing.txt\nindex 43dbe09..542946b 100644\n--- a/Documentation/technical/api-allocation-growing.txt\n+++ b/Documentation/technical/api-allocation-growing.txt\n@@ -5,7 +5,9 @@ Dynamically growing an array using realloc() is error prone and boring.\n \n Define your array with:\n \n-* a pointer (`ary`) that points at the array, initialized to `NULL`;\n+* a pointer (`item`) that points at the array, initialized to `NULL`\n+ (although please name the variable based on its contents, not on its\n+ type);\n \n * an integer variable (`alloc`) that keeps track of how big the current\n allocation is, initialized to `0`;\n@@ -13,22 +15,22 @@ Define your array with:\n * another integer variable (`nr`) to keep track of how many elements the\n array currently has, initialized to `0`.\n \n-Then before adding `n`th element to the array, call `ALLOC_GROW(ary, n,\n+Then before adding `n`th element to the item, call `ALLOC_GROW(item, n,\n alloc)`. This ensures that the array can hold at least `n` elements by\n calling `realloc(3)` and adjusting `alloc` variable.\n \n ------------\n-sometype *ary;\n+sometype *item;\n size_t nr;\n size_t alloc\n \n for (i = 0; i < nr; i++)\n-\tif (we like ary[i] already)\n+\tif (we like item[i] already)\n \t\treturn;\n \n /* we did not like any existing one, so add one */\n-ALLOC_GROW(ary, nr + 1, alloc);\n-ary[nr++] = value you like;\n+ALLOC_GROW(item, nr + 1, alloc);\n+item[nr++] = value you like;\n ------------\n \n You are responsible for updating the `nr` variable.\n-- \n1.8.1.302.g0f4eaa7\n"},{"id":"206184","messageId":"20130106225311.GB6552@pacific.linksys.moosehall","threadId":"32464","inReplyTo":"7v7gnqnjn7.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-01-06T22:53:11Z","receivedAt":"2013-01-06T22:53:11Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Sun, Jan 06, 2013 at 12:25:48PM -0800, Junio C Hamano wrote:\n> Adam Spiers writes:\n> \n> > On Fri, Jan 04, 2013 at 01:03:59PM -0800, Junio C Hamano wrote:\n> >> Adam Spiers writes:\n> >> \n> >> > diff --git a/builtin/clean.c b/builtin/clean.c\n> >> > index 0c7b3d0..bd18b88 100644\n> >> > --- a/builtin/clean.c\n> >> > +++ b/builtin/clean.c\n> >> > @@ -97,9 +97,10 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> >> > \tif (!ignored)\n> >> > \t\tsetup_standard_excludes(&dir);\n> >> > \n> >> > +\tadd_exclude_list(&dir, EXC_CMDL);\n> >> > \tfor (i = 0; i < exclude_list.nr; i++)\n> >> > \t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n> >> > -\t\t\t &dir.exclude_list[EXC_CMDL]);\n> >> > +\t\t\t &dir.exclude_list_groups[EXC_CMDL].ary[0]);\n> >> \n> >> This looks somewhat ugly for two reasons.\n> >> \n> >> * The abstraction add_exclude() offers to its callers is just to\n> >> let them add one pattern to the list of patterns for the kind\n> >> (here, EXC_CMDL); why should they care about .ary[0] part?\n> >\n> > Because the caller has to decide which exclude list the new exclude\n> > should be added to; that is how it has been for a long time, and I am\n> > not proposing we change that.\n> \n> Unless I was mistaken, I never objected to the EXC_CMDL, etc\n> appearing in the text of the calling site of add_exclude().\n> \n> The objection was about the .ary[0] bit. From the point of view of\n> a caller of the API, it:\n> \n> - calls add_exclude_list() to declare \"I now start adding new\n> patterns that come from a new source of patterns\"; then\n> \n> - calls add_exclude() repeatedly to add the patterns that come\n> from that source.\n> \n> no?\n\nCorrect.\n\n> Why does the latter has to keep repeating \"Here is the new\n> pattern for the EXC_CMDL group; it comes from the latest source I\n> earlier declared, by the way\", instead of just \"Here is the new\n> pattern for the EXC_CMDL group\"?\n\nMainly because there is no guarantee that such a group exists.\n\nunpack_trees() has:\n\n\tif (add_excludes_from_file_to_list(git_path(\"info/sparse-checkout\"), \"\", 0, &el, 0) < 0)\n\nso if you change the signature of add_exclude() to require an\nexclude_list_group, then there is no way to implement\nadd_excludes_from_file_to_list().\n\nEven if you could, you still haven't reduced the number of parameters\nadd_exclude() requires, so I'm dubious of the benefits of this\n\"simplification\".\n\n> >> Are\n> >> there cases any sane caller (not the implementation of the\n> >> exclude_list_group machinery e.g. add_excludes_from_... function)\n> >> may want to call it with .ary[1]?\n> >\n> > Effectively yes, although it is not written like \".ary[1]\". For\n> > example prep_exclude() calls add_excludes_from_file_to_list() for each\n> > new .gitignore file\n> \n> That is part of the \"implementation of the machinery\". If the API\n> for the outside callers are to call add_exclude_list() to declare\n> that patterns added by subsequent calls to add_exclude() are from\n> one new source of the patterns (e.g. .gitignore file in a new\n> directory level), and then call add_exclude() to add each pattern,\n> then the callers to add_exclude() shouldn't have to care about the\n> implementation detail that individual sources in exclude_list_group\n> is implemented as an array in that sructure, and the latest ones\n> should go to its ->array[0].\n\nThat's a valid point. However, the ary[0] part which assumes external\nknowledge of the internal implementation can trivially be avoided by\nsquashing this patch onto the commit we are discussing:\n\ndiff --git a/builtin/clean.c b/builtin/clean.c\nindex dd89737..6e21ca6 100644\n--- a/builtin/clean.c\n+++ b/builtin/clean.c\n@@ -45,6 +45,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tstatic const char **pathspec;\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n+\tstruct exclude_list *el;\n \tconst char *qname;\n \tchar *seen = NULL;\n \tstruct option options[] = {\n@@ -97,10 +98,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n \tif (!ignored)\n \t\tsetup_standard_excludes(&dir);\n \n-\tadd_exclude_list(&dir, EXC_CMDL);\n+\tel = add_exclude_list(&dir, EXC_CMDL);\n \tfor (i = 0; i < exclude_list.nr; i++)\n-\t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n-\t\t\t &dir.exclude_list_group[EXC_CMDL].el[0]);\n+\t\tadd_exclude(exclude_list.items[i].string, \"\", 0, el);\n \n \tpathspec = get_pathspec(prefix, argv);\n\n\nand by adopting the same approach for ls-files.c:\n\n \ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 0ca9d8e..0406adc 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -420,10 +420,11 @@ static int option_parse_z(const struct option *opt,\n static int option_parse_exclude(const struct option *opt,\n \t\t\t\tconst char *arg, int unset)\n {\n-\tstruct exclude_list_group *group = opt->value;\n+\tstruct string_list *exclude_list = opt->value;\n \n \texc_given = 1;\n-\tadd_exclude(arg, \"\", 0, &group->el[0]);\n+\tstring_list_append(exclude_list, arg);\n+\tfprintf(stderr, \"append %s\\n\", arg);\n \n \treturn 0;\n }\n@@ -452,9 +453,11 @@ static int option_parse_exclude_standard(const struct option *opt,\n \n int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n {\n-\tint require_work_tree = 0, show_tag = 0;\n+\tint require_work_tree = 0, show_tag = 0, i;\n \tconst char *max_prefix;\n \tstruct dir_struct dir;\n+\tstruct exclude_list *el;\n+\tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n \tstruct option builtin_ls_files_options[] = {\n \t\t{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,\n \t\t\t\"paths are separated with NUL character\",\n@@ -489,7 +492,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t\tOPT_BOOLEAN(0, \"resolve-undo\", &show_resolve_undo,\n \t\t\t \"show resolve-undo information\"),\n \t\t{ OPTION_CALLBACK, 'x', \"exclude\",\n-\t\t\t&dir.exclude_list_group[EXC_CMDL], \"pattern\",\n+\t\t\t&exclude_list, \"pattern\",\n \t\t\t\"skip files matching pattern\",\n \t\t\t0, option_parse_exclude },\n \t\t{ OPTION_CALLBACK, 'X', \"exclude-from\", &dir, \"file\",\n@@ -524,9 +527,13 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \tif (read_cache() < 0)\n \t\tdie(\"index file corrupt\");\n \n-\tadd_exclude_list(&dir, EXC_CMDL);\n \targc = parse_options(argc, argv, prefix, builtin_ls_files_options,\n \t\t\tls_files_usage, 0);\n+\tel = add_exclude_list(&dir, EXC_CMDL);\n+\tfor (i = 0; i < exclude_list.nr; i++) {\n+\t\tfprintf(stderr, \"adding exclude: %s\\n\", exclude_list.items[i].string);\n+\t\tadd_exclude(exclude_list.items[i].string, \"\", 0, el);\n+\t}\n \tif (show_tag || show_valid_bit) {\n \t\ttag_cached = \"H \";\n \t\ttag_unmerged = \"M \";\n\n> The implementation of the machinery may find it more convenient if\n> they can add one or more \"sources\" to an exclude_list_group before\n> starting to add patterns to ->array[0] or ->array[1] or ->array[2],\n> and a finer grained internal API that lets the caller pass an\n> instance of \"struct exclude_list\" regardless of where in an\n> exclude_list_group's ary[] that instance sits may be necessary to do\n> so.\n> \n> But that does not mean other existing callers has to be aware of\n> such inner detail. If the implementation of the machinery needs a\n> helper function that adds an element to any struct exclude_list, not\n> necessarily the one at the tip of an exclude_list_group, we can\n> still do that by having the bulk of the logic in the internal, finer\n> grained helper, say, add_pattern_to_exclude_list(), and keep the\n> external API simpler by making it a thin wrapper around it, perhaps\n> like:\n> \n> static void add_pattern_to_exclude_list(const char *pattern,\n> \t\t const char *base, int baselen,\n> struct exclude_list *el);\n> \n> void add_exclude(const char *pattern,\n> \t\t const char *base, int baselen,\n> struct exclude_list_group *group) {\n> \tadd_pattern_to_exclude_list(pattern, base, baselen, &group->ary[0]);\n\nPresumably you mean\n\n\tadd_pattern_to_exclude_list(pattern, base, baselen,\n\t\t\t\t &group->ary[group->nr - 1]);\n\n(although at your request, I already renamed 'ary' to 'el').\n\nI have made a genuine attempt to implement your suggestion, but due to\nthe unpack_trees() case stated above, I don't see how it can be done.\n"},{"id":"206188","messageId":"20130106231757.GA19655@pacific.linksys.moosehall","threadId":"32464","inReplyTo":"20130106225311.GB6552@pacific.linksys.moosehall","subject":"Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Adam Spiers","fromEmail":"git@adamspiers.org","sentAt":"2013-01-06T23:17:57Z","receivedAt":"2013-01-06T23:17:57Z","isPatch":true,"sender":{"key":"git@adamspiers.org","avatar":"https://avatars.githubusercontent.com/u/100738?v=4"},"body":"On Sun, Jan 06, 2013 at 10:53:11PM +0000, Adam Spiers wrote:\n> That's a valid point. However, the ary[0] part which assumes external\n> knowledge of the internal implementation can trivially be avoided by\n> squashing this patch onto the commit we are discussing:\n\n[snipped]\n\n> diff --git a/builtin/ls-files.c b/builtin/ls-files.c\n> index 0ca9d8e..0406adc 100644\n> --- a/builtin/ls-files.c\n> +++ b/builtin/ls-files.c\n> @@ -420,10 +420,11 @@ static int option_parse_z(const struct option *opt,\n> static int option_parse_exclude(const struct option *opt,\n> \t\t\t\tconst char *arg, int unset)\n> {\n> -\tstruct exclude_list_group *group = opt->value;\n> +\tstruct string_list *exclude_list = opt->value;\n> \n> \texc_given = 1;\n> -\tadd_exclude(arg, \"\", 0, &group->el[0]);\n> +\tstring_list_append(exclude_list, arg);\n> +\tfprintf(stderr, \"append %s\\n\", arg);\n\nWhoops :-)\n\n[snipped]\n\n> @@ -524,9 +527,13 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n> \tif (read_cache() < 0)\n> \t\tdie(\"index file corrupt\");\n> \n> -\tadd_exclude_list(&dir, EXC_CMDL);\n> \targc = parse_options(argc, argv, prefix, builtin_ls_files_options,\n> \t\t\tls_files_usage, 0);\n> +\tel = add_exclude_list(&dir, EXC_CMDL);\n> +\tfor (i = 0; i < exclude_list.nr; i++) {\n> +\t\tfprintf(stderr, \"adding exclude: %s\\n\", exclude_list.items[i].string);\n\nExcluding those two fprintf() calls, of course :-)\n\nI've removed them, and pushed to my github fork a new version of v4\nwith the fixed version of this patch inserted in the appropriate place\n(and labelled with a \"[SQUASH]\" prefix):\n\n git://github.com/aspiers/git.git\n https://github.com/aspiers/git/commits/check-ignore\n\nSince I sent v4 earlier today, to avoid spamming this list, I won't\nresend the whole series yet - not until we have made some progress in\nreviewing v4.\n"},{"id":"206189","messageId":"7v1udxnbm4.fsf@alter.siamese.dyndns.org","threadId":"32464","inReplyTo":"20130106225311.GB6552@pacific.linksys.moosehall","subject":"Re: [PATCH v3 11/19] dir.c: use a single struct exclude_list per source of excludes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-06T23:19:15Z","receivedAt":"2013-01-06T23:19:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Adam Spiers writes:\n\n> That's a valid point. However, the ary[0] part which assumes external\n> knowledge of the internal implementation can trivially be avoided by\n> squashing this patch onto the commit we are discussing:\n>\n> diff --git a/builtin/clean.c b/builtin/clean.c\n> index dd89737..6e21ca6 100644\n> --- a/builtin/clean.c\n> +++ b/builtin/clean.c\n> @@ -45,6 +45,7 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> \tstatic const char **pathspec;\n> \tstruct strbuf buf = STRBUF_INIT;\n> \tstruct string_list exclude_list = STRING_LIST_INIT_NODUP;\n> +\tstruct exclude_list *el;\n> \tconst char *qname;\n> \tchar *seen = NULL;\n> \tstruct option options[] = {\n> @@ -97,10 +98,9 @@ int cmd_clean(int argc, const char **argv, const char *prefix)\n> \tif (!ignored)\n> \t\tsetup_standard_excludes(&dir);\n> \n> -\tadd_exclude_list(&dir, EXC_CMDL);\n> +\tel = add_exclude_list(&dir, EXC_CMDL);\n> \tfor (i = 0; i < exclude_list.nr; i++)\n> -\t\tadd_exclude(exclude_list.items[i].string, \"\", 0,\n> -\t\t\t &dir.exclude_list_group[EXC_CMDL].el[0]);\n> +\t\tadd_exclude(exclude_list.items[i].string, \"\", 0, el);\n> \n> \tpathspec = get_pathspec(prefix, argv);\n>\n>\n> and by adopting the same approach for ls-files.c:\n\nThat is _much_ more readable and easier to explain in the API\ndocumentation, I think.\n\nThanks.\n"}]}