{"thread":{"id":"25047","subject":"[PATCH 0/8] en/object-list-with-pathspec v4","startedAt":"2010-09-08T15:50:15Z","lastAt":"2010-09-14T23:20:25Z","messageCount":21,"participants":["Nguyễn Thái Ngọc Duy","Elijah Newren","Nguyen Thai Ngoc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"150338","messageId":"1283961023-4491-1-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":null,"subject":"[PATCH 0/8] en/object-list-with-pathspec v4","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:15Z","receivedAt":"2010-09-08T15:50:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"OK it's good enough now. I no longer use struct exclude_list.\nIts name does not really match the semantics.\n\nThe last two patches implement tree exclusion and are not meant\nfor submission to en/object-list-with-pathspec. I wanted to see\nif the new struct was extensible. And I need tree exclusion anyway\nin my narrow clone.\n\nElijah Newren (2):\n  Add testcases showing how pathspecs are ignored with rev-list --objects\n  Make rev-list --objects work together with pathspecs\n\nNguyễn Thái Ngọc Duy (6):\n  diff-no-index: use diff_tree_setup_paths()\n  Introduce struct tree_pathspec_list\n  tree_entry_interesting(): remove dependency on struct diff_options\n  tree-walk: move tree_entry_interesting() from tree-diff.c\n  setup_tree_pathspec(): interpret '^' as negative pathspec\n  tree_entry_interesting(): support negative pathspec\n\n builtin/diff-files.c     |    2 +-\n builtin/diff.c           |    4 +-\n builtin/log.c            |    2 +-\n diff-lib.c               |    2 +-\n diff-no-index.c          |   13 ++--\n diff.h                   |    4 +-\n list-objects.c           |   23 ++++++\n revision.c               |   14 ++--\n revision.h               |    3 +-\n t/t6000-rev-list-misc.sh |   51 ++++++++++++\n t/t9999-test.sh          |   44 +++++++++++\n tree-diff.c              |  157 ++-----------------------------------\n tree-walk.c              |  191 ++++++++++++++++++++++++++++++++++++++++++++++\n tree-walk.h              |   16 ++++\n 14 files changed, 356 insertions(+), 170 deletions(-)\n create mode 100755 t/t6000-rev-list-misc.sh\n create mode 100755 t/t9999-test.sh\n"},{"id":"150339","messageId":"1283961023-4491-2-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/8] diff-no-index: use diff_tree_setup_paths()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:16Z","receivedAt":"2010-09-08T15:50:16Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"diff_options.{paths,nr_paths} will be removed later. Do not\nmodify them directly.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n diff-no-index.c |    9 +++++----\n 1 files changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex ce9e783..e48ab92 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -231,8 +231,9 @@ void diff_no_index(struct rev_info *revs,\n \n \tif (prefix) {\n \t\tint len = strlen(prefix);\n+\t\tconst char *paths[3];\n+\t\tmemset(paths, 0, sizeof(paths));\n \n-\t\trevs->diffopt.paths = xcalloc(2, sizeof(char *));\n \t\tfor (i = 0; i < 2; i++) {\n \t\t\tconst char *p = argv[argc - 2 + i];\n \t\t\t/*\n@@ -242,12 +243,12 @@ void diff_no_index(struct rev_info *revs,\n \t\t\tp = (strcmp(p, \"-\")\n \t\t\t     ? xstrdup(prefix_filename(prefix, len, p))\n \t\t\t     : p);\n-\t\t\trevs->diffopt.paths[i] = p;\n+\t\t\tpaths[i] = p;\n \t\t}\n+\t\tdiff_tree_setup_paths(paths, &revs->diffopt);\n \t}\n \telse\n-\t\trevs->diffopt.paths = argv + argc - 2;\n-\trevs->diffopt.nr_paths = 2;\n+\t\tdiff_tree_setup_paths(argv + argc - 2, &revs->diffopt);\n \trevs->diffopt.skip_stat_unmatch = 1;\n \tif (!revs->diffopt.output_format)\n \t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\n-- \n1.7.1.rc1.70.g13aff\n"},{"id":"150340","messageId":"1283961023-4491-3-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/8] Introduce struct tree_pathspec_list","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:17Z","receivedAt":"2010-09-08T15:50:17Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This struct replaces paths, nr_paths and pathlens fields in struct\ndiff_options. Actually diff_options.paths is still kept, now as\ntree_pathspec_list.paths because a lot of places depend on a\ncontinuous list of pathspecs.\n\ntree_entry_interesting() is going to be updated to use these instead.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/diff-files.c |    2 +-\n builtin/diff.c       |    4 ++--\n builtin/log.c        |    2 +-\n diff-lib.c           |    2 +-\n diff-no-index.c      |    4 ++--\n diff.h               |    4 +---\n revision.c           |    6 +-----\n tree-diff.c          |   46 +++++++++++-----------------------------------\n tree-walk.c          |   28 ++++++++++++++++++++++++++++\n tree-walk.h          |   11 +++++++++++\n 10 files changed, 59 insertions(+), 50 deletions(-)\n\ndiff --git a/builtin/diff-files.c b/builtin/diff-files.c\nindex 951c7c8..e740b77 100644\n--- a/builtin/diff-files.c\n+++ b/builtin/diff-files.c\n@@ -61,7 +61,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)\n \t    (rev.diffopt.output_format & DIFF_FORMAT_PATCH))\n \t\trev.combine_merges = rev.dense_combined_merges = 1;\n \n-\tif (read_cache_preload(rev.diffopt.paths) < 0) {\n+\tif (read_cache_preload(rev.diffopt.pathspec.paths) < 0) {\n \t\tperror(\"read_cache_preload\");\n \t\treturn -1;\n \t}\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex a43d326..4247377 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -135,7 +135,7 @@ static int builtin_diff_index(struct rev_info *revs,\n \t    revs->max_count != -1 || revs->min_age != -1 ||\n \t    revs->max_age != -1)\n \t\tusage(builtin_diff_usage);\n-\tif (read_cache_preload(revs->diffopt.paths) < 0) {\n+\tif (read_cache_preload(revs->diffopt.pathspec.paths) < 0) {\n \t\tperror(\"read_cache_preload\");\n \t\treturn -1;\n \t}\n@@ -237,7 +237,7 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv\n \t\trevs->combine_merges = revs->dense_combined_merges = 1;\n \n \tsetup_work_tree();\n-\tif (read_cache_preload(revs->diffopt.paths) < 0) {\n+\tif (read_cache_preload(revs->diffopt.pathspec.paths) < 0) {\n \t\tperror(\"read_cache_preload\");\n \t\treturn -1;\n \t}\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 08b8722..d78744f 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -89,7 +89,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,\n \t\trev->always_show_header = 0;\n \tif (DIFF_OPT_TST(&rev->diffopt, FOLLOW_RENAMES)) {\n \t\trev->always_show_header = 0;\n-\t\tif (rev->diffopt.nr_paths != 1)\n+\t\tif (rev->diffopt.pathspec.nr != 1)\n \t\t\tusage(\"git logs can only follow renames on one pathname at a time\");\n \t}\n \tfor (i = 1; i < argc; i++) {\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 392ce2b..0107e23 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -501,7 +501,7 @@ int do_diff_cache(const unsigned char *tree_sha1, struct diff_options *opt)\n \tactive_nr = dst - active_cache;\n \n \tinit_revisions(&revs, NULL);\n-\trevs.prune_data = opt->paths;\n+\trevs.prune_data = opt->pathspec.paths;\n \ttree = parse_tree_indirect(tree_sha1);\n \tif (!tree)\n \t\tdie(\"bad tree object %s\", sha1_to_hex(tree_sha1));\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex e48ab92..832d692 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -260,8 +260,8 @@ void diff_no_index(struct rev_info *revs,\n \tif (diff_setup_done(&revs->diffopt) < 0)\n \t\tdie(\"diff_setup_done failed\");\n \n-\tif (queue_diff(&revs->diffopt, revs->diffopt.paths[0],\n-\t\t       revs->diffopt.paths[1]))\n+\tif (queue_diff(&revs->diffopt, revs->diffopt.pathspec.paths[0],\n+\t\t       revs->diffopt.pathspec.paths[1]))\n \t\texit(1);\n \tdiff_set_mnemonic_prefix(&revs->diffopt, \"1/\", \"2/\");\n \tdiffcore_std(&revs->diffopt);\ndiff --git a/diff.h b/diff.h\nindex bf2f44d..aaa2983 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -133,9 +133,7 @@ struct diff_options {\n \tFILE *file;\n \tint close_file;\n \n-\tint nr_paths;\n-\tconst char **paths;\n-\tint *pathlens;\n+\tstruct tree_pathspec_list pathspec;\n \tchange_fn_t change;\n \tadd_remove_fn_t add_remove;\n \tdiff_format_fn_t format_callback;\ndiff --git a/revision.c b/revision.c\nindex b1c1890..b2a5867 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -553,11 +553,7 @@ static void cherry_pick_list(struct commit_list *list, struct rev_info *revs)\n \n \tleft_first = left_count < right_count;\n \tinit_patch_ids(&ids);\n-\tif (revs->diffopt.nr_paths) {\n-\t\tids.diffopts.nr_paths = revs->diffopt.nr_paths;\n-\t\tids.diffopts.paths = revs->diffopt.paths;\n-\t\tids.diffopts.pathlens = revs->diffopt.pathlens;\n-\t}\n+\tids.diffopts.pathspec = revs->diffopt.pathspec;\n \n \t/* Compute patch-ids for one side */\n \tfor (p = list; p; p = p->next) {\ndiff --git a/tree-diff.c b/tree-diff.c\nindex cd659c6..270dea0 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -100,16 +100,16 @@ static int tree_entry_interesting(struct tree_desc *desc, const char *base, int\n \tint pathlen;\n \tint never_interesting = -1;\n \n-\tif (!opt->nr_paths)\n+\tif (!opt->pathspec.nr)\n \t\treturn 1;\n \n \tsha1 = tree_entry_extract(desc, &path, &mode);\n \n \tpathlen = tree_entry_len(path, sha1);\n \n-\tfor (i = 0; i < opt->nr_paths; i++) {\n-\t\tconst char *match = opt->paths[i];\n-\t\tint matchlen = opt->pathlens[i];\n+\tfor (i = 0; i < opt->pathspec.nr; i++) {\n+\t\tconst char *match = opt->pathspec.paths[i];\n+\t\tint matchlen = opt->pathspec.info[i].pathlen;\n \t\tint m = -1; /* signals that we haven't called strncmp() */\n \n \t\tif (baselen >= matchlen) {\n@@ -289,7 +289,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2, const char *base, stru\n \t\tif (DIFF_OPT_TST(opt, QUICK) &&\n \t\t    DIFF_OPT_TST(opt, HAS_CHANGES))\n \t\t\tbreak;\n-\t\tif (opt->nr_paths) {\n+\t\tif (opt->pathspec.nr) {\n \t\t\tskip_uninteresting(t1, base, baselen, opt);\n \t\t\tskip_uninteresting(t2, base, baselen, opt);\n \t\t}\n@@ -348,7 +348,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co\n \tDIFF_OPT_SET(&diff_opts, RECURSIVE);\n \tDIFF_OPT_SET(&diff_opts, FIND_COPIES_HARDER);\n \tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n-\tdiff_opts.single_follow = opt->paths[0];\n+\tdiff_opts.single_follow = opt->pathspec.paths[0];\n \tdiff_opts.break_opt = opt->break_opt;\n \tpaths[0] = NULL;\n \tdiff_tree_setup_paths(paths, &diff_opts);\n@@ -368,15 +368,15 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co\n \t\t * diff_queued_diff, we will also use that as the path in\n \t\t * the future!\n \t\t */\n-\t\tif ((p->status == 'R' || p->status == 'C') && !strcmp(p->two->path, opt->paths[0])) {\n+\t\tif ((p->status == 'R' || p->status == 'C') && !strcmp(p->two->path, opt->pathspec.paths[0])) {\n \t\t\t/* Switch the file-pairs around */\n \t\t\tq->queue[i] = choice;\n \t\t\tchoice = p;\n \n \t\t\t/* Update the path we use from now on.. */\n \t\t\tdiff_tree_release_paths(opt);\n-\t\t\topt->paths[0] = xstrdup(p->one->path);\n-\t\t\tdiff_tree_setup_paths(opt->paths, opt);\n+\t\t\topt->pathspec.paths[0] = xstrdup(p->one->path);\n+\t\t\tdiff_tree_setup_paths(opt->pathspec.paths, opt);\n \n \t\t\t/*\n \t\t\t * The caller expects us to return a set of vanilla\n@@ -451,36 +451,12 @@ int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_\n \treturn retval;\n }\n \n-static int count_paths(const char **paths)\n-{\n-\tint i = 0;\n-\twhile (*paths++)\n-\t\ti++;\n-\treturn i;\n-}\n-\n void diff_tree_release_paths(struct diff_options *opt)\n {\n-\tfree(opt->pathlens);\n+\tfree(opt->pathspec.info);\n }\n \n void diff_tree_setup_paths(const char **p, struct diff_options *opt)\n {\n-\topt->nr_paths = 0;\n-\topt->pathlens = NULL;\n-\topt->paths = NULL;\n-\n-\tif (p) {\n-\t\tint i;\n-\n-\t\topt->paths = p;\n-\t\topt->nr_paths = count_paths(p);\n-\t\tif (opt->nr_paths == 0) {\n-\t\t\topt->pathlens = NULL;\n-\t\t\treturn;\n-\t\t}\n-\t\topt->pathlens = xmalloc(opt->nr_paths * sizeof(int));\n-\t\tfor (i=0; i < opt->nr_paths; i++)\n-\t\t\topt->pathlens[i] = strlen(p[i]);\n-\t}\n+\tsetup_tree_pathspec(p, &opt->pathspec);\n }\ndiff --git a/tree-walk.c b/tree-walk.c\nindex a9bbf4e..30c2aa1 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -455,3 +455,31 @@ int get_tree_entry(const unsigned char *tree_sha1, const char *name, unsigned ch\n \tfree(tree);\n \treturn retval;\n }\n+\n+static int count_paths(const char **paths)\n+{\n+\tint i = 0;\n+\twhile (*paths++)\n+\t\ti++;\n+\treturn i;\n+}\n+\n+int setup_tree_pathspec(const char **paths, struct tree_pathspec_list *ps)\n+{\n+\tint i;\n+\n+\tmemset(ps, 0, sizeof(*ps));\n+\tps->nr = count_paths(paths);\n+\tif (!ps->nr)\n+\t\treturn 0;\n+\n+\tps->paths = paths;\n+\tps->info = xmalloc(ps->nr * sizeof(struct tree_pathspec));\n+\tmemset(ps->info, 0, ps->nr * sizeof(struct tree_pathspec));\n+\tfor (i=0; i < ps->nr; i++) {\n+\t\tstruct tree_pathspec *exc = ps->info+i;\n+\t\texc->path = ps->paths[i];\n+\t\texc->pathlen = strlen(exc->path);\n+\t}\n+\treturn 0;\n+}\ndiff --git a/tree-walk.h b/tree-walk.h\nindex 88ea7e9..2d09b7c 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -13,6 +13,15 @@ struct tree_desc {\n \tunsigned int size;\n };\n \n+struct tree_pathspec_list {\n+\tconst char **paths;\n+\tint nr;\n+\tstruct tree_pathspec {\n+\t\tconst char *path;\n+\t\tint pathlen;\n+\t} *info;\n+};\n+\n static inline const unsigned char *tree_entry_extract(struct tree_desc *desc, const char **pathp, unsigned int *modep)\n {\n \t*pathp = desc->entry.path;\n@@ -57,4 +66,6 @@ static inline int traverse_path_len(const struct traverse_info *info, const stru\n \treturn info->pathlen + tree_entry_len(n->path, n->sha1);\n }\n \n+extern int setup_tree_pathspec(const char **paths, struct tree_pathspec_list *ps);\n+\n #endif\n-- \n1.7.1.rc1.70.g13aff\n"},{"id":"150341","messageId":"1283961023-4491-4-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/8] tree_entry_interesting(): remove dependency on struct diff_options","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:18Z","receivedAt":"2010-09-08T15:50:18Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This function can be potentially used in more places than just\ntree-diff.c. \"struct diff_options\" does not make much sense outside\ndiff_tree_sha1().\n\nMoreover people seem to be agree that diff machinery should learn\nproper pathspecs too (i.e. globbing, negative pathspecs...), not just\ntreating pathspecs as tree prefix.\n\nSo tree_entry_interesting() now uses struct tree_pathspec_list, which\ncan be easily extended later on.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n tree-diff.c |   28 +++++++++++-----------------\n 1 files changed, 11 insertions(+), 17 deletions(-)\n\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 270dea0..b0aa3a0 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -91,25 +91,20 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const\n  *  - zero for no\n  *  - negative for \"no, and no subsequent entries will be either\"\n  */\n-static int tree_entry_interesting(struct tree_desc *desc, const char *base, int baselen, struct diff_options *opt)\n+static int tree_entry_interesting(const struct name_entry *entry, const char *base, int baselen, const struct tree_pathspec_list *ps)\n {\n-\tconst char *path;\n-\tconst unsigned char *sha1;\n-\tunsigned mode;\n \tint i;\n \tint pathlen;\n \tint never_interesting = -1;\n \n-\tif (!opt->pathspec.nr)\n+\tif (!ps->nr)\n \t\treturn 1;\n \n-\tsha1 = tree_entry_extract(desc, &path, &mode);\n-\n-\tpathlen = tree_entry_len(path, sha1);\n+\tpathlen = tree_entry_len(entry->path, entry->sha1);\n \n-\tfor (i = 0; i < opt->pathspec.nr; i++) {\n-\t\tconst char *match = opt->pathspec.paths[i];\n-\t\tint matchlen = opt->pathspec.info[i].pathlen;\n+\tfor (i = 0; i < ps->nr; i++) {\n+\t\tconst char *match = ps->info[i].path;\n+\t\tint matchlen = ps->info[i].pathlen;\n \t\tint m = -1; /* signals that we haven't called strncmp() */\n \n \t\tif (baselen >= matchlen) {\n@@ -147,7 +142,7 @@ static int tree_entry_interesting(struct tree_desc *desc, const char *base, int\n \t\t\t * Does match sort strictly earlier than path\n \t\t\t * with their common parts?\n \t\t\t */\n-\t\t\tm = strncmp(match, path,\n+\t\t\tm = strncmp(match, entry->path,\n \t\t\t\t    (matchlen < pathlen) ? matchlen : pathlen);\n \t\t\tif (m < 0)\n \t\t\t\tcontinue;\n@@ -174,7 +169,7 @@ static int tree_entry_interesting(struct tree_desc *desc, const char *base, int\n \t\tif (matchlen > pathlen) {\n \t\t\tif (match[pathlen] != '/')\n \t\t\t\tcontinue;\n-\t\t\tif (!S_ISDIR(mode))\n+\t\t\tif (!S_ISDIR(entry->mode))\n \t\t\t\tcontinue;\n \t\t}\n \n@@ -183,7 +178,7 @@ static int tree_entry_interesting(struct tree_desc *desc, const char *base, int\n \t\t\t * we cheated and did not do strncmp(), so we do\n \t\t\t * that here.\n \t\t\t */\n-\t\t\tm = strncmp(match, path, pathlen);\n+\t\t\tm = strncmp(match, entry->path, pathlen);\n \n \t\t/*\n \t\t * If common part matched earlier then it is a hit,\n@@ -206,8 +201,7 @@ static void show_tree(struct diff_options *opt, const char *prefix, struct tree_\n \t\tif (all_interesting)\n \t\t\tshow = 1;\n \t\telse {\n-\t\t\tshow = tree_entry_interesting(desc, base, baselen,\n-\t\t\t\t\t\t      opt);\n+\t\t\tshow = tree_entry_interesting(&desc->entry, base, baselen, &opt->pathspec);\n \t\t\tif (show == 2)\n \t\t\t\tall_interesting = 1;\n \t\t}\n@@ -266,7 +260,7 @@ static void skip_uninteresting(struct tree_desc *t, const char *base, int basele\n \t\tif (all_interesting)\n \t\t\tshow = 1;\n \t\telse {\n-\t\t\tshow = tree_entry_interesting(t, base, baselen, opt);\n+\t\t\tshow = tree_entry_interesting(&t->entry, base, baselen, &opt->pathspec);\n \t\t\tif (show == 2)\n \t\t\t\tall_interesting = 1;\n \t\t}\n-- \n1.7.1.rc1.70.g13aff\n"},{"id":"150342","messageId":"1283961023-4491-5-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 4/8] tree-walk: move tree_entry_interesting() from tree-diff.c","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:19Z","receivedAt":"2010-09-08T15:50:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n tree-diff.c |  109 -----------------------------------------------------------\n tree-walk.c |  109 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n tree-walk.h |    2 +\n 3 files changed, 111 insertions(+), 109 deletions(-)\n\ndiff --git a/tree-diff.c b/tree-diff.c\nindex b0aa3a0..4b9d551 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -82,115 +82,6 @@ static int compare_tree_entry(struct tree_desc *t1, struct tree_desc *t2, const\n \treturn 0;\n }\n \n-/*\n- * Is a tree entry interesting given the pathspec we have?\n- *\n- * Return:\n- *  - 2 for \"yes, and all subsequent entries will be\"\n- *  - 1 for yes\n- *  - zero for no\n- *  - negative for \"no, and no subsequent entries will be either\"\n- */\n-static int tree_entry_interesting(const struct name_entry *entry, const char *base, int baselen, const struct tree_pathspec_list *ps)\n-{\n-\tint i;\n-\tint pathlen;\n-\tint never_interesting = -1;\n-\n-\tif (!ps->nr)\n-\t\treturn 1;\n-\n-\tpathlen = tree_entry_len(entry->path, entry->sha1);\n-\n-\tfor (i = 0; i < ps->nr; i++) {\n-\t\tconst char *match = ps->info[i].path;\n-\t\tint matchlen = ps->info[i].pathlen;\n-\t\tint m = -1; /* signals that we haven't called strncmp() */\n-\n-\t\tif (baselen >= matchlen) {\n-\t\t\t/* If it doesn't match, move along... */\n-\t\t\tif (strncmp(base, match, matchlen))\n-\t\t\t\tcontinue;\n-\n-\t\t\t/*\n-\t\t\t * If the base is a subdirectory of a path which\n-\t\t\t * was specified, all of them are interesting.\n-\t\t\t */\n-\t\t\tif (!matchlen ||\n-\t\t\t    base[matchlen] == '/' ||\n-\t\t\t    match[matchlen - 1] == '/')\n-\t\t\t\treturn 2;\n-\n-\t\t\t/* Just a random prefix match */\n-\t\t\tcontinue;\n-\t\t}\n-\n-\t\t/* Does the base match? */\n-\t\tif (strncmp(base, match, baselen))\n-\t\t\tcontinue;\n-\n-\t\tmatch += baselen;\n-\t\tmatchlen -= baselen;\n-\n-\t\tif (never_interesting) {\n-\t\t\t/*\n-\t\t\t * We have not seen any match that sorts later\n-\t\t\t * than the current path.\n-\t\t\t */\n-\n-\t\t\t/*\n-\t\t\t * Does match sort strictly earlier than path\n-\t\t\t * with their common parts?\n-\t\t\t */\n-\t\t\tm = strncmp(match, entry->path,\n-\t\t\t\t    (matchlen < pathlen) ? matchlen : pathlen);\n-\t\t\tif (m < 0)\n-\t\t\t\tcontinue;\n-\n-\t\t\t/*\n-\t\t\t * If we come here even once, that means there is at\n-\t\t\t * least one pathspec that would sort equal to or\n-\t\t\t * later than the path we are currently looking at.\n-\t\t\t * In other words, if we have never reached this point\n-\t\t\t * after iterating all pathspecs, it means all\n-\t\t\t * pathspecs are either outside of base, or inside the\n-\t\t\t * base but sorts strictly earlier than the current\n-\t\t\t * one.  In either case, they will never match the\n-\t\t\t * subsequent entries.  In such a case, we initialized\n-\t\t\t * the variable to -1 and that is what will be\n-\t\t\t * returned, allowing the caller to terminate early.\n-\t\t\t */\n-\t\t\tnever_interesting = 0;\n-\t\t}\n-\n-\t\tif (pathlen > matchlen)\n-\t\t\tcontinue;\n-\n-\t\tif (matchlen > pathlen) {\n-\t\t\tif (match[pathlen] != '/')\n-\t\t\t\tcontinue;\n-\t\t\tif (!S_ISDIR(entry->mode))\n-\t\t\t\tcontinue;\n-\t\t}\n-\n-\t\tif (m == -1)\n-\t\t\t/*\n-\t\t\t * we cheated and did not do strncmp(), so we do\n-\t\t\t * that here.\n-\t\t\t */\n-\t\t\tm = strncmp(match, entry->path, pathlen);\n-\n-\t\t/*\n-\t\t * If common part matched earlier then it is a hit,\n-\t\t * because we rejected the case where path is not a\n-\t\t * leading directory and is shorter than match.\n-\t\t */\n-\t\tif (!m)\n-\t\t\treturn 1;\n-\t}\n-\treturn never_interesting; /* No matches */\n-}\n-\n /* A whole sub-tree went away or appeared */\n static void show_tree(struct diff_options *opt, const char *prefix, struct tree_desc *desc, const char *base, int baselen)\n {\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 30c2aa1..e7041d7 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -483,3 +483,112 @@ int setup_tree_pathspec(const char **paths, struct tree_pathspec_list *ps)\n \t}\n \treturn 0;\n }\n+\n+/*\n+ * Is a tree entry interesting given the pathspec we have?\n+ *\n+ * Return:\n+ *  - 2 for \"yes, and all subsequent entries will be\"\n+ *  - 1 for yes\n+ *  - zero for no\n+ *  - negative for \"no, and no subsequent entries will be either\"\n+ */\n+int tree_entry_interesting(const struct name_entry *entry, const char *base, int baselen, const struct tree_pathspec_list *ps)\n+{\n+\tint i;\n+\tint pathlen;\n+\tint never_interesting = -1;\n+\n+\tif (!ps->nr)\n+\t\treturn 1;\n+\n+\tpathlen = tree_entry_len(entry->path, entry->sha1);\n+\n+\tfor (i = 0; i < ps->nr; i++) {\n+\t\tconst char *match = ps->info[i].path;\n+\t\tint matchlen = ps->info[i].pathlen;\n+\t\tint m = -1; /* signals that we haven't called strncmp() */\n+\n+\t\tif (baselen >= matchlen) {\n+\t\t\t/* If it doesn't match, move along... */\n+\t\t\tif (strncmp(base, match, matchlen))\n+\t\t\t\tcontinue;\n+\n+\t\t\t/*\n+\t\t\t * If the base is a subdirectory of a path which\n+\t\t\t * was specified, all of them are interesting.\n+\t\t\t */\n+\t\t\tif (!matchlen ||\n+\t\t\t    base[matchlen] == '/' ||\n+\t\t\t    match[matchlen - 1] == '/')\n+\t\t\t\treturn 2;\n+\n+\t\t\t/* Just a random prefix match */\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/* Does the base match? */\n+\t\tif (strncmp(base, match, baselen))\n+\t\t\tcontinue;\n+\n+\t\tmatch += baselen;\n+\t\tmatchlen -= baselen;\n+\n+\t\tif (never_interesting) {\n+\t\t\t/*\n+\t\t\t * We have not seen any match that sorts later\n+\t\t\t * than the current path.\n+\t\t\t */\n+\n+\t\t\t/*\n+\t\t\t * Does match sort strictly earlier than path\n+\t\t\t * with their common parts?\n+\t\t\t */\n+\t\t\tm = strncmp(match, entry->path,\n+\t\t\t\t    (matchlen < pathlen) ? matchlen : pathlen);\n+\t\t\tif (m < 0)\n+\t\t\t\tcontinue;\n+\n+\t\t\t/*\n+\t\t\t * If we come here even once, that means there is at\n+\t\t\t * least one pathspec that would sort equal to or\n+\t\t\t * later than the path we are currently looking at.\n+\t\t\t * In other words, if we have never reached this point\n+\t\t\t * after iterating all pathspecs, it means all\n+\t\t\t * pathspecs are either outside of base, or inside the\n+\t\t\t * base but sorts strictly earlier than the current\n+\t\t\t * one.  In either case, they will never match the\n+\t\t\t * subsequent entries.  In such a case, we initialized\n+\t\t\t * the variable to -1 and that is what will be\n+\t\t\t * returned, allowing the caller to terminate early.\n+\t\t\t */\n+\t\t\tnever_interesting = 0;\n+\t\t}\n+\n+\t\tif (pathlen > matchlen)\n+\t\t\tcontinue;\n+\n+\t\tif (matchlen > pathlen) {\n+\t\t\tif (match[pathlen] != '/')\n+\t\t\t\tcontinue;\n+\t\t\tif (!S_ISDIR(entry->mode))\n+\t\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (m == -1)\n+\t\t\t/*\n+\t\t\t * we cheated and did not do strncmp(), so we do\n+\t\t\t * that here.\n+\t\t\t */\n+\t\t\tm = strncmp(match, entry->path, pathlen);\n+\n+\t\t/*\n+\t\t * If common part matched earlier then it is a hit,\n+\t\t * because we rejected the case where path is not a\n+\t\t * leading directory and is shorter than match.\n+\t\t */\n+\t\tif (!m)\n+\t\t\treturn 1;\n+\t}\n+\treturn never_interesting; /* No matches */\n+}\ndiff --git a/tree-walk.h b/tree-walk.h\nindex 2d09b7c..bb55656 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -67,5 +67,7 @@ static inline int traverse_path_len(const struct traverse_info *info, const stru\n }\n \n extern int setup_tree_pathspec(const char **paths, struct tree_pathspec_list *ps);\n+extern int tree_entry_interesting(const struct name_entry *entry, const char *base, int baselen,\n+\t\t\t\t  const struct tree_pathspec_list *ps);\n \n #endif\n-- \n1.7.1.rc1.70.g13aff\n"},{"id":"150343","messageId":"1283961023-4491-6-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 5/8] Add testcases showing how pathspecs are ignored with rev-list --objects","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:20Z","receivedAt":"2010-09-08T15:50:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"From: Elijah Newren <newren@gmail.com>\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n t/t6000-rev-list-misc.sh |   51 ++++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 51 insertions(+), 0 deletions(-)\n create mode 100755 t/t6000-rev-list-misc.sh\n\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nnew file mode 100755\nindex 0000000..b3c1dd8\n--- /dev/null\n+++ b/t/t6000-rev-list-misc.sh\n@@ -0,0 +1,51 @@\n+#!/bin/sh\n+\n+test_description='miscellaneous rev-list tests'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\techo content1 >wanted_file &&\n+\techo content2 >unwanted_file &&\n+\tgit add wanted_file unwanted_file &&\n+\tgit commit -m one\n+'\n+\n+test_expect_failure 'rev-list --objects heeds pathspecs' '\n+\tgit rev-list --objects HEAD -- wanted_file >output &&\n+\tgrep wanted_file output &&\n+\t! grep unwanted_file output\n+'\n+\n+test_expect_failure 'rev-list --objects with pathspecs and deeper paths' '\n+\tmkdir foo &&\n+\t>foo/file &&\n+\tgit add foo/file &&\n+\tgit commit -m two &&\n+\n+\tgit rev-list --objects HEAD -- foo >output &&\n+\tgrep foo/file output &&\n+\n+\tgit rev-list --objects HEAD -- foo/file >output &&\n+\tgrep foo/file output &&\n+\t! grep unwanted_file output\n+'\n+\n+test_expect_failure 'rev-list --objects with pathspecs and copied files' '\n+\tgit checkout --orphan junio-testcase &&\n+\tgit rm -rf . &&\n+\n+\tmkdir two &&\n+\techo frotz >one &&\n+\tcp one two/three &&\n+\tgit add one two/three &&\n+\ttest_tick &&\n+\tgit commit -m that &&\n+\n+\tONE=$(git rev-parse HEAD:one)\n+\tgit rev-list --objects HEAD two >output &&\n+\tgrep \"$ONE two/three\" output &&\n+\t! grep one output\n+'\n+\n+test_done\n-- \n1.7.1.rc1.70.g13aff\n"},{"id":"150344","messageId":"1283961023-4491-7-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 6/8] Make rev-list --objects work together with pathspecs","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:21Z","receivedAt":"2010-09-08T15:50:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"From: Elijah Newren <newren@gmail.com>\n\nWhen traversing commits, the selection of commits would heed the list of\npathspecs passed, but subsequent walking of the trees of those commits\nwould not.  This resulted in 'rev-list --objects HEAD -- <paths>'\ndisplaying objects at unwanted paths.\n\nHave process_tree() call tree_entry_interesting() to determine which paths\nare interesting and should be walked.\n\nNaturally, this change can provide a large speedup when paths are specified\ntogether with --objects, since many tree entries are now correctly ignored.\nInterestingly, though, this change also gives me a small (~1%) but\nrepeatable speedup even when no paths are specified with --objects.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n list-objects.c           |   23 +++++++++++++++++++++++\n revision.c               |    8 ++++++--\n revision.h               |    3 ++-\n t/t6000-rev-list-misc.sh |    6 +++---\n 4 files changed, 34 insertions(+), 6 deletions(-)\n\ndiff --git a/list-objects.c b/list-objects.c\nindex 8953548..be4cf9f 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -67,6 +67,9 @@ static void process_tree(struct rev_info *revs,\n \tstruct tree_desc desc;\n \tstruct name_entry entry;\n \tstruct name_path me;\n+\tint all_interesting = (revs->diffopt.pathspec.nr == 0);\n+\tchar *full_prefix = NULL;\n+\tint full_prefix_len = 0;\n \n \tif (!revs->tree_objects)\n \t\treturn;\n@@ -82,9 +85,28 @@ static void process_tree(struct rev_info *revs,\n \tme.elem = name;\n \tme.elem_len = strlen(name);\n \n+\tif (!all_interesting) {\n+\t\tfull_prefix = path_name_impl(path, name, 1);\n+\t\tfull_prefix_len = strlen(full_prefix);\n+\t}\n+\n \tinit_tree_desc(&desc, tree->buffer, tree->size);\n \n \twhile (tree_entry(&desc, &entry)) {\n+\t\tif (!all_interesting) {\n+\t\t\tint showit = tree_entry_interesting(&entry,\n+\t\t\t\t\t\t\t    full_prefix,\n+\t\t\t\t\t\t\t    full_prefix_len,\n+\t\t\t\t\t\t\t    &revs->diffopt.pathspec);\n+\n+\t\t\tif (showit < 0)\n+\t\t\t\tbreak;\n+\t\t\telse if (!showit)\n+\t\t\t\tcontinue;\n+\t\t\telse if (showit == 2)\n+\t\t\t\tall_interesting = 1;\n+\t\t}\n+\n \t\tif (S_ISDIR(entry.mode))\n \t\t\tprocess_tree(revs,\n \t\t\t\t     lookup_tree(entry.sha1),\n@@ -97,6 +119,7 @@ static void process_tree(struct rev_info *revs,\n \t\t\t\t     lookup_blob(entry.sha1),\n \t\t\t\t     show, &me, entry.path);\n \t}\n+\tfree(full_prefix);\n \tfree(tree->buffer);\n \ttree->buffer = NULL;\n }\ndiff --git a/revision.c b/revision.c\nindex b2a5867..4d7dc4b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -16,7 +16,7 @@\n \n volatile show_early_output_fn_t show_early_output;\n \n-char *path_name(const struct name_path *path, const char *name)\n+char *path_name_impl(const struct name_path *path, const char *name, int isdir)\n {\n \tconst struct name_path *p;\n \tchar *n, *m;\n@@ -27,7 +27,7 @@ char *path_name(const struct name_path *path, const char *name)\n \t\tif (p->elem_len)\n \t\t\tlen += p->elem_len + 1;\n \t}\n-\tn = xmalloc(len);\n+\tn = xmalloc(len + !!isdir);\n \tm = n + len - (nlen + 1);\n \tstrcpy(m, name);\n \tfor (p = path; p; p = p->up) {\n@@ -37,6 +37,10 @@ char *path_name(const struct name_path *path, const char *name)\n \t\t\tm[p->elem_len] = '/';\n \t\t}\n \t}\n+\tif (isdir && len > 1) {\n+\t\tn[len-1] = '/';\n+\t\tn[len] = '\\0';\n+\t}\n \treturn n;\n }\n \ndiff --git a/revision.h b/revision.h\nindex 05659c6..92f4feb 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -173,7 +173,8 @@ struct name_path {\n \tconst char *elem;\n };\n \n-char *path_name(const struct name_path *path, const char *name);\n+char *path_name_impl(const struct name_path *path, const char *name, int isdir);\n+#define path_name(path, name) path_name_impl(path, name, 0)\n \n extern void add_object(struct object *obj,\n \t\t       struct object_array *p,\ndiff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh\nindex b3c1dd8..b10685a 100755\n--- a/t/t6000-rev-list-misc.sh\n+++ b/t/t6000-rev-list-misc.sh\n@@ -11,13 +11,13 @@ test_expect_success setup '\n \tgit commit -m one\n '\n \n-test_expect_failure 'rev-list --objects heeds pathspecs' '\n+test_expect_success 'rev-list --objects heeds pathspecs' '\n \tgit rev-list --objects HEAD -- wanted_file >output &&\n \tgrep wanted_file output &&\n \t! grep unwanted_file output\n '\n \n-test_expect_failure 'rev-list --objects with pathspecs and deeper paths' '\n+test_expect_success 'rev-list --objects with pathspecs and deeper paths' '\n \tmkdir foo &&\n \t>foo/file &&\n \tgit add foo/file &&\n@@ -31,7 +31,7 @@ test_expect_failure 'rev-list --objects with pathspecs and deeper paths' '\n \t! grep unwanted_file output\n '\n \n-test_expect_failure 'rev-list --objects with pathspecs and copied files' '\n+test_expect_success 'rev-list --objects with pathspecs and copied files' '\n \tgit checkout --orphan junio-testcase &&\n \tgit rm -rf . &&\n \n-- \n1.7.1.rc1.70.g13aff\n"},{"id":"150345","messageId":"1283961023-4491-8-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 7/8] setup_tree_pathspec(): interpret '^' as negative pathspec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:22Z","receivedAt":"2010-09-08T15:50:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This patch does preparation work for tree exclusion in\ntree_entry_interesting(). '^' has similar meaning to '!' in\ngitexcludes. '!' is not used because bash does not like arguments with\na leading '!'.\n\nEventually, \"git diff -- foo ^foo/bar\" should show differences in foo,\nexcept foo/bar. If \"git diff -- ^foo\" is given, then it implies\neverything except foo, which could surprise users that\n\"bar\" in \"git diff -- bar ^foo\" has no effect at all.\n\nNOTE: pathspec in diff machinery is also used by ce_path_match() and\nread_index_preload(), which currently do not understand '^' at all.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n tree-walk.c |   28 ++++++++++++++++++++++++++++\n tree-walk.h |    3 +++\n 2 files changed, 31 insertions(+), 0 deletions(-)\n\ndiff --git a/tree-walk.c b/tree-walk.c\nindex e7041d7..a2f4a00 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -479,7 +479,35 @@ int setup_tree_pathspec(const char **paths, struct tree_pathspec_list *ps)\n \tfor (i=0; i < ps->nr; i++) {\n \t\tstruct tree_pathspec *exc = ps->info+i;\n \t\texc->path = ps->paths[i];\n+\t\tif (*exc->path == '^') {\n+\t\t\texc->path++;\n+\t\t\texc->to_exclude = 1;\n+\t\t}\n \t\texc->pathlen = strlen(exc->path);\n+\t\tif (exc->to_exclude) {\n+\t\t\tint j, found = 0;\n+\n+\t\t\tfor (j = i-1; j >= 0; j--) {\n+\t\t\t\tint len = strlen(ps->info[j].path);\n+\t\t\t\tif (len < exc->pathlen &&\n+\t\t\t\t    !strncmp(ps->info[j].path, exc->path, len) &&\n+\t\t\t\t    exc->path[len] == '/') {\n+\t\t\t\t\tps->info[j].has_sub_pathspec = 1;\n+\t\t\t\t\tfound = 1;\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\t/*\n+\t\t\t * If a negative pathspec has nothing to\n+\t\t\t * negate from, include everything so it can\n+\t\t\t * negate from that.  This way is not\n+\t\t\t * perfect. You may be surprised to find out\n+\t\t\t * \"^Documentation t\" does not take \"t\" into\n+\t\t\t * account at all\n+\t\t\t */\n+\t\t\tif (!found)\n+\t\t\t\tps->include_by_default = 1;\n+\t\t}\n \t}\n \treturn 0;\n }\ndiff --git a/tree-walk.h b/tree-walk.h\nindex bb55656..6be1e6c 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -16,9 +16,12 @@ struct tree_desc {\n struct tree_pathspec_list {\n \tconst char **paths;\n \tint nr;\n+\tint include_by_default:1;\n \tstruct tree_pathspec {\n \t\tconst char *path;\n \t\tint pathlen;\n+\t\tint to_exclude:1;\n+\t\tint has_sub_pathspec:1;\n \t} *info;\n };\n \n-- \n1.7.1.rc1.70.g13aff\n"},{"id":"150346","messageId":"1283961023-4491-9-git-send-email-pclouds@gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 8/8] tree_entry_interesting(): support negative pathspec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-08T15:50:23Z","receivedAt":"2010-09-08T15:50:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n t/t9999-test.sh |   44 ++++++++++++++++++++++++++++++++++++++++++++\n tree-walk.c     |   36 +++++++++++++++++++++++++++++++-----\n 2 files changed, 75 insertions(+), 5 deletions(-)\n create mode 100755 t/t9999-test.sh\n\ndiff --git a/t/t9999-test.sh b/t/t9999-test.sh\nnew file mode 100755\nindex 0000000..535386e\n--- /dev/null\n+++ b/t/t9999-test.sh\n@@ -0,0 +1,44 @@\n+#!/bin/sh\n+test_description=f\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\tmkdir one one/two &&\n+\ttouch file one/file one/two/file one/zoo &&\n+\tgit add . &&\n+\tgit commit -m 1 &&\n+\tfor i in file one/file one/two/file one/zoo; do echo 1 >>$i;done &&\n+\tgit add . && git commit -m 2\n+'\n+\n+test_expect_success 'diff' '\n+\tcat >expected <<-\\EOF &&\n+:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/file\n+:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/two/file\n+:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/zoo\n+EOF\n+\tgit diff-tree -r HEAD^ HEAD -- one >result &&\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'diff one ^one/two' '\n+\tcat >expected <<-\\EOF &&\n+:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/file\n+:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/zoo\n+EOF\n+\tgit diff-tree -r HEAD^ HEAD -- one ^one/two >result &&\n+\ttest_cmp expected result\n+'\n+\n+test_expect_success 'diff ^one/two' '\n+\tcat >expected <<-\\EOF &&\n+:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tfile\n+:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/file\n+:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/zoo\n+EOF\n+\tgit diff-tree -r HEAD^ HEAD -- ^one/two >result &&\n+\ttest_cmp expected result\n+'\n+\n+test_done\ndiff --git a/tree-walk.c b/tree-walk.c\nindex a2f4a00..fd19681 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -532,7 +532,7 @@ int tree_entry_interesting(const struct name_entry *entry, const char *base, int\n \n \tpathlen = tree_entry_len(entry->path, entry->sha1);\n \n-\tfor (i = 0; i < ps->nr; i++) {\n+\tfor (i = ps->nr-1; i >= 0; i--) {\n \t\tconst char *match = ps->info[i].path;\n \t\tint matchlen = ps->info[i].pathlen;\n \t\tint m = -1; /* signals that we haven't called strncmp() */\n@@ -548,8 +548,18 @@ int tree_entry_interesting(const struct name_entry *entry, const char *base, int\n \t\t\t */\n \t\t\tif (!matchlen ||\n \t\t\t    base[matchlen] == '/' ||\n-\t\t\t    match[matchlen - 1] == '/')\n-\t\t\t\treturn 2;\n+\t\t\t    match[matchlen - 1] == '/') {\n+\t\t\t\tif (!ps->info[i].has_sub_pathspec &&\n+\t\t\t\t    ps->info[i].to_exclude)\n+\t\t\t\t\treturn -1;\n+\t\t\t\t/*\n+\t\t\t\t * If has_sub_pathspec is 1, there is\n+\t\t\t\t * another sub pathspec that will\n+\t\t\t\t * negate the result. Don't return \"2\"\n+\t\t\t\t * in that case.\n+\t\t\t\t */\n+\t\t\t\treturn ps->info[i].has_sub_pathspec ? 1 : 2;\n+\t\t\t}\n \n \t\t\t/* Just a random prefix match */\n \t\t\tcontinue;\n@@ -615,8 +625,24 @@ int tree_entry_interesting(const struct name_entry *entry, const char *base, int\n \t\t * because we rejected the case where path is not a\n \t\t * leading directory and is shorter than match.\n \t\t */\n-\t\tif (!m)\n+\t\tif (!m) {\n+\t\t\t/*\n+\t\t\t * If this is negative pathspec, we exclude\n+\t\t\t * the path only if it's a perfect\n+\t\t\t * match. Otherwise assume it's in, because\n+\t\t\t * there could be a positive subpathspec,\n+\t\t\t * which includes some trees again.\n+\t\t\t */\n+\t\t\tif (ps->info[i].to_exclude)\n+\t\t\t\treturn matchlen == pathlen ? 0 : 1;\n \t\t\treturn 1;\n+\t\t}\n \t}\n-\treturn never_interesting; /* No matches */\n+\n+\t/*\n+\t * if include_by_default is 1, all items should be included by\n+\t * default, because some of them will be excluded later on by\n+\t * negative pathspecs.\n+\t */\n+\treturn ps->include_by_default ? 1 : never_interesting; /* No matches */\n }\n-- \n1.7.1.rc1.70.g13aff\n"},{"id":"150503","messageId":"AANLkTimEcrFEGZ3RYXsByLT275=X0E-D29Sio3pOjH6T@mail.gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 0/8] en/object-list-with-pathspec v4","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-11T17:19:09Z","receivedAt":"2010-09-11T17:19:09Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\n2010/9/8 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:\n> OK it's good enough now. I no longer use struct exclude_list.\n> Its name does not really match the semantics.\n>\n> The last two patches implement tree exclusion and are not meant\n> for submission to en/object-list-with-pathspec. I wanted to see\n> if the new struct was extensible. And I need tree exclusion anyway\n> in my narrow clone.\n\nPerhaps the last two patches should be split off and submitted\nseparately?  I really like your work here to add negated pathspecs;\nthey'll be really helpful for me.  However, as you say, they are\nreally moving into a different topic.\n\nI've reviewed and tested patches 1-4 of this series, and they look\ngood to me.  (Am I supposed to add a Reviewed-by and Tested-by or an\nAcked-by for these?  Still not sure what the rules are there).\nPatches 5-6 are already part of pu (modulo the return of tree_entry(),\nwhich is nice) and already have my signoff, so I don't need to comment\non those further.\n\nI've got some comments for patches 7 & 8, which I think may need a\nlittle more work; I'll add them to the individual emails.\n\n\nElijah\n"},{"id":"150506","messageId":"AANLkTin_m+zjHND5AwFhkrZM-VEkn70qgCTwpB2B+RA+@mail.gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-8-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 7/8] setup_tree_pathspec(): interpret '^' as negative pathspec","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-11T17:29:00Z","receivedAt":"2010-09-11T17:29:00Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"2010/9/8 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:\n> This patch does preparation work for tree exclusion in\n> tree_entry_interesting(). '^' has similar meaning to '!' in\n> gitexcludes. '!' is not used because bash does not like arguments with\n> a leading '!'.\n>\n> Eventually, \"git diff -- foo ^foo/bar\" should show differences in foo,\n> except foo/bar. If \"git diff -- ^foo\" is given, then it implies\n> everything except foo, which could surprise users that\n> \"bar\" in \"git diff -- bar ^foo\" has no effect at all.\n\nI really like the work here.  There are just two things that I think\nare missing:\n  * It doesn't handle files with leading carats in their name\n  * It handles some nested include/exclude cases (e.g. dir\n^dir/subdir) but not more complicated ones.\n\nYou could add these three testcases (on top of your 8/8 patch) to see\nwhat I mean:\n\n\ntest_expect_failure 'diff ^one one/two' '\n\tprintf \"file\\none/two/file\\n\" >expected &&\n\tgit diff --name-only HEAD^ HEAD -- ^one one/two >result &&\n\ttest_cmp expected result\n'\n\ntest_expect_failure 'diff ^funny-filename' '\n\ttouch ^funny-filename &&\n\tgit add ^funny-filename &&\n\tgit commit -m \"Add a filename with a leading carat\" &&\n\n\tgit ls-files >expected &&\n\tgit diff --name-only HEAD^^ HEAD -- ^funny-filename >result &&\n\ttest_cmp expected result &&\n\n\techo ^funny-filename >expected &&\n\tgit diff --name-only HEAD^^ HEAD -- ^^funny-filename >result &&\n\ttest_cmp expected result &&\n\n\tgit ls-files | grep -v funny-filename >expected &&\n\tgit diff --name-only HEAD^^ HEAD -- ^^^funny-filename >result &&\n\ttest_cmp expected result\n'\n\ntest_expect_failure 'deeper nested exclude/include' '\n\ttouch one/two/zoo &&\n\tfor i in one/file one/zoo one/two/file; do echo 2 >>$i; done &&\n\tgit add one &&\n\tgit commit -m 4 &&\n\n\tprintf \"one/file\\none/two/file\\none/zoo\\n\" > expected &&\n\tgit diff --name-only HEAD^ HEAD -- one ^one/two one/two/file >result &&\n\ttest_cmp expected result &&\n\n\tprintf \"one/two/zoo\\n\" > expected &&\n\tgit diff --name-only HEAD^ HEAD -- ^one one/two ^one/two/file >result &&\n\ttest_cmp expected result\n'\n\nNote: In the second test, I used:\n * \"^funny\" to search for all files EXCEPT \"funny\"\n * \"^^funny\" to search for a file named \"^funny\"\n * \"^^^funny\" to search for all files EXCEPT \"^funny\"\nI'm not sure if that's really the syntax we want to adopt, but it\nshould be easy to change if we decide on some other syntax.\n"},{"id":"150507","messageId":"AANLkTimxbkVBEMhNbieP9ST1CEupFaR=xBxRNssOdLU6@mail.gmail.com","threadId":"25047","inReplyTo":"1283961023-4491-9-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 8/8] tree_entry_interesting(): support negative pathspec","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-09-11T17:33:42Z","receivedAt":"2010-09-11T17:33:42Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\n2010/9/8 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:\n> +\n> +test_expect_success 'diff' '\n> +       cat >expected <<-\\EOF &&\n> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M     one/file\n> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M     one/two/file\n> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M     one/zoo\n> +EOF\n> +       git diff-tree -r HEAD^ HEAD -- one >result &&\n> +       test_cmp expected result\n> +'\n\nHaving the full mode & sha1sum seems to make the test harder to read\nthan necessary.  Perhaps you could use printf or git ls-files together\nwith --name-only to simplify and shorten these a bit?  Example\nalternative versions for three of your testcases:\n\ntest_expect_success 'diff' '\n\tgit ls-files one >expected &&\n\tgit diff --name-only HEAD^ HEAD -- one >result &&\n\ttest_cmp expected result\n'\n\ntest_expect_success 'diff one ^one/two' '\n\tgit ls-files one | grep -v one/two >expected &&\n\tgit diff --name-only HEAD^ HEAD -- one ^one/two >result &&\n\ttest_cmp expected result\n'\n\ntest_expect_success 'diff ^one/two' '\n\tprintf \"file\\none/file\\none/zoo\\n\" >expected &&\n\tgit diff --name-only HEAD^ HEAD -- ^one/two >result &&\n\ttest_cmp expected result\n'\n\nThoughts?\n"},{"id":"150569","messageId":"AANLkTikKtu9Xp1kvHwCzMoPRubLb0VFKTzbgCvF6Sfxf@mail.gmail.com","threadId":"25047","inReplyTo":"AANLkTin_m+zjHND5AwFhkrZM-VEkn70qgCTwpB2B+RA+@mail.gmail.com","subject":"Re: [PATCH 7/8] setup_tree_pathspec(): interpret '^' as negative pathspec","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-13T01:39:10Z","receivedAt":"2010-09-13T01:39:10Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/9/12 Elijah Newren <newren@gmail.com>:\n> 2010/9/8 Nguyễn Thái Ngọc Duy <pclouds@gmail.com>:\n>> This patch does preparation work for tree exclusion in\n>> tree_entry_interesting(). '^' has similar meaning to '!' in\n>> gitexcludes. '!' is not used because bash does not like arguments with\n>> a leading '!'.\n>>\n>> Eventually, \"git diff -- foo ^foo/bar\" should show differences in foo,\n>> except foo/bar. If \"git diff -- ^foo\" is given, then it implies\n>> everything except foo, which could surprise users that\n>> \"bar\" in \"git diff -- bar ^foo\" has no effect at all.\n>\n> I really like the work here.  There are just two things that I think\n> are missing:\n>  * It doesn't handle files with leading carats in their name\n>  * It handles some nested include/exclude cases (e.g. dir\n> ^dir/subdir) but not more complicated ones.\n\nYeah. I originally needed it to compare trees outside narrow area\n(i.e. negating all pathspecs). But I would need a more robust\nimplementation soon when I implement tree widening.\n\n> Note: In the second test, I used:\n>  * \"^funny\" to search for all files EXCEPT \"funny\"\n>  * \"^^funny\" to search for a file named \"^funny\"\n>  * \"^^^funny\" to search for all files EXCEPT \"^funny\"\n> I'm not sure if that's really the syntax we want to adopt, but it\n> should be easy to change if we decide on some other syntax.\n\nAnother way is always treat the leading ^ as negative pathspec. If you\nhave file \"^foo\", specify it with ./^foo. There's still problem with\ntop level entries this way.\n-- \nDuy\n"},{"id":"150695","messageId":"7vr5gw8geb.fsf@alter.siamese.dyndns.org","threadId":"25047","inReplyTo":"1283961023-4491-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/8] tree_entry_interesting(): remove dependency on struct diff_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-14T15:59:56Z","receivedAt":"2010-09-14T15:59:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> This function can be potentially used in more places than just\n> tree-diff.c. \"struct diff_options\" does not make much sense outside\n> diff_tree_sha1().\n>\n> Moreover people seem to be agree that diff machinery should learn\n> proper pathspecs too (i.e. globbing, negative pathspecs...), not just\n> treating pathspecs as tree prefix.\n\nThere is nothing improper about the prefix match.  Perhaps \"richer\"?\nI am not so sure about the need/desirability of negative match, but we\nwill see. \n\nAbout naming.  Where else other than \"tree\" (in the \"hierarchical\nnamespace\" sense) context do you see pathspec?  Does the struct really\nneed to be called TREE_pathspec_list?\n"},{"id":"150697","messageId":"7vmxrk8g9y.fsf@alter.siamese.dyndns.org","threadId":"25047","inReplyTo":"1283961023-4491-6-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 5/8] Add testcases showing how pathspecs are ignored with rev-list --objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-14T16:02:33Z","receivedAt":"2010-09-14T16:02:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As I hinted in the earlier round, no need to \"demonstrate\" the breakage\nand then flip them with s/failure/success/ in this series, as --objects\nwere not designed to be used with pathspecs to begin with.  Just add the\npathspec limiting feature and protect it with a new test.\n"},{"id":"150698","messageId":"7viq288g2z.fsf@alter.siamese.dyndns.org","threadId":"25047","inReplyTo":"1283961023-4491-8-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 7/8] setup_tree_pathspec(): interpret '^' as negative pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-14T16:06:44Z","receivedAt":"2010-09-14T16:06:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> This patch does preparation work for tree exclusion in\n> tree_entry_interesting(). '^' has similar meaning to '!' in\n> gitexcludes. '!' is not used because bash does not like arguments with\n> a leading '!'.\n\nDo not even mention gitexcludes as you have to make awkward excuse like\nthis.\n\nInstead say something like\n\n    '^' works exactly like prefix '^' for revs (e.g. \"log ^maint master\")\n    to mark what is prefixed is excluded.\n"},{"id":"150699","messageId":"7vd3sg8fjn.fsf@alter.siamese.dyndns.org","threadId":"25047","inReplyTo":"1283961023-4491-9-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 8/8] tree_entry_interesting(): support negative pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-14T16:18:20Z","receivedAt":"2010-09-14T16:18:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  t/t9999-test.sh |   44 ++++++++++++++++++++++++++++++++++++++++++++\n\nIf this is about the basic tree traversal logic, that belongs somewhere in\n2xxx series.  You seem to be testing only diff-tree and nothing else, so I\nsuspect that you shouldn't even consume a new test number but just adding\na new test or two to 4013 should suffice.\n\n> +test_expect_success 'diff' '\n> +\tcat >expected <<-\\EOF &&\n\nWhat's the point of the dash in \"<<-\\EOF\" if you are not going to indent\nthe here doc?\n\n> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/file\n> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/two/file\n> +:100644 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 d00491fd7e5bb6fa28c517a0bb32b8b506539d4d M\tone/zoo\n> +EOF\n\nOutside t0xxx series please avoid making the test depend on absolute\nobject names like this.  Since you are using only two blobs, you could\nrun two hash-objects to learn the hash for \"\" and \"1\\n\" and say\n\n\tcat >expected <<-EOF &&\n\t:100644 100644 $none $one M\tone/file\n        ...\n\tEOF\n\nBetter yet, think if you even need the raw output here?  Perhaps using\noutput formats like --name-only or --name-status is more appropriate and\nmake the test more readable?\n\n> diff --git a/tree-walk.c b/tree-walk.c\n> index a2f4a00..fd19681 100644\n> --- a/tree-walk.c\n> +++ b/tree-walk.c\n> @@ -532,7 +532,7 @@ int tree_entry_interesting(const struct name_entry *entry, const char *base, int\n>  \n>  \tpathlen = tree_entry_len(entry->path, entry->sha1);\n>  \n> -\tfor (i = 0; i < ps->nr; i++) {\n> +\tfor (i = ps->nr-1; i >= 0; i--) {\n\nNo explanation in the commit log message why.  Besides, spell binary \"-\"\noperator with a SP each on both ends, please.\n\nAlso the code seems to use a new term subpathspec without defining nor\nexplaining.  Not good.\n"},{"id":"150714","messageId":"AANLkTimHykY5V=C7vmDoA-HCWYLJT_p4MEyfdMqXgB6+@mail.gmail.com","threadId":"25047","inReplyTo":"7vr5gw8geb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/8] tree_entry_interesting(): remove dependency on struct diff_options","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-14T22:33:41Z","receivedAt":"2010-09-14T22:33:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/9/15 Junio C Hamano <gitster@pobox.com>:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> This function can be potentially used in more places than just\n>> tree-diff.c. \"struct diff_options\" does not make much sense outside\n>> diff_tree_sha1().\n>>\n>> Moreover people seem to be agree that diff machinery should learn\n>> proper pathspecs too (i.e. globbing, negative pathspecs...), not just\n>> treating pathspecs as tree prefix.\n>\n> There is nothing improper about the prefix match.  Perhaps \"richer\"?\n> I am not so sure about the need/desirability of negative match, but we\n> will see.\n\nNegative match has been mentioned twice recently, once in Elijah's\nsparse clone [1] and once by a user [2].\n\n[1] http://mid.gmane.org/AANLkTikJhSVJw2hXkp0j6XA+k-J-AtSYzKWumjnqqsgz@mail.gmail.com\n\n[2] http://mid.gmane.org/AANLkTikUadS+tj3ARdRqo=PSBVhTsJaUxaJv+=4hMf4s@mail.gmail.com\n\n>\n> About naming.  Where else other than \"tree\" (in the \"hierarchical\n> namespace\" sense) context do you see pathspec?  Does the struct really\n> need to be called TREE_pathspec_list?\n\nPathspecs are usually stored in a list, const char ** and I don't want\nto take the generic name \"pathspec_list\", unless we convert all to use\nthis struct. Any suggestions of other names?\n-- \nDuy\n"},{"id":"150715","messageId":"AANLkTi=i4P=Xa+PPxp1aLdJOSA7cTptA+KnMsQwjSth1@mail.gmail.com","threadId":"25047","inReplyTo":"7viq288g2z.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/8] setup_tree_pathspec(): interpret '^' as negative pathspec","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-14T22:41:46Z","receivedAt":"2010-09-14T22:41:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/9/15 Junio C Hamano <gitster@pobox.com>:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> This patch does preparation work for tree exclusion in\n>> tree_entry_interesting(). '^' has similar meaning to '!' in\n>> gitexcludes. '!' is not used because bash does not like arguments with\n>> a leading '!'.\n>\n> Do not even mention gitexcludes as you have to make awkward excuse like\n> this.\n>\n> Instead say something like\n>\n>    '^' works exactly like prefix '^' for revs (e.g. \"log ^maint master\")\n>    to mark what is prefixed is excluded.\n\nIt was not thought through. The way this patch does means\nget_pathspec() will need to be updated to handle the case \"(in\nsubdir)$ git diff ^foo\", which has to be turned to \"(in toplevel)$ git\ndiff ^subdir/foo\", not \"(in toplevel)$ git diff subdir/^foo\".\n\nA better approach would be separate \"^\" out as an argument by itself,\ni.e. \"git diff ^ foo\". Less work in get_pathspec(), and tab-comletion\non path also works. But now as '^' stands alone, '!' can be used too\n(no more bash history expansion). And I think '!' is more intuitive\nthan '^'.\n-- \nDuy\n"},{"id":"150716","messageId":"AANLkTinkGpWGXTEpDqm4zPRky06C9X9xUoo2roKH2kG=@mail.gmail.com","threadId":"25047","inReplyTo":"7vd3sg8fjn.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 8/8] tree_entry_interesting(): support negative pathspec","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-14T22:46:26Z","receivedAt":"2010-09-14T22:46:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/9/15 Junio C Hamano <gitster@pobox.com>:\n>> diff --git a/tree-walk.c b/tree-walk.c\n>> index a2f4a00..fd19681 100644\n>> --- a/tree-walk.c\n>> +++ b/tree-walk.c\n>> @@ -532,7 +532,7 @@ int tree_entry_interesting(const struct name_entry *entry, const char *base, int\n>>\n>>       pathlen = tree_entry_len(entry->path, entry->sha1);\n>>\n>> -     for (i = 0; i < ps->nr; i++) {\n>> +     for (i = ps->nr-1; i >= 0; i--) {\n>\n> No explanation in the commit log message why.  Besides, spell binary \"-\"\n> operator with a SP each on both ends, please.\n>\n> Also the code seems to use a new term subpathspec without defining nor\n> explaining.  Not good.\n\nNo, not good. It also has bugs as Elijah pointed out. I'm going to\nsubmit these last two patches as a separate series that will make\nnegative match work on (hopefully) all commands.\n\nThanks for your comments.\n-- \nDuy\n"},{"id":"150718","messageId":"7veicv7w06.fsf@alter.siamese.dyndns.org","threadId":"25047","inReplyTo":"AANLkTimHykY5V=C7vmDoA-HCWYLJT_p4MEyfdMqXgB6+@mail.gmail.com","subject":"Re: [PATCH 3/8] tree_entry_interesting(): remove dependency on struct diff_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-09-14T23:20:25Z","receivedAt":"2010-09-14T23:20:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n>> About naming.  Where else other than \"tree\" (in the \"hierarchical\n>> namespace\" sense) context do you see pathspec?  Does the struct really\n>> need to be called TREE_pathspec_list?\n>\n> Pathspecs are usually stored in a list, const char ** and I don't want\n> to take the generic name \"pathspec_list\", unless we convert all to use\n> this struct. Any suggestions of other names?\n\nI was hoping [*1*] that in the longer term we would have a unified\nmachinery to handle pathspecs across ls-tree, ls-files, diff-tree,\ndiff-files and grep.  These commands more or less share the same idea of\nwhat purpose a pathspec serves, but take quite different codepaths, and\nthe biggest problem is that some know globbing while others don't.\n\nThe problem arising from the two semantics [*2*] is visible when you run\n\"git add\" with pathspec where add_files_to_cache() updates the index only\nat changed paths (found using diff-files machinery, that implements \"diff\"\nfamily of pathspec semantics to match only directory prefix) and then\nadd_files() adds the paths that are untracked (found using the ls-files\nmachinery, that knows about globbing).\n\nOnce we have a unified machinery to handle pathspec, the data structure\nthat holds the pathspec should naturally be called \"struct pathspec\",\nwhile an element on that list would be \"struct pathspec_elem\" or perhaps\n\"struct pathspec_pattern\" [*3*].\n\nI would imagine that \"struct exclude\" (in dir.[ch]) that is contained in\n\"struct exclude_list\" might be a good place to start from, in the sense\nthat it shows how a match pattern can be pre-parsed to optimize the\nmatching operation [*4*].  It however may not know about one particular\nkind of optimization that is essential when dealing with tree objects: the\nability to ask \"is this subtree worth descending into?\".  That logic is\nnecessary to avoid opening unnecessary tree objects in diff-tree and grep.\n\n\n[Footnote]\n\n*1* https://git.wiki.kernel.org/index.php/SoC2010Ideas#Unify_Pathspec_Semantics\n\n*2* As the maintainer, I do consider it also is a problem that each\nof these semantics has multiple codepaths to implement it, but that is a\nsecondary issue.  Having two semantics is visible to the end user and is a\nbigger problem.\n\n*3* I personally tend to consider the whole set as _a_ \"path specification\"\neven when you give more than one patterns to match on the command line,\nbut it may be just me.  I am Ok with \"struct pathspec_set\" that holds a\nset of \"struct pathspec\", too.  Unlike exclude-list where the order of\nelements in it has a meaning, the matching patterns in a pathspec are\nunordered, so even if we may end up implementing it as a list, it would be\nincorrect to call that \"struct pathspec_list\".\n\n*4* \"struct exclude_list\" is somewhat special in that it is a dynamic data\nsource where you learn more matching rules as you dig deeper.  We do not\nneed that aspect of the dir.[ch] codepath for unified pathspec handling.\n"}]}