{"thread":{"id":"19494","subject":"[PATCH RFC] Convert ce_path_match() use to match_pathspec()","startedAt":"2009-05-25T08:42:04Z","lastAt":"2009-05-26T11:04:35Z","messageCount":3,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"114637","messageId":"1243240924-5981-1-git-send-email-pclouds@gmail.com","threadId":"19494","inReplyTo":null,"subject":"[PATCH RFC] Convert ce_path_match() use to match_pathspec()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2009-05-25T08:42:04Z","receivedAt":"2009-05-25T08:42:04Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Back in history, ce_path_match() was first introduced in commit\nfdee7d07ba6c79b3e5125e96adbe1d9c3e75ce1d, in diff-cache.c. It seems\nto be used to handle pathspec that we have today. But it did not\nsupport wildcards.\n\nAbout one year later, match_pathspec() was introduced as match() in\ncommit 0d78153952e70c21e94dc6b7eefcb2ac5337a902, builtin-add.c. This\nversion supported wildcards.\n\nFor some reasons diff code did not get converted to use\nmatch_pathspec(). So diff commands do not understand wildcards. I was\nnot here that time to know the reasons. But I find it quite handy to\ndo \"git diff -- '*.sh'\", just like the rest of git commands.\n\nHence this patch, which simply calls match_pathspec() inside\nce_path_match(). With this, \"git diff-files\" and \"git diff-index\" now\nsupport wildcards. \"git diff-tree\" does not because it does not use\nce_path_match().\n\n\"git update-index --again\" is also affected (in a good way hopefully)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n So.. comments?\n\n read-cache.c |   20 +-------------------\n 1 files changed, 1 insertions(+), 19 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 3f58711..d2daf01 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -677,28 +677,10 @@ int ce_same_name(struct cache_entry *a, struct cache_entry *b)\n \n int ce_path_match(const struct cache_entry *ce, const char **pathspec)\n {\n-\tconst char *match, *name;\n-\tint len;\n-\n \tif (!pathspec)\n \t\treturn 1;\n \n-\tlen = ce_namelen(ce);\n-\tname = ce->name;\n-\twhile ((match = *pathspec++) != NULL) {\n-\t\tint matchlen = strlen(match);\n-\t\tif (matchlen > len)\n-\t\t\tcontinue;\n-\t\tif (memcmp(name, match, matchlen))\n-\t\t\tcontinue;\n-\t\tif (matchlen && name[matchlen-1] == '/')\n-\t\t\treturn 1;\n-\t\tif (name[matchlen] == '/' || !name[matchlen])\n-\t\t\treturn 1;\n-\t\tif (!matchlen)\n-\t\t\treturn 1;\n-\t}\n-\treturn 0;\n+\treturn match_pathspec(pathspec, ce->name, ce_namelen(ce), 0, NULL);\n }\n \n /*\n-- \ntest\n"},{"id":"114707","messageId":"7voctgvnic.fsf@alter.siamese.dyndns.org","threadId":"19494","inReplyTo":"1243240924-5981-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH RFC] Convert ce_path_match() use to match_pathspec()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-25T22:14:19Z","receivedAt":"2009-05-25T22:14:19Z","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> For some reasons diff code did not get converted to use\n> match_pathspec(). So diff commands do not understand wildcards.\n\nce_path_match() is not just about diffs; there may be places that do not\nexpect pathspecs to match cruft with globs.  Have you checked all the\ncallsites and they are Ok with globbing?\n\nI think using glob in diff-files should be Ok, but that would make it\ninconsistent with diff-tree (and possibly diff-index but I didn't check).\nThe correct operation of diff-tree (and path pruning in \"git log\" family)\nheavily relies on an early-exit optimization not to recurse into a\ndirectory when we can detect that none of the paths in that directory will\never match any of the given pathspecs, and this is done based on the\nnon-globbing (iow \"leading path\") semantics; you need to be extra careful\nabout this.\n"},{"id":"114736","messageId":"fcaeb9bf0905260404h3eb35355p4fec33bc1912ce9b@mail.gmail.com","threadId":"19494","inReplyTo":"7voctgvnic.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH RFC] Convert ce_path_match() use to match_pathspec()","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2009-05-26T11:04:35Z","receivedAt":"2009-05-26T11:04:35Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2009/5/26 Junio C Hamano <gitster@pobox.com>:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n>\n>> For some reasons diff code did not get converted to use\n>> match_pathspec(). So diff commands do not understand wildcards.\n>\n> ce_path_match() is not just about diffs; there may be places that do not\n> expect pathspecs to match cruft with globs.  Have you checked all the\n> callsites and they are Ok with globbing?\n\nI'm pretty sure except preload-index. I have checked again, all\nread_cache_preload() callsites seem to prefer files and leading path\nover globbing.\n\n> I think using glob in diff-files should be Ok, but that would make it\n> inconsistent with diff-tree (and possibly diff-index but I didn't check).\n> The correct operation of diff-tree (and path pruning in \"git log\" family)\n> heavily relies on an early-exit optimization not to recurse into a\n> directory when we can detect that none of the paths in that directory will\n> ever match any of the given pathspecs, and this is done based on the\n> non-globbing (iow \"leading path\") semantics; you need to be extra careful\n> about this.\n\nNow I know the reason. Thanks.\n-- \nDuy\n"}]}