{"thread":{"id":"26701","subject":"[WIP PATCH 0/5] support --exclude for diff/log commands","startedAt":"2011-03-10T03:13:34Z","lastAt":"2011-03-23T13:34:52Z","messageCount":20,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"163103","messageId":"1299726819-5576-1-git-send-email-pclouds@gmail.com","threadId":"26701","inReplyTo":null,"subject":"[WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T03:13:34Z","receivedAt":"2011-03-10T03:13:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is not really ready for review or use, but I'd like to have\nfeedback if any, where this series is heading because it requires some\ncode refactor in diff/rev machinery. It does not feel absolutely right\nto me.\n\nOn the other hand, if we go with ':' as a mark of special pathspecs, then\n\n - \":/\" mark pathspecs relative to worktree root\n - \":!\" may mark negative pathspecs\n\nLess changes in diff/rev this way.\n\nNguyễn Thái Ngọc Duy (5):\n  tree-walk: support negative pathspec\n  match_pathspec_depth: support negative pathspec\n\nmatch_pathspec_depth needs work. tree_entry_interesting() can use some\noptimization, but leave it for now.\n\n  revision.c: get rid of struct rev_info.prune_data\n  diff: refactor init/release API\n\nThese two facilitate post pathspec manipulation to transform --exclude\nto pathspecs with to_exclude = 1.\n\n  diff: support --exclude\n\n Documentation/technical/api-diff.txt |    2 +-\n builtin/add.c                        |    3 +-\n builtin/blame.c                      |   22 +++++----------\n builtin/diff.c                       |    6 ++--\n builtin/fast-export.c                |    2 +-\n builtin/merge.c                      |    2 +-\n builtin/reset.c                      |    4 +-\n cache.h                              |    1 +\n diff-lib.c                           |    6 ++--\n diff-no-index.c                      |    6 ++--\n diff.c                               |   49 ++++++++++++++++++++++++++++++++-\n diff.h                               |    6 ++--\n dir.c                                |    5 +++\n merge-recursive.c                    |    2 +-\n notes-merge.c                        |    8 +++---\n patch-ids.c                          |    2 +-\n revision.c                           |   22 ++++++++-------\n revision.h                           |    1 -\n tree-diff.c                          |   21 +++------------\n tree-walk.c                          |   37 +++++++++++++++++++------\n wt-status.c                          |    6 +---\n 21 files changed, 130 insertions(+), 83 deletions(-)\n\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"163108","messageId":"1299726819-5576-2-git-send-email-pclouds@gmail.com","threadId":"26701","inReplyTo":"1299726819-5576-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/5] tree-walk: support negative pathspec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T03:13:35Z","receivedAt":"2011-03-10T03:13:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"While the UI will be simple, where negative pathspecs are always\nappended in the end, this could should be able to handle a mixed set\nof negative and positive pathspecs (ie. overlapping must be handled\ncorrectly).\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n cache.h     |    1 +\n tree-walk.c |   37 ++++++++++++++++++++++++++++---------\n 2 files changed, 29 insertions(+), 9 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 08a9022..2189366 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -510,6 +510,7 @@ struct pathspec {\n \t\tconst char *match;\n \t\tint len;\n \t\tint has_wildcard:1;\n+\t\tint to_exclude:1;\n \t} *items;\n };\n \ndiff --git a/tree-walk.c b/tree-walk.c\nindex 322becc..acbee1b 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -560,6 +560,7 @@ int tree_entry_interesting(const struct name_entry *entry,\n \tint i;\n \tint pathlen, baselen = base->len - base_offset;\n \tint never_interesting = ps->has_wildcard ? 0 : -1;\n+\tint all_interesting_ok = !ps->recursive || ps->max_depth == -1;\n \n \tif (!ps->nr) {\n \t\tif (!ps->recursive || ps->max_depth == -1)\n@@ -569,6 +570,19 @@ int tree_entry_interesting(const struct name_entry *entry,\n \t\t\t\t      ps->max_depth);\n \t}\n \n+\t/*\n+\t * TODO: add prepare_pathspec() to scan for pathspecs\n+\t * overlapped with negative ones. The ones that do not overlap\n+\t * can still return 2.\n+\t */\n+\tfor (i = 0; i < ps->nr; i++) {\n+\t\tconst struct pathspec_item *item = ps->items+i;\n+\t\tif (item->to_exclude) {\n+\t\t\tall_interesting_ok = 0;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\n \tpathlen = tree_entry_len(entry->path, entry->sha1);\n \n \tfor (i = ps->nr - 1; i >= 0; i--) {\n@@ -578,17 +592,22 @@ int tree_entry_interesting(const struct name_entry *entry,\n \t\tint matchlen = item->len;\n \n \t\tif (baselen >= matchlen) {\n+\t\t\tint ret;\n+\n \t\t\t/* If it doesn't match, move along... */\n \t\t\tif (!match_dir_prefix(base_str, baselen, match, matchlen))\n \t\t\t\tgoto match_wildcards;\n \n-\t\t\tif (!ps->recursive || ps->max_depth == -1)\n+\t\t\tif (all_interesting_ok)\n \t\t\t\treturn 2;\n \n-\t\t\treturn !!within_depth(base_str + matchlen + 1,\n-\t\t\t\t\t      baselen - matchlen - 1,\n-\t\t\t\t\t      !!S_ISDIR(entry->mode),\n-\t\t\t\t\t      ps->max_depth);\n+\t\t\tret = !!within_depth(base_str + matchlen + 1,\n+\t\t\t\t\t     baselen - matchlen - 1,\n+\t\t\t\t\t     !!S_ISDIR(entry->mode),\n+\t\t\t\t\t     ps->max_depth);\n+\t\t\tif (item->to_exclude)\n+\t\t\t\tret = !ret;\n+\t\t\treturn ret;\n \t\t}\n \n \t\t/* Does the base match? */\n@@ -596,18 +615,18 @@ int tree_entry_interesting(const struct name_entry *entry,\n \t\t\tif (match_entry(entry, pathlen,\n \t\t\t\t\tmatch + baselen, matchlen - baselen,\n \t\t\t\t\t&never_interesting))\n-\t\t\t\treturn 1;\n+\t\t\t\treturn item->to_exclude ? 0 : 1;\n \n \t\t\tif (ps->items[i].has_wildcard) {\n \t\t\t\tif (!fnmatch(match + baselen, entry->path, 0))\n-\t\t\t\t\treturn 1;\n+\t\t\t\t\treturn item->to_exclude ? 0 : 1;\n \n \t\t\t\t/*\n \t\t\t\t * Match all directories. We'll try to\n \t\t\t\t * match files later on.\n \t\t\t\t */\n \t\t\t\tif (ps->recursive && S_ISDIR(entry->mode))\n-\t\t\t\t\treturn 1;\n+\t\t\t\t\treturn item->to_exclude ? 0 : 1;\n \t\t\t}\n \n \t\t\tcontinue;\n@@ -626,7 +645,7 @@ match_wildcards:\n \n \t\tif (!fnmatch(match, base->buf + base_offset, 0)) {\n \t\t\tstrbuf_setlen(base, base_offset + baselen);\n-\t\t\treturn 1;\n+\t\t\treturn item->to_exclude ? 0 : 1;\n \t\t}\n \t\tstrbuf_setlen(base, base_offset + baselen);\n \n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"163104","messageId":"1299726819-5576-3-git-send-email-pclouds@gmail.com","threadId":"26701","inReplyTo":"1299726819-5576-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/5] match_pathspec_depth: support negative pathspec","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T03:13:36Z","receivedAt":"2011-03-10T03:13:36Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The return value of this function is bugging me, as well as seen[]\narray. Not really sure how to do with them when a negative pathspec is\nmatched.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n dir.c |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 168dad6..75c4a8f 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -279,6 +279,11 @@ int match_pathspec_depth(const struct pathspec *ps,\n \t\t\t\thow = 0;\n \t\t}\n \t\tif (how) {\n+\t\t\tif (ps->items[i].to_exclude) {\n+\t\t\t\tif (seen && seen[i] < how)\n+\t\t\t\t\tseen[i] = how;\n+\t\t\t\treturn 0;\n+\t\t\t}\n \t\t\tif (retval < how)\n \t\t\t\tretval = how;\n \t\t\tif (seen && seen[i] < how)\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"163105","messageId":"1299726819-5576-4-git-send-email-pclouds@gmail.com","threadId":"26701","inReplyTo":"1299726819-5576-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 3/5] revision.c: get rid of struct rev_info.prune_data","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T03:13:37Z","receivedAt":"2011-03-10T03:13:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"There are three struct pathspec(s) in struct rev_info:\n\n - prune_data\n - pruning.pathspec\n - diffopt.pathspec\n\nIn some places, this pathspec is used, in other places another\none. I'd like to have only one pathspec in struct rev_info, but i'm\nnot sure why diffopt can't be used in place of pruning. So in the\nmeantime, prune_data will be gone.\n\nThe driving force behind this is now diff_options.pathspec can be\nmanipulated behind the scene by diff_setup_done() to support\n--exclude. Rev machinery does not know about this and does not need\nto. Remove prune_data in favor of diffopt.pathspec, handing complex\npathspec manipulation to diff machinery.\n\nAs part of the changes, setup_revisions() now can take pathspecs\ndirectly by setting argc = 0, argv = pathspec\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n builtin/add.c         |    3 +--\n builtin/diff.c        |    6 +++---\n builtin/fast-export.c |    2 +-\n diff-lib.c            |    6 +++---\n revision.c            |   20 +++++++++++---------\n revision.h            |    1 -\n wt-status.c           |    6 ++----\n 7 files changed, 21 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/add.c b/builtin/add.c\nindex f7a17e4..904db6b 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -85,8 +85,7 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n \tstruct update_callback_data data;\n \tstruct rev_info rev;\n \tinit_revisions(&rev, prefix);\n-\tsetup_revisions(0, NULL, &rev, NULL);\n-\tinit_pathspec(&rev.prune_data, pathspec);\n+\tsetup_revisions(0, pathspec, &rev, NULL);\n \trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = update_callback;\n \tdata.flags = flags;\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex 4c9deb2..1c69da2 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -374,10 +374,10 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\t}\n \t\tdie(\"unhandled object '%s' given.\", name);\n \t}\n-\tif (rev.prune_data.nr) {\n+\tif (rev.diffopt.pathspec.nr) {\n \t\tif (!path)\n-\t\t\tpath = rev.prune_data.items[0].match;\n-\t\tpaths += rev.prune_data.nr;\n+\t\t\tpath = rev.diffopt.pathspec.items[0].match;\n+\t\tpaths += rev.diffopt.pathspec.nr;\n \t}\n \n \t/*\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex daf1945..b225427 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -651,7 +651,7 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \tif (import_filename)\n \t\timport_marks(import_filename);\n \n-\tif (import_filename && revs.prune_data.nr)\n+\tif (import_filename && revs.diffopt.pathspec.nr)\n \t\tfull_tree = 1;\n \n \tget_tags_and_duplicates(&revs.pending, &extra_refs);\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 1e22992..b3ac269 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -106,7 +106,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\tDIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))\n \t\t\tbreak;\n \n-\t\tif (!ce_path_match(ce, &revs->prune_data))\n+\t\tif (!ce_path_match(ce, &revs->diffopt.pathspec))\n \t\t\tcontinue;\n \n \t\tif (ce_stage(ce)) {\n@@ -427,7 +427,7 @@ static int oneway_diff(struct cache_entry **src, struct unpack_trees_options *o)\n \tif (tree == o->df_conflict_entry)\n \t\ttree = NULL;\n \n-\tif (ce_path_match(idx ? idx : tree, &revs->prune_data))\n+\tif (ce_path_match(idx ? idx : tree, &revs->diffopt.pathspec))\n \t\tdo_oneway_diff(o, idx, tree);\n \n \treturn 0;\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-\tinit_pathspec(&revs.prune_data, opt->pathspec.raw);\n+\tinit_pathspec(&revs.diffopt.pathspec, opt->pathspec.raw);\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/revision.c b/revision.c\nindex 86d2470..f9f66de 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -323,7 +323,7 @@ static int rev_compare_tree(struct rev_info *revs, struct commit *parent, struct\n \t\t * tagged commit by specifying both --simplify-by-decoration\n \t\t * and pathspec.\n \t\t */\n-\t\tif (!revs->prune_data.nr)\n+\t\tif (!revs->diffopt.pathspec.nr)\n \t\t\treturn REV_TREE_SAME;\n \t}\n \n@@ -969,7 +969,7 @@ static void prepare_show_merge(struct rev_info *revs)\n \t\tstruct cache_entry *ce = active_cache[i];\n \t\tif (!ce_stage(ce))\n \t\t\tcontinue;\n-\t\tif (ce_path_match(ce, &revs->prune_data)) {\n+\t\tif (ce_path_match(ce, &revs->diffopt.pathspec)) {\n \t\t\tprune_num++;\n \t\t\tprune = xrealloc(prune, sizeof(*prune) * prune_num);\n \t\t\tprune[prune_num-2] = ce->name;\n@@ -979,8 +979,8 @@ static void prepare_show_merge(struct rev_info *revs)\n \t\t       ce_same_name(ce, active_cache[i+1]))\n \t\t\ti++;\n \t}\n-\tfree_pathspec(&revs->prune_data);\n-\tinit_pathspec(&revs->prune_data, prune);\n+\tfree_pathspec(&revs->diffopt.pathspec);\n+\tinit_pathspec(&revs->diffopt.pathspec, prune);\n \trevs->limited = 1;\n }\n \n@@ -1488,6 +1488,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (opt)\n \t\tsubmodule = opt->submodule;\n \n+\t/* argv is a list of pathspec */\n+\tif (!argc && argv)\n+\t\tprune_data = argv;\n+\n \t/* First, search for \"--\" */\n \tseen_dashdash = 0;\n \tfor (i = 1; i < argc; i++) {\n@@ -1617,7 +1621,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t}\n \n \tif (prune_data)\n-\t\tinit_pathspec(&revs->prune_data, get_pathspec(revs->prefix, prune_data));\n+\t\tinit_pathspec(&revs->diffopt.pathspec, get_pathspec(revs->prefix, prune_data));\n \n \tif (revs->def == NULL)\n \t\trevs->def = opt ? opt->def : NULL;\n@@ -1648,19 +1652,17 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tif (revs->topo_order)\n \t\trevs->limited = 1;\n \n-\tif (revs->prune_data.nr) {\n-\t\tdiff_tree_setup_paths(revs->prune_data.raw, &revs->pruning);\n+\tif (revs->diffopt.pathspec.nr) {\n \t\t/* Can't prune commits with rename following: the paths change.. */\n \t\tif (!DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))\n \t\t\trevs->prune = 1;\n-\t\tif (!revs->full_diff)\n-\t\t\tdiff_tree_setup_paths(revs->prune_data.raw, &revs->diffopt);\n \t}\n \tif (revs->combine_merges)\n \t\trevs->ignore_merges = 0;\n \trevs->diffopt.abbrev = revs->abbrev;\n \tif (diff_setup_done(&revs->diffopt) < 0)\n \t\tdie(\"diff_setup_done failed\");\n+\tdiff_tree_setup_paths(revs->diffopt.pathspec.raw, &revs->pruning);\n \n \tcompile_grep_patterns(&revs->grep_filter);\n \ndiff --git a/revision.h b/revision.h\nindex 82509dd..066e407 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -34,7 +34,6 @@ struct rev_info {\n \t/* Basic information */\n \tconst char *prefix;\n \tconst char *def;\n-\tstruct pathspec prune_data;\n \tunsigned int early_output;\n \n \t/* Traversal flags */\ndiff --git a/wt-status.c b/wt-status.c\nindex a82b11d..7c4171e 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -312,7 +312,7 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)\n \tstruct rev_info rev;\n \n \tinit_revisions(&rev, NULL);\n-\tsetup_revisions(0, NULL, &rev, NULL);\n+\tsetup_revisions(0, s->pathspec, &rev, NULL);\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n \tDIFF_OPT_SET(&rev.diffopt, DIRTY_SUBMODULES);\n \tif (!s->show_untracked_files)\n@@ -323,7 +323,6 @@ static void wt_status_collect_changes_worktree(struct wt_status *s)\n     }\n \trev.diffopt.format_callback = wt_status_collect_changed_cb;\n \trev.diffopt.format_callback_data = s;\n-\tinit_pathspec(&rev.prune_data, s->pathspec);\n \trun_diff_files(&rev, 0);\n }\n \n@@ -335,7 +334,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \tinit_revisions(&rev, NULL);\n \tmemset(&opt, 0, sizeof(opt));\n \topt.def = s->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference;\n-\tsetup_revisions(0, NULL, &rev, &opt);\n+\tsetup_revisions(0, s->pathspec, &rev, &opt);\n \n \tif (s->ignore_submodule_arg) {\n \t\tDIFF_OPT_SET(&rev.diffopt, OVERRIDE_SUBMODULE_CONFIG);\n@@ -348,7 +347,6 @@ static void wt_status_collect_changes_index(struct wt_status *s)\n \trev.diffopt.detect_rename = 1;\n \trev.diffopt.rename_limit = 200;\n \trev.diffopt.break_opt = 0;\n-\tinit_pathspec(&rev.prune_data, s->pathspec);\n \trun_diff_index(&rev, 1);\n }\n \n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"163106","messageId":"1299726819-5576-5-git-send-email-pclouds@gmail.com","threadId":"26701","inReplyTo":"1299726819-5576-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 4/5] diff: refactor init/release API","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T03:13:38Z","receivedAt":"2011-03-10T03:13:38Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This patch:\n\n - renames diff_setup() to init_diff()\n - gets rid of diff_tree_setup_paths()\n - renames diff_tree_release_paths() to release_diff()\n\nThe first one makes it probably just personal taste. I find \"init\"\nbetter name for where input can be garbage, as opposed to \"setup\",\nwhere the input is at least sane.\n\nrelease_diff() is supposed to take care of more cleanup stuff as\nstruct diff_options grows.\n\ndiff_tree_release_paths() is not just a simple wrapper around\ninit_pathspec(). Remove it and manipulate diff_options->pathspec\ndirectly.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n Documentation/technical/api-diff.txt |    2 +-\n builtin/blame.c                      |   22 +++++++---------------\n builtin/merge.c                      |    2 +-\n builtin/reset.c                      |    4 ++--\n diff-no-index.c                      |    6 +++---\n diff.c                               |    8 +++++++-\n diff.h                               |    5 ++---\n merge-recursive.c                    |    2 +-\n notes-merge.c                        |    8 ++++----\n patch-ids.c                          |    2 +-\n revision.c                           |    4 ++--\n tree-diff.c                          |   21 ++++-----------------\n 12 files changed, 35 insertions(+), 51 deletions(-)\n\ndiff --git a/Documentation/technical/api-diff.txt b/Documentation/technical/api-diff.txt\nindex 20b0241..b88ee9e 100644\n--- a/Documentation/technical/api-diff.txt\n+++ b/Documentation/technical/api-diff.txt\n@@ -18,7 +18,7 @@ Calling sequence\n ----------------\n \n * Prepare `struct diff_options` to record the set of diff options, and\n-  then call `diff_setup()` to initialize this structure.  This sets up\n+  then call `init_diff()` to initialize this structure.  This sets up\n   the vanilla default.\n \n * Fill in the options structure to specify desired output format, rename\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex aa30ec5..f58e6e4 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -383,14 +383,13 @@ static struct origin *find_origin(struct scoreboard *sb,\n \t * and origin first.  Most of the time they are the\n \t * same and diff-tree is fairly efficient about this.\n \t */\n-\tdiff_setup(&diff_opts);\n+\tinit_diff(&diff_opts);\n \tDIFF_OPT_SET(&diff_opts, RECURSIVE);\n \tdiff_opts.detect_rename = 0;\n \tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tpaths[0] = origin->path;\n \tpaths[1] = NULL;\n-\n-\tdiff_tree_setup_paths(paths, &diff_opts);\n+\tinit_pathspec(&diff_opts.pathspec, paths);\n \tif (diff_setup_done(&diff_opts) < 0)\n \t\tdie(\"diff-setup\");\n \n@@ -441,7 +440,7 @@ static struct origin *find_origin(struct scoreboard *sb,\n \t\t}\n \t}\n \tdiff_flush(&diff_opts);\n-\tdiff_tree_release_paths(&diff_opts);\n+\trelease_diff(&diff_opts);\n \tif (porigin) {\n \t\t/*\n \t\t * Create a freestanding copy that is not part of\n@@ -469,15 +468,12 @@ static struct origin *find_rename(struct scoreboard *sb,\n \tstruct origin *porigin = NULL;\n \tstruct diff_options diff_opts;\n \tint i;\n-\tconst char *paths[2];\n \n-\tdiff_setup(&diff_opts);\n+\tinit_diff(&diff_opts);\n \tDIFF_OPT_SET(&diff_opts, RECURSIVE);\n \tdiff_opts.detect_rename = DIFF_DETECT_RENAME;\n \tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tdiff_opts.single_follow = origin->path;\n-\tpaths[0] = NULL;\n-\tdiff_tree_setup_paths(paths, &diff_opts);\n \tif (diff_setup_done(&diff_opts) < 0)\n \t\tdie(\"diff-setup\");\n \n@@ -500,7 +496,7 @@ static struct origin *find_rename(struct scoreboard *sb,\n \t\t}\n \t}\n \tdiff_flush(&diff_opts);\n-\tdiff_tree_release_paths(&diff_opts);\n+\trelease_diff(&diff_opts);\n \treturn porigin;\n }\n \n@@ -1048,7 +1044,6 @@ static int find_copy_in_parent(struct scoreboard *sb,\n \t\t\t       int opt)\n {\n \tstruct diff_options diff_opts;\n-\tconst char *paths[1];\n \tint i, j;\n \tint retval;\n \tstruct blame_list *blame_list;\n@@ -1058,12 +1053,9 @@ static int find_copy_in_parent(struct scoreboard *sb,\n \tif (!blame_list)\n \t\treturn 1; /* nothing remains for this target */\n \n-\tdiff_setup(&diff_opts);\n+\tinit_diff(&diff_opts);\n \tDIFF_OPT_SET(&diff_opts, RECURSIVE);\n \tdiff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;\n-\n-\tpaths[0] = NULL;\n-\tdiff_tree_setup_paths(paths, &diff_opts);\n \tif (diff_setup_done(&diff_opts) < 0)\n \t\tdie(\"diff-setup\");\n \n@@ -1147,7 +1139,7 @@ static int find_copy_in_parent(struct scoreboard *sb,\n \t}\n \treset_scanned_flag(sb);\n \tdiff_flush(&diff_opts);\n-\tdiff_tree_release_paths(&diff_opts);\n+\trelease_diff(&diff_opts);\n \treturn retval;\n }\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex a89ddbb..c74ed1f 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -379,7 +379,7 @@ static void finish(const unsigned char *new_head, const char *msg)\n \t}\n \tif (new_head && show_diffstat) {\n \t\tstruct diff_options opts;\n-\t\tdiff_setup(&opts);\n+\t\tinit_diff(&opts);\n \t\topts.output_format |=\n \t\t\tDIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;\n \t\topts.detect_rename = DIFF_DETECT_RENAME;\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 5de2bce..36b0605 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -195,10 +195,10 @@ static int read_from_tree(const char *prefix, const char **argv,\n \tstruct diff_options opt;\n \n \tmemset(&opt, 0, sizeof(opt));\n-\tdiff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);\n \topt.output_format = DIFF_FORMAT_CALLBACK;\n \topt.format_callback = update_index_from_diff;\n \topt.format_callback_data = &index_was_discarded;\n+\tinit_pathspec(&opt.pathspec, get_pathspec(prefix, (const char **)argv));\n \n \tindex_fd = hold_locked_index(lock, 1);\n \tindex_was_discarded = 0;\n@@ -207,7 +207,7 @@ static int read_from_tree(const char *prefix, const char **argv,\n \t\treturn 1;\n \tdiffcore_std(&opt);\n \tdiff_flush(&opt);\n-\tdiff_tree_release_paths(&opt);\n+\trelease_diff(&opt);\n \n \tif (!index_was_discarded)\n \t\t/* The index is still clobbered from do_diff_cache() */\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 3a36144..a60eda7 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -203,7 +203,7 @@ void diff_no_index(struct rev_info *revs,\n \t\tusagef(\"git diff %s <path> <path>\",\n \t\t       no_index ? \"--no-index\" : \"[--no-index]\");\n \n-\tdiff_setup(&revs->diffopt);\n+\tinit_diff(&revs->diffopt);\n \tfor (i = 1; i < argc - 2; ) {\n \t\tint j;\n \t\tif (!strcmp(argv[i], \"--no-index\"))\n@@ -245,10 +245,10 @@ void diff_no_index(struct rev_info *revs,\n \t\t\t     : p);\n \t\t\tpaths[i] = p;\n \t\t}\n-\t\tdiff_tree_setup_paths(paths, &revs->diffopt);\n+\t\tinit_pathspec(&revs->diffopt.pathspec, paths);\n \t}\n \telse\n-\t\tdiff_tree_setup_paths(argv + argc - 2, &revs->diffopt);\n+\t\tinit_pathspec(&revs->diffopt.pathspec, argv + argc - 2);\n \trevs->diffopt.skip_stat_unmatch = 1;\n \tif (!revs->diffopt.output_format)\n \t\trevs->diffopt.output_format = DIFF_FORMAT_PATCH;\ndiff --git a/diff.c b/diff.c\nindex 6640857..6f206a9 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2847,7 +2847,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)\n \tbuiltin_checkdiff(name, other, attr_path, p->one, p->two, o);\n }\n \n-void diff_setup(struct diff_options *options)\n+void init_diff(struct diff_options *options)\n {\n \tmemcpy(options, &default_diff_options, sizeof(*options));\n \n@@ -2976,6 +2976,12 @@ int diff_setup_done(struct diff_options *options)\n \treturn 0;\n }\n \n+\n+void release_diff(struct diff_options *o)\n+{\n+\tfree_pathspec(&o->pathspec);\n+}\n+\n static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)\n {\n \tchar c, *eq;\ndiff --git a/diff.h b/diff.h\nindex 310bd6b..12a9907 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -160,8 +160,6 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix);\n \n extern const char mime_boundary_leader[];\n \n-extern void diff_tree_setup_paths(const char **paths, struct diff_options *);\n-extern void diff_tree_release_paths(struct diff_options *);\n extern int diff_tree(struct tree_desc *t1, struct tree_desc *t2,\n \t\t     const char *base, struct diff_options *opt);\n extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,\n@@ -226,9 +224,10 @@ extern int parse_long_opt(const char *opt, const char **argv,\n extern int git_diff_basic_config(const char *var, const char *value, void *cb);\n extern int git_diff_ui_config(const char *var, const char *value, void *cb);\n extern int diff_use_color_default;\n-extern void diff_setup(struct diff_options *);\n+extern void init_diff(struct diff_options *);\n extern int diff_opt_parse(struct diff_options *, const char **, int);\n extern int diff_setup_done(struct diff_options *);\n+extern void release_diff(struct diff_options *);\n \n #define DIFF_DETECT_RENAME\t1\n #define DIFF_DETECT_COPY\t2\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 16c2dbe..bbf48c9 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -429,7 +429,7 @@ static struct string_list *get_renames(struct merge_options *o,\n \tstruct diff_options opts;\n \n \trenames = xcalloc(1, sizeof(struct string_list));\n-\tdiff_setup(&opts);\n+\tinit_diff(&opts);\n \tDIFF_OPT_SET(&opts, RECURSIVE);\n \topts.detect_rename = DIFF_DETECT_RENAME;\n \topts.rename_limit = o->merge_rename_limit >= 0 ? o->merge_rename_limit :\ndiff --git a/notes-merge.c b/notes-merge.c\nindex 1467ad3..acf98e5 100644\n--- a/notes-merge.c\n+++ b/notes-merge.c\n@@ -131,7 +131,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,\n \ttrace_printf(\"\\tdiff_tree_remote(base = %.7s, remote = %.7s)\\n\",\n \t       sha1_to_hex(base), sha1_to_hex(remote));\n \n-\tdiff_setup(&opt);\n+\tinit_diff(&opt);\n \tDIFF_OPT_SET(&opt, RECURSIVE);\n \topt.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opt) < 0)\n@@ -178,7 +178,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,\n \t\t       sha1_to_hex(mp->remote));\n \t}\n \tdiff_flush(&opt);\n-\tdiff_tree_release_paths(&opt);\n+\trelease_diff(&opt);\n \n \t*num_changes = len;\n \treturn changes;\n@@ -195,7 +195,7 @@ static void diff_tree_local(struct notes_merge_options *o,\n \ttrace_printf(\"\\tdiff_tree_local(len = %i, base = %.7s, local = %.7s)\\n\",\n \t       len, sha1_to_hex(base), sha1_to_hex(local));\n \n-\tdiff_setup(&opt);\n+\tinit_diff(&opt);\n \tDIFF_OPT_SET(&opt, RECURSIVE);\n \topt.output_format = DIFF_FORMAT_NO_OUTPUT;\n \tif (diff_setup_done(&opt) < 0)\n@@ -265,7 +265,7 @@ static void diff_tree_local(struct notes_merge_options *o,\n \t\t       sha1_to_hex(mp->local));\n \t}\n \tdiff_flush(&opt);\n-\tdiff_tree_release_paths(&opt);\n+\trelease_diff(&opt);\n }\n \n static void check_notes_merge_worktree(struct notes_merge_options *o)\ndiff --git a/patch-ids.c b/patch-ids.c\nindex 5717257..e8ff156 100644\n--- a/patch-ids.c\n+++ b/patch-ids.c\n@@ -37,7 +37,7 @@ struct patch_id_bucket {\n int init_patch_ids(struct patch_ids *ids)\n {\n \tmemset(ids, 0, sizeof(*ids));\n-\tdiff_setup(&ids->diffopts);\n+\tinit_diff(&ids->diffopts);\n \tDIFF_OPT_SET(&ids->diffopts, RECURSIVE);\n \tif (diff_setup_done(&ids->diffopts) < 0)\n \t\treturn error(\"diff_setup_done failed\");\ndiff --git a/revision.c b/revision.c\nindex f9f66de..014b723 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -925,7 +925,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)\n \trevs->grep_filter.header_tail = &(revs->grep_filter.header_list);\n \trevs->grep_filter.regflags = REG_NEWLINE;\n \n-\tdiff_setup(&revs->diffopt);\n+\tinit_diff(&revs->diffopt);\n \tif (prefix && !revs->diffopt.prefix) {\n \t\trevs->diffopt.prefix = prefix;\n \t\trevs->diffopt.prefix_length = strlen(prefix);\n@@ -1662,7 +1662,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \trevs->diffopt.abbrev = revs->abbrev;\n \tif (diff_setup_done(&revs->diffopt) < 0)\n \t\tdie(\"diff_setup_done failed\");\n-\tdiff_tree_setup_paths(revs->diffopt.pathspec.raw, &revs->pruning);\n+\tinit_pathspec(&revs->pruning.pathspec, revs->diffopt.pathspec.raw);\n \n \tcompile_grep_patterns(&revs->grep_filter);\n \ndiff --git a/tree-diff.c b/tree-diff.c\nindex 3954281..7df9064 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -209,26 +209,23 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co\n \tstruct diff_options diff_opts;\n \tstruct diff_queue_struct *q = &diff_queued_diff;\n \tstruct diff_filepair *choice;\n-\tconst char *paths[1];\n \tint i;\n \n \t/* Remove the file creation entry from the diff queue, and remember it */\n \tchoice = q->queue[0];\n \tq->nr = 0;\n \n-\tdiff_setup(&diff_opts);\n+\tinit_diff(&diff_opts);\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->pathspec.raw[0];\n \tdiff_opts.break_opt = opt->break_opt;\n-\tpaths[0] = NULL;\n-\tdiff_tree_setup_paths(paths, &diff_opts);\n \tif (diff_setup_done(&diff_opts) < 0)\n \t\tdie(\"unable to set up diff options to follow renames\");\n \tdiff_tree(t1, t2, base, &diff_opts);\n \tdiffcore_std(&diff_opts);\n-\tdiff_tree_release_paths(&diff_opts);\n+\trelease_diff(&diff_opts);\n \n \t/* Go through the new set of filepairing, and see if we find a more interesting one */\n \topt->found_follow = 0;\n@@ -247,9 +244,9 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co\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\trelease_diff(opt);\n \t\t\topt->pathspec.raw[0] = xstrdup(p->one->path);\n-\t\t\tdiff_tree_setup_paths(opt->pathspec.raw, opt);\n+\t\t\tinit_pathspec(&opt->pathspec, opt->pathspec.raw);\n \n \t\t\t/*\n \t\t\t * The caller expects us to return a set of vanilla\n@@ -323,13 +320,3 @@ int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_\n \tfree(tree);\n \treturn retval;\n }\n-\n-void diff_tree_release_paths(struct diff_options *opt)\n-{\n-\tfree_pathspec(&opt->pathspec);\n-}\n-\n-void diff_tree_setup_paths(const char **p, struct diff_options *opt)\n-{\n-\tinit_pathspec(&opt->pathspec, p);\n-}\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"163107","messageId":"1299726819-5576-6-git-send-email-pclouds@gmail.com","threadId":"26701","inReplyTo":"1299726819-5576-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 5/5] diff: support --exclude","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T03:13:39Z","receivedAt":"2011-03-10T03:13:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"diff_setup_done() will update diff_options.pathspec, so if you give\n\n--exclude '*.h' -- src/\n\nthen the final pathspec looks like\n\n - src/\n - *.h (negated)\n\nThe code is protected against multiple call because setup_revisions()\nmay call it once, then the outer code calls it again. I'm not sure if\ndoing it this way is correct though.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n diff.c |   41 ++++++++++++++++++++++++++++++++++++++++-\n diff.h |    1 +\n 2 files changed, 41 insertions(+), 1 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 6f206a9..14f1cfc 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2973,6 +2973,34 @@ int diff_setup_done(struct diff_options *options)\n \t\tDIFF_OPT_SET(options, EXIT_WITH_STATUS);\n \t}\n \n+\t/*\n+\t * Adjust options->pathspec. All --exclude will be appended to\n+\t * options->pathspec with to_exclude set.\n+\t */\n+\tif (options->paths &&\n+\t    options->pathspec.raw != options->paths) { /* avoid being called twice */\n+\t\tint i, nr, new_nr, exc;\n+\n+\t\tnr = options->pathspec.nr ? options->pathspec.nr : 1;\n+\t\tfor (exc = 0; options->paths[exc]; exc++)\n+\t\t\t;\t\t\t  /* nothing */\n+\t\tnew_nr = nr + exc;\n+\n+\t\toptions->paths = xrealloc(options->paths, sizeof(char*) * (new_nr + 1));\n+\t\tmemmove(options->paths + nr, options->paths, sizeof(char*) * (exc + 1));\n+\t\tif (options->pathspec.nr)\n+\t\t\tmemcpy(options->paths, options->pathspec.raw, sizeof(char*) * nr);\n+\t\telse\n+\t\t\toptions->paths[0] = \"\";\n+\n+\t\tfree_pathspec(&options->pathspec);\n+\t\tinit_pathspec(&options->pathspec, options->paths);\n+\t\tfor (i = nr; i < new_nr; i++) {\n+\t\t\tstruct pathspec_item *item = options->pathspec.items + i;\n+\t\t\titem->to_exclude = 1;\n+\t\t}\n+\t}\n+\n \treturn 0;\n }\n \n@@ -2980,6 +3008,7 @@ int diff_setup_done(struct diff_options *options)\n void release_diff(struct diff_options *o)\n {\n \tfree_pathspec(&o->pathspec);\n+\tfree(o->paths);\n }\n \n static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)\n@@ -3347,7 +3376,17 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\t\tdie_errno(\"Could not open '%s'\", optarg);\n \t\toptions->close_file = 1;\n \t\treturn argcount;\n-\t} else\n+\t}\n+\telse if ((argcount = parse_long_opt(\"exclude\", av, &optarg))) {\n+\t\tint i = 1;\n+\t\twhile (options->paths && options->paths[i])\n+\t\t\ti++;\n+\t\toptions->paths = xrealloc(options->paths, sizeof(char*) * (i + 1));\n+\t\toptions->paths[i - 1] = optarg;\n+\t\toptions->paths[i] = 0;\n+\t\treturn argcount;\n+\t}\n+\telse\n \t\treturn 0;\n \treturn 1;\n }\ndiff --git a/diff.h b/diff.h\nindex 12a9907..b60057c 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -134,6 +134,7 @@ struct diff_options {\n \tint close_file;\n \n \tstruct pathspec pathspec;\n+\tconst char **paths;\n \tchange_fn_t change;\n \tadd_remove_fn_t add_remove;\n \tdiff_format_fn_t format_callback;\n-- \n1.7.3.1.256.g2539c.dirty\n"},{"id":"163111","messageId":"7vmxl37bdp.fsf@alter.siamese.dyndns.org","threadId":"26701","inReplyTo":"1299726819-5576-1-git-send-email-pclouds@gmail.com","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-10T06:11:46Z","receivedAt":"2011-03-10T06:11:46Z","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> On the other hand, if we go with ':' as a mark of special pathspecs, then\n>\n>  - \":/\" mark pathspecs relative to worktree root\n>  - \":!\" may mark negative pathspecs\n\nI am still moderately negative on this \"negative pathspec\" stuff, as it\nwill complicate the semantics (just one example: would a path that is\ncovered by both positive and negative pathspecs included or excluded?\nwould the last one win? would the more specific one win?) and makes the\ndesign harder to explain to the users. Depending on the semantics chosen,\nit may also make the implementation less efficient and more complex.\n\nAs the choice of the syntax goes, in the recent \"grep --full-tree\"\ndiscussion, I thought people are more or less happy with the colon\nprefixed \"magic pathspec\" syntax, and something along the lines of the\nabove two seems to be a good design.\n\nThe two most important things to consider are to make sure that people\nwith funny pathnames can work it around by quoting, and the prefixing\nscheme is extensible so that other types of magic can later be introduced\nwith the same kind of escape hatch for people with funny pathnames that\nbegin with or contain new magic characters used to trigger the new magic.\n\nI said \"something along the lines\" above because \":/ for root, :! for\nnegative\" does not yet specify how the scheme would satisfy the above\ntwo consideration very well.\n"},{"id":"163113","messageId":"7vhbbb7ays.fsf@alter.siamese.dyndns.org","threadId":"26701","inReplyTo":"1299726819-5576-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 3/5] revision.c: get rid of struct rev_info.prune_data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-10T06:20:43Z","receivedAt":"2011-03-10T06:20:43Z","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> There are three struct pathspec(s) in struct rev_info:\n>\n>  - prune_data\n>  - pruning.pathspec\n>  - diffopt.pathspec\n>\n> In some places, this pathspec is used, in other places another\n> ... but i'm\n> not sure why diffopt can't be used in place of pruning.\n\nA command in the \"log\" family uses revision traversal and uses one\npathspec (pruning.pathspec) to cull side branches and skip irrelevant\ncommits, and another pathspec (diffopt.pathspec) to specify how the found\ncommit is shown.  Off the top of my head, currently the only case the\npathspec of these diff options differ is when --full-diff is used, but\nthey are independent concepts.\n\nI would suspect this would matter even more once somebody wants to redo\nthe --follow hack to actually follow different renamed-from path per\nmerged branches, but I haven't thought through the issues.\n"},{"id":"163114","messageId":"AANLkTintigx=smQisL9FPLs93btDYJQPnU9bsSws0T0L@mail.gmail.com","threadId":"26701","inReplyTo":"7vhbbb7ays.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/5] revision.c: get rid of struct rev_info.prune_data","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T06:52:41Z","receivedAt":"2011-03-10T06:52:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/3/10 Junio C Hamano <gitster@pobox.com>:\n>> In some places, this pathspec is used, in other places another\n>> ... but i'm\n>> not sure why diffopt can't be used in place of pruning.\n>\n> A command in the \"log\" family uses revision traversal and uses one\n> pathspec (pruning.pathspec) to cull side branches and skip irrelevant\n> commits, and another pathspec (diffopt.pathspec) to specify how the found\n> commit is shown.  Off the top of my head, currently the only case the\n> pathspec of these diff options differ is when --full-diff is used, but\n> they are independent concepts.\n\nOK then I did it wrong. I should have removed prune_data in favor of\npruning.pathspec, leaving diffopt.pathspec intact.\n-- \nDuy\n"},{"id":"163115","messageId":"AANLkTimixVPGuZH9o6gkB7J=ZOmJ2PuitCH5HY8AnDe+@mail.gmail.com","threadId":"26701","inReplyTo":"7vmxl37bdp.fsf@alter.siamese.dyndns.org","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T07:03:57Z","receivedAt":"2011-03-10T07:03:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/3/10 Junio C Hamano <gitster@pobox.com>:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> On the other hand, if we go with ':' as a mark of special pathspecs, then\n>>\n>>  - \":/\" mark pathspecs relative to worktree root\n>>  - \":!\" may mark negative pathspecs\n>\n> I am still moderately negative on this \"negative pathspec\" stuff, as it\n> will complicate the semantics (just one example: would a path that is\n> covered by both positive and negative pathspecs included or excluded?\n> would the last one win? would the more specific one win?) and makes the\n> design harder to explain to the users. Depending on the semantics chosen,\n> it may also make the implementation less efficient and more complex.\n\nThe semantics resembles .gitignore, or at least that's my intention.\nBut yes, this kind of notion is more complex than simple '--exclude'\noption, which would suit 90% cases, I guess.\n\n> The two most important things to consider are to make sure that people\n> with funny pathnames can work it around by quoting, and the prefixing\n> scheme is extensible so that other types of magic can later be introduced\n> with the same kind of escape hatch for people with funny pathnames that\n> begin with or contain new magic characters used to trigger the new magic.\n>\n> I said \"something along the lines\" above because \":/ for root, :! for\n> negative\" does not yet specify how the scheme would satisfy the above\n> two consideration very well.\n\nThe quoting part may go through another painful migration plan if you\ndon't want to surprise users.\n\nI have already suggested to reserve the next character after ':' (or\nany char chosen as the magic one) as control character, for this\nextensibility. Any unrecognized control character will be rejected.\nEven if we limit ourselves to punctuation letters only, there's still\nplenty room for future after '/' and '!' are taken.\n-- \nDuy\n"},{"id":"163120","messageId":"7vvczr5pw6.fsf@alter.siamese.dyndns.org","threadId":"26701","inReplyTo":"7vmxl37bdp.fsf@alter.siamese.dyndns.org","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-10T08:41:13Z","receivedAt":"2011-03-10T08:41:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> On the other hand, if we go with ':' as a mark of special pathspecs, then\n>>\n>>  - \":/\" mark pathspecs relative to worktree root\n>>  - \":!\" may mark negative pathspecs\n> ...\n>\n> As the choice of the syntax goes, in the recent \"grep --full-tree\"\n> discussion, I thought people are more or less happy with the colon\n> prefixed \"magic pathspec\" syntax, and something along the lines of the\n> above two seems to be a good design.\n>\n> The two most important things to consider are to make sure that people\n> with funny pathnames can work it around by quoting, and the prefixing\n> scheme is extensible so that other types of magic can later be introduced\n> with the same kind of escape hatch for people with funny pathnames that\n> begin with or contain new magic characters used to trigger the new magic.\n>\n> I said \"something along the lines\" above because \":/ for root, :! for\n> negative\" does not yet specify how the scheme would satisfy the above\n> two consideration very well.\n\nLet's step back a bit.\n\nWe chose to use \":/<regexp>\" as one new form of extended SHA-1 expression\nto name an object for two reasons: (1) no normal <ref> can have a colon in\nit, because of check-ref-format restriction; (2) \":\" is an unlikely letter\nto appear at the beginning of a pathname, and people with such a path can\nwork around by saying \"./:frotz\" or \"\\:xyzzy\".\n\nThere is a disambiguation logic to check a list of arguments that lacks an\nexplicit \"--\" separator to make sure that each element early on the list\ncan only be interpreted as an object name but not as a pathname that\nexists on the filesystem, and all the remaining elements are pathnames\nthat exist on the filesystem.\n\nIf we introduce an extended syntax for pathspec and make the prefix magic\ncharacter \":\", and if we choose to use \":/\" as one kind of magic, I was a\nbit worried that this may affect the disambiguation.  The users must use\nan explicit \"--\" when feeding a pathspec with the magic so that the parser\nknows which kind of magic (either object name magic or pathspec magic)\nthey are talking about.\n\nI however realized that it is not an issue at all, because the users\nalready need to disambiguate with \"--\" when using wildcards in their\npathspecs (e.g. \"git log 'Makefil*'\" will give you an ambiguity error).\nAdmittedly, wildcard pathspecs are lessor kind of magic, but they are\nmagic nevertheless.\n\nSo my tentative conclusion is that there is no problem using the same \":\"\nas the magic introducer for pathspecs, just like we do for object names.\n\n\tSide note: please point out flaws in the above train of thought\n        that would make the end result non-workable.\n\nAlthough I like the general approach Michael took with his \"alternative\nsetup.c that understands tree-wide pathspec\" patch (Cf. $gmane/168207), I\nagree \":/\" would be better than saying \"':' is root, and we don't allow\nany other magic under ':'\" for the sake of extensibility.\n\nI am not very happy with \":!\" as negative from syntactical point of view,\nbecause traditionally \"!\" is a tad cumbersome to quote in shells with\nhistory support (e.g. \"csh\"), but unfortunately I don't think of any good\nalternative that is easy to type.  Pathspecs are globs by nature, and \"!\"\nis a good choice of nagation indicator for that reason (think \"[!aeiou]\"\nnot \"[^aeiou]\" the latter of which is a regexp and not glob), though.\n"},{"id":"163127","messageId":"AANLkTimsv9bO+Go6Mqrrp_1-AZ=sC3ndyAuskPYLVbkv@mail.gmail.com","threadId":"26701","inReplyTo":"7vvczr5pw6.fsf@alter.siamese.dyndns.org","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-10T10:05:22Z","receivedAt":"2011-03-10T10:05:22Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/3/10 Junio C Hamano <gitster@pobox.com>:\n> Let's step back a bit.\n>\n> We chose to use \":/<regexp>\" as one new form of extended SHA-1 expression\n> to name an object for two reasons: (1) no normal <ref> can have a colon in\n> it, because of check-ref-format restriction; (2) \":\" is an unlikely letter\n> to appear at the beginning of a pathname, and people with such a path can\n> work around by saying \"./:frotz\" or \"\\:xyzzy\".\n>\n> There is a disambiguation logic to check a list of arguments that lacks an\n> explicit \"--\" separator to make sure that each element early on the list\n> can only be interpreted as an object name but not as a pathname that\n> exists on the filesystem, and all the remaining elements are pathnames\n> that exist on the filesystem.\n>\n> If we introduce an extended syntax for pathspec and make the prefix magic\n> character \":\", and if we choose to use \":/\" as one kind of magic, I was a\n> bit worried that this may affect the disambiguation.  The users must use\n> an explicit \"--\" when feeding a pathspec with the magic so that the parser\n> knows which kind of magic (either object name magic or pathspec magic)\n> they are talking about.\n\nOr.. we can consider this ':' a special form of wildcard and interpret\nthe same way:\n\n - first try exact match. If it matches, the leading ':' is\ninterpreted literally as part of file name.\n - magic.\n\n> I however realized that it is not an issue at all, because the users\n> already need to disambiguate with \"--\" when using wildcards in their\n> pathspecs (e.g. \"git log 'Makefil*'\" will give you an ambiguity error).\n> Admittedly, wildcard pathspecs are lessor kind of magic, but they are\n> magic nevertheless.\n\nYes, ':' is on the same side with wildcard pathspecs.\n\n> So my tentative conclusion is that there is no problem using the same \":\"\n> as the magic introducer for pathspecs, just like we do for object names.\n>\n>        Side note: please point out flaws in the above train of thought\n>        that would make the end result non-workable.\n\nHuh? I failed.\n\n> I am not very happy with \":!\" as negative from syntactical point of view,\n> because traditionally \"!\" is a tad cumbersome to quote in shells with\n> history support (e.g. \"csh\"), but unfortunately I don't think of any good\n> alternative that is easy to type.  Pathspecs are globs by nature, and \"!\"\n> is a good choice of nagation indicator for that reason (think \"[!aeiou]\"\n> not \"[^aeiou]\" the latter of which is a regexp and not glob), though.\n\nWe don't need to stick to ':!'. I just needed something to start\ncoding. Other candidates can be '^' or '-' or '~'. '^' looks best in\nmy opinion.\n-- \nDuy\n"},{"id":"163866","messageId":"AANLkTimWPoMEYdLCwTTit6AeuJegOc96gE1JsO8B4fh=@mail.gmail.com","threadId":"26701","inReplyTo":"AANLkTimsv9bO+Go6Mqrrp_1-AZ=sC3ndyAuskPYLVbkv@mail.gmail.com","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-21T04:02:02Z","receivedAt":"2011-03-21T04:02:02Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Mar 10, 2011 at 5:05 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n> 2011/3/10 Junio C Hamano <gitster@pobox.com>:\n>> Let's step back a bit.\n>>\n>> We chose to use \":/<regexp>\" as one new form of extended SHA-1 expression\n>> to name an object for two reasons: (1) no normal <ref> can have a colon in\n>> it, because of check-ref-format restriction; (2) \":\" is an unlikely letter\n>> to appear at the beginning of a pathname, and people with such a path can\n>> work around by saying \"./:frotz\" or \"\\:xyzzy\".\n>>\n>> There is a disambiguation logic to check a list of arguments that lacks an\n>> explicit \"--\" separator to make sure that each element early on the list\n>> can only be interpreted as an object name but not as a pathname that\n>> exists on the filesystem, and all the remaining elements are pathnames\n>> that exist on the filesystem.\n>>\n>> If we introduce an extended syntax for pathspec and make the prefix magic\n>> character \":\", and if we choose to use \":/\" as one kind of magic, I was a\n>> bit worried that this may affect the disambiguation.  The users must use\n>> an explicit \"--\" when feeding a pathspec with the magic so that the parser\n>> knows which kind of magic (either object name magic or pathspec magic)\n>> they are talking about.\n>\n> Or.. we can consider this ':' a special form of wildcard and interpret\n> the same way:\n>\n>  - first try exact match. If it matches, the leading ':' is\n> interpreted literally as part of file name.\n>  - magic.\n\nOne thing that makes it different from Michael's approach is, :/foo\nwill match ':/foo' literally in every directories and foo at top-tree.\nI feel mildly uncomfortable with it, but it makes it consistent with\nother wildcards. If no one objects, I'll try to make a patch with this\napproach.\n-- \nDuy\n"},{"id":"163882","messageId":"4D871E51.8090106@drmicha.warpmail.net","threadId":"26701","inReplyTo":"AANLkTimWPoMEYdLCwTTit6AeuJegOc96gE1JsO8B4fh=@mail.gmail.com","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-21T09:45:53Z","receivedAt":"2011-03-21T09:45:53Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyen Thai Ngoc Duy venit, vidit, dixit 21.03.2011 05:02:\n> On Thu, Mar 10, 2011 at 5:05 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n>> 2011/3/10 Junio C Hamano <gitster@pobox.com>:\n>>> Let's step back a bit.\n>>>\n>>> We chose to use \":/<regexp>\" as one new form of extended SHA-1 expression\n>>> to name an object for two reasons: (1) no normal <ref> can have a colon in\n>>> it, because of check-ref-format restriction; (2) \":\" is an unlikely letter\n>>> to appear at the beginning of a pathname, and people with such a path can\n>>> work around by saying \"./:frotz\" or \"\\:xyzzy\".\n>>>\n>>> There is a disambiguation logic to check a list of arguments that lacks an\n>>> explicit \"--\" separator to make sure that each element early on the list\n>>> can only be interpreted as an object name but not as a pathname that\n>>> exists on the filesystem, and all the remaining elements are pathnames\n>>> that exist on the filesystem.\n>>>\n>>> If we introduce an extended syntax for pathspec and make the prefix magic\n>>> character \":\", and if we choose to use \":/\" as one kind of magic, I was a\n>>> bit worried that this may affect the disambiguation.  The users must use\n>>> an explicit \"--\" when feeding a pathspec with the magic so that the parser\n>>> knows which kind of magic (either object name magic or pathspec magic)\n>>> they are talking about.\n>>\n>> Or.. we can consider this ':' a special form of wildcard and interpret\n>> the same way:\n>>\n>>  - first try exact match. If it matches, the leading ':' is\n>> interpreted literally as part of file name.\n>>  - magic.\n> \n> One thing that makes it different from Michael's approach is, :/foo\n> will match ':/foo' literally in every directories and foo at top-tree.\n> I feel mildly uncomfortable with it, but it makes it consistent with\n> other wildcards. If no one objects, I'll try to make a patch with this\n> approach.\n\nMaybe I misunderstand which topic you are referring to. I have a patch\nfor \":\" (to denote repo-root in pathspecs), there's no need to make\nanother one for \":/\". (I would really prefer to do it myself since I\nbrought it up.) I was just waiting for agreement to settle in about\nwhich prefix to use. Also, I have checked with J&J of mingw/msysgit fame\nmeanwhile to make sure that the notation does not create problems on the\nwindows side of git.\n\nMichael\n"},{"id":"163891","messageId":"AANLkTin3VVcVZZv9A+Wam41uxhReuCAaQL_6MOgCpN+h@mail.gmail.com","threadId":"26701","inReplyTo":"4D871E51.8090106@drmicha.warpmail.net","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-21T10:02:54Z","receivedAt":"2011-03-21T10:02:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Mar 21, 2011 at 4:45 PM, Michael J Gruber\n<git@drmicha.warpmail.net> wrote:\n>> One thing that makes it different from Michael's approach is, :/foo\n>> will match ':/foo' literally in every directories and foo at top-tree.\n>> I feel mildly uncomfortable with it, but it makes it consistent with\n>> other wildcards. If no one objects, I'll try to make a patch with this\n>> approach.\n>\n> Maybe I misunderstand which topic you are referring to. I have a patch\n> for \":\" (to denote repo-root in pathspecs), there's no need to make\n> another one for \":/\". (I would really prefer to do it myself since I\n> brought it up.)\n\nIf you make it a wildcard as I mentioned above, the code would be\ndifferent. Your patch will match only files at top, while wildcards\ncan also match any files literally. You can do it, I don't mind at\nall.\n\n> I was just waiting for agreement to settle in about\n> which prefix to use. Also, I have checked with J&J of mingw/msysgit fame\n> meanwhile to make sure that the notation does not create problems on the\n> windows side of git.\n\nThe nice (or not) thing about wildcard behavior is that because it\nwould match both cases, we would not need to worry too much. People\nwho don't know the magic can still match files (although more, not\nless).\n-- \nDuy\n"},{"id":"164100","messageId":"7v7hbqk8ox.fsf@alter.siamese.dyndns.org","threadId":"26701","inReplyTo":"4D871E51.8090106@drmicha.warpmail.net","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-22T23:59:58Z","receivedAt":"2011-03-22T23:59:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n>> One thing that makes it different from Michael's approach is, :/foo\n>> will match ':/foo' literally in every directories and foo at top-tree.\n>> I feel mildly uncomfortable with it, but it makes it consistent with\n>> other wildcards. If no one objects, I'll try to make a patch with this\n>> approach.\n>\n> Maybe I misunderstand which topic you are referring to. I have a patch\n> for \":\" (to denote repo-root in pathspecs), there's no need to make\n> another one for \":/\". (I would really prefer to do it myself since I\n> brought it up.) I was just waiting for agreement to settle in about\n> which prefix to use.\n\nI am not sure if there is any disagreement in the desired semantics\nbetween the two.\n\nLet's clarify by dumping my understanding of what we aim to achieve.\n\n - The user gives ':/<frotz>' and '<xyzzy>' from the command line, in a\n   subdirectory.\n\n - Internally we chdir() up to the top of the working tree, and the prefix\n   is set to the path to the subdirectory (with trailing slash);\n   get_pathspec(prefix, pathspec) is called, where pathspec is often a\n   later part of argv[] we got from the command line.\n\n - Currently, get_pathspec() gives either:\n\n    - the same as incoming pathspec, if <prefix> is NULL (this happens at\n      the root of the working tree);\n\n    - { <prefix>, NULL }, if the incoming pathspec array is empty; or\n\n    - an array of strings taken from pathspec, each prefixed with <prefix>.\n\n   The list of the resulting strings is the pathspec, relative to the root\n   of the working tree, and is used to determine if a path matches the\n   given pathspec.  The match traditionally had two semantics (diff-tree\n   family and everybody else, only the latter group knew about globs), but\n   an earlier work by Nguyen has taught globs to everybody (at least it\n   aimed to; we might have leftover cases that we haven't uncovered).\n\nBoth the \":/<path>\" proposal (or your \":<path>\" proposal) changes only a\nvery small part of the above, namely, \"each prefixed with '<prefix>'\" is\nchanged to if the element in original pathspec has the magic colon prefix,\nthe magic is stripped away, and the remainder becomes the element in the\nresulting pathspec array without additional <prefix> in front.\n\nIf Nguyen's proposal is to also match \":/<path>\" (or \":<path>\") literally,\nthat part should be scrapped.  If somebody wants to match such an unusual\npath component, it can always be expressed by quoting it as a glob,\ni.e. \"[:]/<path>\" (or \"[:]<path\").\n\nI am slightly in favor of \":<path>\" syntax (than \":/<path>\"), but I do not\ncare too deeply.  Either has the same problem that it will be confusing\nwith existing and well-established syntax (the former would conflict with\n\"name of the blob in the index\", the latter with \"name of the commit that\nmatch the regexp).\n"},{"id":"164132","messageId":"AANLkTimeEWOvmNXQV9fJKe23Qe3sipT7q=qHVB2fLq0n@mail.gmail.com","threadId":"26701","inReplyTo":"7v7hbqk8ox.fsf@alter.siamese.dyndns.org","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-23T12:10:07Z","receivedAt":"2011-03-23T12:10:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 23, 2011 at 6:59 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Let's clarify by dumping my understanding of what we aim to achieve.\n>\n> ...\n>\n> Both the \":/<path>\" proposal (or your \":<path>\" proposal) changes only a\n> very small part of the above, namely, \"each prefixed with '<prefix>'\" is\n> changed to if the element in original pathspec has the magic colon prefix,\n> the magic is stripped away, and the remainder becomes the element in the\n> resulting pathspec array without additional <prefix> in front.\n\nCorrect.\n\n> If Nguyen's proposal is to also match \":/<path>\" (or \":<path>\") literally,\n> that part should be scrapped.  If somebody wants to match such an unusual\n> path component, it can always be expressed by quoting it as a glob,\n> i.e. \"[:]/<path>\" (or \"[:]<path\").\n\nOK.\n\n> I am slightly in favor of \":<path>\" syntax (than \":/<path>\"), but I do not\n> care too deeply.  Either has the same problem that it will be confusing\n> with existing and well-established syntax (the former would conflict with\n> \"name of the blob in the index\", the latter with \"name of the commit that\n> match the regexp).\n\nHow about \":<path>\" for root pathspecs, but reserve ':[^0-9A-Za-z]*'\nfor future use?\n\n(on top of Michael's patch)\n\n-- 8< --\ndiff --git a/setup.c b/setup.c\nindex ef55e5d..1ebe1d2 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -146,8 +146,13 @@ const char **get_pathspec(const char *prefix,\nconst char **pathspec)\n \twhile (*src) {\n \t\tconst char *p;\n\n-\t\tif ((*src)[0] == ':')\n+\t\tif ((*src)[0] == ':') {\n+\t\t\tconst char *reserved = \"~`!@#$%^&*()-_=+[{]}\\\\|;:'\\\",<.>/?\";\n+\t\t\tif (strchr(reserved, (*src)[1]))\n+\t\t\t\tdie(\"':%c' syntax is not supported. \"\n+\t\t\t\t    \"Quote it to match literally.\", (*src)[1]);\n \t\t\tp = prefix_path(NULL, 0, (*src)+1);\n+\t\t}\n \t\telse\n \t\t\tp = prefix_path(prefix, prefixlen, *src);\n \t\t*(dst++) = p;\n-- 8< --\n-- \nDuy\n"},{"id":"164136","messageId":"AANLkTimtGCdo40V-pYZ5YHEO_pzV7zx6Py3bC5aVufYb@mail.gmail.com","threadId":"26701","inReplyTo":"AANLkTimeEWOvmNXQV9fJKe23Qe3sipT7q=qHVB2fLq0n@mail.gmail.com","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-23T12:18:24Z","receivedAt":"2011-03-23T12:18:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 23, 2011 at 7:10 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n> How about \":<path>\" for root pathspecs, but reserve ':[^0-9A-Za-z]*'\n> for future use?\n> ..\n> +                       if (strchr(reserved, (*src)[1]))\n> +                               die(\"':%c' syntax is not supported. \"\n> +                                   \"Quote it to match literally.\", (*src)[1]);\n\nHmm.. I need a letter to become quoter. Let's take '\\' for that. I\nalso leave '.' out of reserved range because it may be used frequently\n(hidden files).\n\ndiff --git a/setup.c b/setup.c\nindex ef55e5d..b6294be 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -146,8 +146,16 @@ const char **get_pathspec(const char *prefix,\nconst char **pathspec)\n \twhile (*src) {\n \t\tconst char *p;\n\n-\t\tif ((*src)[0] == ':')\n-\t\t\tp = prefix_path(NULL, 0, (*src)+1);\n+\t\tif ((*src)[0] == ':') {\n+\t\t\tconst char *reserved = \"~`!@#$%^&*()-_=+[{]}|;:'\\\",<>/?\";\n+\t\t\tconst char *s = *src + 1;\n+\t\t\tif (*s == '\\\\')\n+\t\t\t\ts++;\n+\t\t\telse if (strchr(reserved, *s))\n+\t\t\t\tdie(\"':%c' syntax is not supported. \"\n+\t\t\t\t    \"Quote it to match literally.\", *s);\n+\t\t\tp = prefix_path(NULL, 0, s);\n+\t\t}\n \t\telse\n \t\t\tp = prefix_path(prefix, prefixlen, *src);\n \t\t*(dst++) = p;\n-- \nDuy\n"},{"id":"164137","messageId":"4D89ECC6.7040903@drmicha.warpmail.net","threadId":"26701","inReplyTo":"AANLkTimeEWOvmNXQV9fJKe23Qe3sipT7q=qHVB2fLq0n@mail.gmail.com","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-03-23T12:51:18Z","receivedAt":"2011-03-23T12:51:18Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Nguyen Thai Ngoc Duy venit, vidit, dixit 23.03.2011 13:10:\n> On Wed, Mar 23, 2011 at 6:59 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Let's clarify by dumping my understanding of what we aim to achieve.\n>>\n>> ...\n>>\n>> Both the \":/<path>\" proposal (or your \":<path>\" proposal) changes only a\n>> very small part of the above, namely, \"each prefixed with '<prefix>'\" is\n>> changed to if the element in original pathspec has the magic colon prefix,\n>> the magic is stripped away, and the remainder becomes the element in the\n>> resulting pathspec array without additional <prefix> in front.\n> \n> Correct.\n> \n>> If Nguyen's proposal is to also match \":/<path>\" (or \":<path>\") literally,\n>> that part should be scrapped.  If somebody wants to match such an unusual\n>> path component, it can always be expressed by quoting it as a glob,\n>> i.e. \"[:]/<path>\" (or \"[:]<path\").\n> \n> OK.\n> \n>> I am slightly in favor of \":<path>\" syntax (than \":/<path>\"), but I do not\n>> care too deeply.  Either has the same problem that it will be confusing\n>> with existing and well-established syntax (the former would conflict with\n>> \"name of the blob in the index\", the latter with \"name of the commit that\n>> match the regexp).\n> \n> How about \":<path>\" for root pathspecs, but reserve ':[^0-9A-Za-z]*'\n> for future use?\n> \n> (on top of Michael's patch)\n> \n> -- 8< --\n> diff --git a/setup.c b/setup.c\n> index ef55e5d..1ebe1d2 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -146,8 +146,13 @@ const char **get_pathspec(const char *prefix,\n> const char **pathspec)\n>  \twhile (*src) {\n>  \t\tconst char *p;\n> \n> -\t\tif ((*src)[0] == ':')\n> +\t\tif ((*src)[0] == ':') {\n> +\t\t\tconst char *reserved = \"~`!@#$%^&*()-_=+[{]}\\\\|;:'\\\",<.>/?\";\n> +\t\t\tif (strchr(reserved, (*src)[1]))\n> +\t\t\t\tdie(\"':%c' syntax is not supported. \"\n> +\t\t\t\t    \"Quote it to match literally.\", (*src)[1]);\n>  \t\t\tp = prefix_path(NULL, 0, (*src)+1);\n> +\t\t}\n>  \t\telse\n>  \t\t\tp = prefix_path(prefix, prefixlen, *src);\n>  \t\t*(dst++) = p;\n> -- 8< --\n\nSounds good to me so far.\n\nNote that there is also possible notational overlap with the WIP \"attr:\nmake attributes depend on file type\" at\n\nhttp://permalink.gmane.org/gmane.comp.version-control.git/168116\n\nwhich suggested something like\n\n:symlink:* diff=symlinkdiffdriverofyourchoice\n\nas a notation for attributes which depend on the file type. Is that a\nproblem, should I do something different there? (Also, I still need help\nwith that...)\n\nMichael\n"},{"id":"164139","messageId":"AANLkTinsVPHV7H_AGHxyGAXpjvJ0n5SvNja1ZBvt4fGd@mail.gmail.com","threadId":"26701","inReplyTo":"4D89ECC6.7040903@drmicha.warpmail.net","subject":"Re: [WIP PATCH 0/5] support --exclude for diff/log commands","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-03-23T13:34:52Z","receivedAt":"2011-03-23T13:34:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Wed, Mar 23, 2011 at 7:51 PM, Michael J Gruber\n<git@drmicha.warpmail.net> wrote:\n> Note that there is also possible notational overlap with the WIP \"attr:\n> make attributes depend on file type\" at\n>\n> http://permalink.gmane.org/gmane.comp.version-control.git/168116\n>\n> which suggested something like\n>\n> :symlink:* diff=symlinkdiffdriverofyourchoice\n>\n> as a notation for attributes which depend on the file type. Is that a\n> problem, should I do something different there? (Also, I still need help\n> with that...)\n\nI would do\n\n:{symlink}* diff=symlinkdiffdriverofyourchoice\n\nThere's also blank quote problem in gitattr. Tricks like 'hello.world'\nwould match path 'hello world' but I'd rather have something better.\nIf ':' is the special char, I could teach gitattr to recognize\n<colon>+<quote> as a mark to do C quotation without regressions.\n-- \nDuy\n"}]}