{"thread":{"id":"27000","subject":"[PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","startedAt":"2011-04-05T22:10:22Z","lastAt":"2011-04-08T19:40:14Z","messageCount":11,"participants":["Junio C Hamano","Nguyen Thai Ngoc Duy","Michael J Gruber"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"165222","messageId":"7vr59gl581.fsf@alter.siamese.dyndns.org","threadId":"27000","inReplyTo":null,"subject":"[PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-05T22:10:22Z","receivedAt":"2011-04-05T22:10:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As the point of the last change is to allow use of strings as\nliterals no matter what characters are in them, \"has_wildcard\"\ndoes not match what we use this field for anymore.\n\nIt is used to decide if the wildcard matching should be used, so\nrename it to match the usage better.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I was looking at the codepaths that would need to be touched in order\n to properly support the \"magic pathspec\" we have been discussing, and\n am leaning to conclude that all the users of get_pathspec() need to be\n rewritten to throw the remainder of argv[] at a function that fills a\n struct pathspec (in other words, a combination of get_pathspec() and\n init_pathspec()). Michael's \"alternative approach to grep --full-tree\"\n was operating at the get_pathspec() level, but that function is an\n interface to return an array of plain-vanilla strings only, and there\n is no place to hook richer per-item information on the elements. We\n would need \"struct pathspec\" in the function where we parse the argv[]\n and combine its elements with prefix.\n\n In any case, pathspec.has_wildcard vs pathspec.item[i].has_wildcard was\n difficult to grep and find patterns, and this is an obvious fix to that\n problem, independent from the above observation.\n\n builtin/ls-files.c | 2 +-\n builtin/ls-tree.c | 3 ++-\n cache.h | 2 +-\n dir.c | 6 +++---\n tree-walk.c | 4 ++--\n 5 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 199af46..1570123 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -361,7 +361,7 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix)\n \t\tmatchbuf[0] = prefix;\n \t\tmatchbuf[1] = NULL;\n \t\tinit_pathspec(&pathspec, matchbuf);\n-\t\tpathspec.items[0].has_wildcard = 0;\n+\t\tpathspec.items[0].use_wildcard = 0;\n \t} else\n \t\tinit_pathspec(&pathspec, NULL);\n \tif (read_tree(tree, 1, &pathspec))\ndiff --git a/builtin/ls-tree.c b/builtin/ls-tree.c\nindex 4290723..f08c5b0 100644\n--- a/builtin/ls-tree.c\n+++ b/builtin/ls-tree.c\n@@ -168,7 +168,8 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)\n \n \tinit_pathspec(&pathspec, get_pathspec(prefix, argv + 1));\n \tfor (i = 0; i < pathspec.nr; i++)\n-\t\tpathspec.items[i].has_wildcard = 0;\n+\t\tpathspec.items[i].use_wildcard = 0;\n+\tpathspec.has_wildcard = 0;\n \ttree = parse_tree_indirect(sha1);\n \tif (!tree)\n \t\tdie(\"not a tree object\");\ndiff --git a/cache.h b/cache.h\nindex edd5b5a..43d719d 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -509,7 +509,7 @@ struct pathspec {\n \tstruct pathspec_item {\n \t\tconst char *match;\n \t\tint len;\n-\t\tunsigned int has_wildcard:1;\n+\t\tunsigned int use_wildcard:1;\n \t} *items;\n };\n \ndiff --git a/dir.c b/dir.c\nindex 168dad6..91f1502 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -230,7 +230,7 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,\n \t\t\treturn MATCHED_RECURSIVELY;\n \t}\n \n-\tif (item->has_wildcard && !fnmatch(match, name, 0))\n+\tif (item->use_wildcard && !fnmatch(match, name, 0))\n \t\treturn MATCHED_FNMATCH;\n \n \treturn 0;\n@@ -1286,8 +1286,8 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)\n \n \t\titem->match = path;\n \t\titem->len = strlen(path);\n-\t\titem->has_wildcard = !no_wildcard(path);\n-\t\tif (item->has_wildcard)\n+\t\titem->use_wildcard = !no_wildcard(path);\n+\t\tif (item->use_wildcard)\n \t\t\tpathspec->has_wildcard = 1;\n \t}\n \ndiff --git a/tree-walk.c b/tree-walk.c\nindex 322becc..33f749e 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -598,7 +598,7 @@ int tree_entry_interesting(const struct name_entry *entry,\n \t\t\t\t\t&never_interesting))\n \t\t\t\treturn 1;\n \n-\t\t\tif (ps->items[i].has_wildcard) {\n+\t\t\tif (ps->items[i].use_wildcard) {\n \t\t\t\tif (!fnmatch(match + baselen, entry->path, 0))\n \t\t\t\t\treturn 1;\n \n@@ -614,7 +614,7 @@ int tree_entry_interesting(const struct name_entry *entry,\n \t\t}\n \n match_wildcards:\n-\t\tif (!ps->items[i].has_wildcard)\n+\t\tif (!ps->items[i].use_wildcard)\n \t\t\tcontinue;\n \n \t\t/*\n-- \n1.7.5.rc0.131.gfa38c\n"},{"id":"165283","messageId":"BANLkTikJ3=H_OgcNueMjnwwQ2W-2kamf=w@mail.gmail.com","threadId":"27000","inReplyTo":"7vr59gl581.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-04-06T15:54:19Z","receivedAt":"2011-04-06T15:54:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2011/4/6 Junio C Hamano :\n>  * I was looking at the codepaths that would need to be touched in order\n>   to properly support the \"magic pathspec\" we have been discussing, and\n>   am leaning to conclude that all the users of get_pathspec() need to be\n>   rewritten to throw the remainder of argv[] at a function that fills a\n>   struct pathspec (in other words, a combination of get_pathspec() and\n>   init_pathspec()).  Michael's \"alternative approach to grep --full-tree\"\n>   was operating at the get_pathspec() level, but that function is an\n>   interface to return an array of plain-vanilla strings only, and there\n>   is no place to hook richer per-item information on the elements.  We\n>   would need \"struct pathspec\" in the function where we parse the argv[]\n>   and combine its elements with prefix.\n\nI have started working on the conversion, but it may take a while\nbecause in many places pathspec is still assumed a prefix (and handled\nseparately, which is not good for negative pathspec). Fundamental\nsupport for magic pathspec and \"top dir\" notation probably do not need\nget_pathspec() converted to struct pathspec.\n-- \nDuy\n"},{"id":"165289","messageId":"7vfwpvjobl.fsf@alter.siamese.dyndns.org","threadId":"27000","inReplyTo":"BANLkTikJ3=H_OgcNueMjnwwQ2W-2kamf=w@mail.gmail.com","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-06T17:13:02Z","receivedAt":"2011-04-06T17:13:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy writes:\n\n> 2011/4/6 Junio C Hamano :\n>\n> I have started working on the conversion, but it may take a while\n> because in many places pathspec is still assumed a prefix (and handled\n> separately, which is not good for negative pathspec). Fundamental\n> support for magic pathspec and \"top dir\" notation probably do not need\n> get_pathspec() converted to struct pathspec.\n\nI think you meant to say \"If we only want to have 'top dir' magic,\nfundamental support and get_pathspec() conversion are unnecessary\", and I\nagree 100%.\n\nI am actually tempted to add Michael's hack to get_pathspec() only to\nsupport the \"from the top\" (and error out with any other magic---as the\napproach without a proper restructuring will not work with anything but\nthat particular magic), to get the \"add -u\" topic going, and let you (or\nother people who are interested in the pathspec rationalization) later fix\nit up just a small part of existing issues.\n\nThe extensible syntax I have in mind (we only parse in get_pathspec() in\nsuch a tentative version but anything other than the :/ will error out) is\nto use:\n\n (1) Colon, a run of selected non-alpha (i.e. magic signature), an\n optional colon to terminate the magic signature, followed by the\n path, e.g.\n\n - \":/hello.c\" is a path from the top.\n\n - \":!/hello.c\" is path from the top but no globbing.\n\n - \":/!hello.c\" is the same as above.\n\n - \":/::hello.c\" is \":hello.c\" from the top, the second colon\n terminates the magic signature and allows the funny file with a\n leading colon to be named.\n\n - \"::hello.c\" does not have any magic, is the same as \"hello.c\".\n\n (2) Colon, open parenthesis, a comma separated list of words to name\n magic, close parenthesis, followed by the path, e.g. these are the\n long-hand counterparts to the examples in (1)\n\n - \":(top)hello.c\"\n - \":(top,noglob)hello.c\"\n - \":(noglob,top)hello.c\"\n - \":(noglob,top):hello.c\"\n - \":()hello.c\"\n\nAt this point, I am not interested in building the repertoire of magic\nyet, but would want to nail a syntax that is\n\n - concise in common cases (e.g. \"from the top, not a funny name\" is ':/'\n followed by the name);\n - is extensible in the future; and\n - easy to parse and error out on magic we do not understand.\n"},{"id":"165305","messageId":"7vsjtvi2dx.fsf@alter.siamese.dyndns.org","threadId":"27000","inReplyTo":"7vfwpvjobl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-06T19:52:10Z","receivedAt":"2011-04-06T19:52:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I am actually tempted to add Michael's hack to get_pathspec() only to\n> support the \"from the top\" (and error out with any other magic---as the\n> approach without a proper restructuring will not work with anything but\n> that particular magic), to get the \"add -u\" topic going, and let you (or\n> other people who are interested in the pathspec rationalization) later fix\n> it up just a small part of existing issues.\n\nJust a heads-up; it would look like this (no docs, tests, nor \"add -u\"\nmessage updates yet). The entries other than \"top\" should not be in the\nfinal version but I just wanted to make sure that the extensibility would\nlook sane.\n\n\n setup.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n 1 files changed, 99 insertions(+), 2 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 03cd84f..704a930 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -126,6 +126,104 @@ void verify_non_filename(const char *prefix, const char *arg)\n \t \"Use '--' to separate filenames from revisions\", arg);\n }\n \n+/*\n+ * Magic pathspec\n+ *\n+ * NEEDSWORK: These need to be moved to dir.h or even to a new\n+ * pathspec.h when we restructure get_pathspec() users to use the\n+ * \"struct pathspec\" interface.\n+ */\n+#define PATHSPEC_FROMTOP (1<<0)\n+#define PATHSPEC_NOGLOB (1<<1)\n+#define PATHSPEC_ICASE (1<<2)\n+#define PATHSPEC_RECURSIVE (1<<3)\n+#define PATHSPEC_REGEXP (1<<4)\n+\n+struct pathspec_magic {\n+\tunsigned bit;\n+\tchar mnemonic; /* cannot be ':'! */\n+\tconst char *name;\n+} pathspec_magic[] = {\n+\t{ PATHSPEC_FROMTOP, '/', \"top\" },\n+\t{ PATHSPEC_NOGLOB, '!', \"noglob\" },\n+\t{ PATHSPEC_ICASE, '\\0', \"icase\" },\n+\t{ PATHSPEC_RECURSIVE, '*', \"recursive\" },\n+\t{ PATHSPEC_REGEXP, '\\0', \"regexp\" },\n+};\n+\n+/*\n+ * Take an element of a pathspec and check for magic signatures.\n+ * Append the result to the prefix.\n+ *\n+ * For now, we only parse the syntax and throw out anything other than\n+ * \"top\" magic.\n+ *\n+ * NEEDSWORK: This needs to be rewritten when we start migrating\n+ * get_pathspec() users to use the \"struct pathspec\" interface. For\n+ * example, a pathspec element may be marked as case-insensitive, but\n+ * the prefix part must always match literally, and a single stupid\n+ * string cannot express such a case.\n+ */\n+const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)\n+{\n+\tunsigned magic = 0;\n+\tconst char *copyfrom = elt;\n+\tint i;\n+\n+\tif (elt[0] != ':') {\n+\t\t; /* nothing to do */\n+\t} else if (elt[1] == '(') {\n+\t\t/* longhand */\n+\t\tconst char *nextat;\n+\t\tfor (copyfrom = elt + 2;\n+\t\t *copyfrom && *copyfrom != ')';\n+\t\t copyfrom = nextat) {\n+\t\t\tsize_t len = strcspn(copyfrom, \",)\");\n+\t\t\tif (copyfrom[len] == ')')\n+\t\t\t\tnextat = copyfrom + len;\n+\t\t\telse\n+\t\t\t\tnextat = copyfrom + len + 1;\n+\t\t\tif (!len)\n+\t\t\t\tcontinue;\n+\t\t\tfor (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)\n+\t\t\t\tif (strlen(pathspec_magic[i].name) == len &&\n+\t\t\t\t !strncmp(pathspec_magic[i].name, copyfrom, len)) {\n+\t\t\t\t\tmagic |= pathspec_magic[i].bit;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\tif (ARRAY_SIZE(pathspec_magic) <= i)\n+\t\t\t\tdie(\"Invalid pathspec magic '%.*s' in '%s'\",\n+\t\t\t\t (int) len, copyfrom, elt);\n+\t\t}\n+\t\tif (*copyfrom == ')')\n+\t\t\tcopyfrom++;\n+\t} else {\n+\t\t/* shorthand */\n+\t\tfor (copyfrom = elt + 1;\n+\t\t *copyfrom && *copyfrom != ':';\n+\t\t copyfrom++) {\n+\t\t\tchar ch = *copyfrom;\n+\t\t\tfor (i = 0; i < ARRAY_SIZE(pathspec_magic); i++)\n+\t\t\t\tif (pathspec_magic[i].mnemonic == ch) {\n+\t\t\t\t\tmagic |= pathspec_magic[i].bit;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\tif (ARRAY_SIZE(pathspec_magic) <= i)\n+\t\t\t\tbreak;\n+\t\t}\n+\t\tif (*copyfrom == ':')\n+\t\t\tcopyfrom++;\n+\t}\n+\n+\tif (magic & ~(PATHSPEC_FROMTOP))\n+\t\tdie(\"Unsupported pathspec magic in '%s'\", elt);\n+\n+\tif (magic & PATHSPEC_FROMTOP)\n+\t\treturn xstrdup(copyfrom);\n+\telse\n+\t\treturn prefix_path(prefix, prefixlen, copyfrom);\n+}\n+\n const char **get_pathspec(const char *prefix, const char **pathspec)\n {\n \tconst char *entry = *pathspec;\n@@ -147,8 +245,7 @@ const char **get_pathspec(const char *prefix, const char **pathspec)\n \tdst = pathspec;\n \tprefixlen = prefix ? strlen(prefix) : 0;\n \twhile (*src) {\n-\t\tconst char *p = prefix_path(prefix, prefixlen, *src);\n-\t\t*(dst++) = p;\n+\t\t*(dst++) = prefix_pathspec(prefix, prefixlen, *src);\n \t\tsrc++;\n \t}\n \t*dst = NULL;\n"},{"id":"165344","messageId":"4D9D9B60.4030404@drmicha.warpmail.net","threadId":"27000","inReplyTo":"7vfwpvjobl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-07T11:09:20Z","receivedAt":"2011-04-07T11:09:20Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 06.04.2011 19:13:\n> Nguyen Thai Ngoc Duy writes:\n> \n>> 2011/4/6 Junio C Hamano :\n>>\n>> I have started working on the conversion, but it may take a while\n>> because in many places pathspec is still assumed a prefix (and handled\n>> separately, which is not good for negative pathspec). Fundamental\n>> support for magic pathspec and \"top dir\" notation probably do not need\n>> get_pathspec() converted to struct pathspec.\n> \n> I think you meant to say \"If we only want to have 'top dir' magic,\n> fundamental support and get_pathspec() conversion are unnecessary\", and I\n> agree 100%.\n> \n> I am actually tempted to add Michael's hack to get_pathspec() only to\n> support the \"from the top\" (and error out with any other magic---as the\n> approach without a proper restructuring will not work with anything but\n> that particular magic), to get the \"add -u\" topic going, and let you (or\n> other people who are interested in the pathspec rationalization) later fix\n> it up just a small part of existing issues.\n> \n\nJunio, we're in rc phase ;)\n\n> The extensible syntax I have in mind (we only parse in get_pathspec() in\n> such a tentative version but anything other than the :/ will error out) is\n> to use:\n> \n> (1) Colon, a run of selected non-alpha (i.e. magic signature), an\n> optional colon to terminate the magic signature, followed by the\n> path, e.g.\n> \n> - \":/hello.c\" is a path from the top.\n> \n> - \":!/hello.c\" is path from the top but no globbing.\n> \n> - \":/!hello.c\" is the same as above.\n> \n> - \":/::hello.c\" is \":hello.c\" from the top, the second colon\n> terminates the magic signature and allows the funny file with a\n> leading colon to be named.\n> \n> - \"::hello.c\" does not have any magic, is the same as \"hello.c\".\n> \n> (2) Colon, open parenthesis, a comma separated list of words to name\n> magic, close parenthesis, followed by the path, e.g. these are the\n> long-hand counterparts to the examples in (1)\n> \n> - \":(top)hello.c\"\n> - \":(top,noglob)hello.c\"\n> - \":(noglob,top)hello.c\"\n> - \":(noglob,top):hello.c\"\n> - \":()hello.c\"\n> \n\nDo we need the parentheses? I guess we need them to have the magic start\nwith a non-alpha, otherwise we could do with \":top,noglob:hello.c\"\n(which I like but breaks those with a file named \":top:hello.c\").\n\nWhat about these:\n\n:/(noglob)hello.c\n:(top)!hello.c\n\nI.e. do we allow combinations?\n\nDo we need the the second colon at all then? I.e. we can declare \":\"\nnon-magic after seeing it once so that \":(top):hello.c\" would be\n\":hello.c\" at top.\n\n> At this point, I am not interested in building the repertoire of magic\n> yet, but would want to nail a syntax that is\n> \n> - concise in common cases (e.g. \"from the top, not a funny name\" is ':/'\n> followed by the name);\n> - is extensible in the future; and\n> - easy to parse and error out on magic we do not understand.\n> \n\n- and does dot break users with files named xyz.\n\nSpelling out xyz would already nail down the syntax quite a bit. It\nseems you accept to break people with files named \":(top)hello.c\" but\nnot those with files named \":top:hello.c\". Which is OK if it's a\nconscious decision, of course.\n\nMichael\n"},{"id":"165350","messageId":"BANLkTi=sjhWdav6QLguUnr6HE3Roc0NM4Q@mail.gmail.com","threadId":"27000","inReplyTo":"7vfwpvjobl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-04-07T12:51:34Z","receivedAt":"2011-04-07T12:51:34Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Apr 7, 2011 at 12:13 AM, Junio C Hamano wrote:\n> The extensible syntax I have in mind (we only parse in get_pathspec() in\n> such a tentative version but anything other than the :/ will error out) is\n> to use:\n>\n>  (1) Colon, a run of selected non-alpha (i.e. magic signature), an\n>     optional colon to terminate the magic signature, followed by the\n>     path, e.g.\n>\n>     - \":/hello.c\" is a path from the top.\n>\n>     - \":!/hello.c\" is path from the top but no globbing.\n>\n>     - \":/!hello.c\" is the same as above.\n>\n>     - \":/::hello.c\" is \":hello.c\" from the top, the second colon\n>       terminates the magic signature and allows the funny file with a\n>       leading colon to be named.\n>\n>     - \"::hello.c\" does not have any magic, is the same as \"hello.c\".\n\nWhat about \":hello.c\", same as \"hello.c\"? I think we need to reserve\nsome more characters for future extension. The selected set of magic\nin your patch misses my favourite negate magic :)\n\nBy the way, I think we should use a better term than \"magic\". Pathspec modifier?\n\n>  (2) Colon, open parenthesis, a comma separated list of words to name\n>     magic, close parenthesis, followed by the path, e.g. these are the\n>     long-hand counterparts to the examples in (1)\n>\n>     - \":(top)hello.c\"\n>     - \":(top,noglob)hello.c\"\n>     - \":(noglob,top)hello.c\"\n>     - \":(noglob,top):hello.c\"\n>     - \":()hello.c\"\n>\n\nCan we mix short and long magic? It seems impossible in your patch. I\ndon't know if that's the intention.\n\n> At this point, I am not interested in building the repertoire of magic\n> yet, but would want to nail a syntax that is\n>\n>  - concise in common cases (e.g. \"from the top, not a funny name\" is ':/'\n>   followed by the name);\n>  - is extensible in the future; and\n>  - easy to parse and error out on magic we do not understand.\n-- \nDuy\n"},{"id":"165351","messageId":"BANLkTinkdR4gg1=z-b4941Pppfsg+CgD-g@mail.gmail.com","threadId":"27000","inReplyTo":"4D9D9B60.4030404@drmicha.warpmail.net","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-04-07T13:00:11Z","receivedAt":"2011-04-07T13:00:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Apr 7, 2011 at 6:09 PM, Michael J Gruber\n wrote:\n> Junio, we're in rc phase ;)\n\nHe was excited. It's good, we may get this feature sooner :)\n\n>>  (2) Colon, open parenthesis, a comma separated list of words to name\n>>      magic, close parenthesis, followed by the path, e.g. these are the\n>>      long-hand counterparts to the examples in (1)\n>>\n>>      - \":(top)hello.c\"\n>>      - \":(top,noglob)hello.c\"\n>>      - \":(noglob,top)hello.c\"\n>>      - \":(noglob,top):hello.c\"\n>>      - \":()hello.c\"\n>>\n>\n> Do we need the parentheses? I guess we need them to have the magic start\n> with a non-alpha, otherwise we could do with \":top,noglob:hello.c\"\n> (which I like but breaks those with a file named \":top:hello.c\").\n\nThat has something to do with \"easy to parse and catch error\", I\nthink. At the moment \":hello/:world\" is interpreted as \"hello/:world\".\nWithout parentheses, we need to look further for colon and may\naccidentally misinterpret it.\n\n> What about these:\n>\n> :/(noglob)hello.c\n> :(top)!hello.c\n>\n> I.e. do we allow combinations?\n\nI think we should. I don't like typing long pathspecs, just because\nonly remember mnemonic for \"top\" and happen to need \"noglob\" too.\n\n> It seems you accept to break people with files named \":(top)hello.c\" but\n> not those with files named \":top:hello.c\". Which is OK if it's a\n> conscious decision, of course.\n\nAt least once we agree on colon being the magic introducer and general\nrules, we can do these earlier in 1.7.x\n\n - warn people that \":hello.c\" may change semantics in future\n - recommend to escape (\"::hello.c\" instead of \":hello.c\")\n-- \nDuy\n"},{"id":"165378","messageId":"7vvcypeti4.fsf@alter.siamese.dyndns.org","threadId":"27000","inReplyTo":"4D9D9B60.4030404@drmicha.warpmail.net","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-07T19:44:35Z","receivedAt":"2011-04-07T19:44:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Junio C Hamano venit, vidit, dixit 06.04.2011 19:13:\n> ...\n>> I think you meant to say \"If we only want to have 'top dir' magic,\n>> fundamental support and get_pathspec() conversion are unnecessary\", and I\n>> agree 100%.\n>> \n>> I am actually tempted to add Michael's hack to get_pathspec() only to\n>> support the \"from the top\" (and error out with any other magic---as the\n>> approach without a proper restructuring will not work with anything but\n>> that particular magic), to get the \"add -u\" topic going, and let you (or\n>> other people who are interested in the pathspec rationalization) later fix\n>> it up just a small part of existing issues.\n>\n> Junio, we're in rc phase ;)\n\nI know ;-). All the patches I sent out yesterday are to be held until\n1.7.5 ships.\n\nBecause I doubt that the major restructuring we discussed earlier won't be\nready within the 1.7.6 timeframe, I think it would make sense to ship the\nversion of \"git add -u/-A\" update that does not use any confvar but does\nsuggest the use of ':/' (it seems none of us is totally unhappy with that\nnotation) or '.', which I think is a far superiour solution, in 1.7.6.\n\nThat gives us more time to enhance the magic pathspec for later versions,\nbut still will give plenty of time to the users for \"git add -u/-A\"\nmigration planned for 1.8.0 version bump. Hooking at the get_pathspec()\nlevel is a compromise to make that happen, as long as we nailed the syntax\nright so that we do not have to break people's fingers and scripts when we\nredo the logic for the magic pathspec at the right level of the API.\n\n>> (1) Colon, a run of selected non-alpha (i.e. magic signature), an\n>> optional colon to terminate the magic signature, followed by the\n>> path, e.g.\n>> \n>> - \":/hello.c\" is a path from the top.\n>> \n>> - \":!/hello.c\" is path from the top but no globbing.\n>> \n>> - \":/!hello.c\" is the same as above.\n>> \n>> - \":/::hello.c\" is \":hello.c\" from the top, the second colon\n>> terminates the magic signature and allows the funny file with a\n>> leading colon to be named.\n>> \n>> - \"::hello.c\" does not have any magic, is the same as \"hello.c\".\n>> \n>> (2) Colon, open parenthesis, a comma separated list of words to name\n>> magic, close parenthesis, followed by the path, e.g. these are the\n>> long-hand counterparts to the examples in (1)\n>> \n>> - \":(top)hello.c\"\n>> - \":(top,noglob)hello.c\"\n>> - \":(noglob,top)hello.c\"\n>> - \":(noglob,top):hello.c\"\n>> - \":()hello.c\"\n>\n> Do we need the parentheses?\n\nWe bracket spelled-out names, e.g. %(refname), ^{commit}, as opposed to\nnot bracket a short-hand to keep them, eh, short. Also the fact that \":(\"\nis a very unlikely character sequence to begin a pathname will help avoid\nclashing with real pathnames. Short-form magic mnemonic consists only of\nnon-alphanumeric, and \":x\" for any 'x' that is a non-alphanumeric is also\na very unlikely character sequence to begin a pathname.\n\n> What about these:\n>\n> :/(noglob)hello.c\n> :(top)!hello.c\n\nNot interested. It is Ok if mnemonic form is a bit complex to explain, as\nlong as the end result is short to type once the user understands it. When\na long form is involved, the rule should be as simple as possible to help\npeople who haven't taken the time to understand the possible complex short\nform parsing rules that is necessary to keep the short form short.\n\n> Spelling out xyz would already nail down the syntax quite a bit. It\n> seems you accept to break people with files named \":(top)hello.c\" but\n> not those with files named \":top:hello.c\".\n\nI think we actually _do_ break \":top:hello.c\", which the yesterday's patch\nwould parse as a pattern \"top:hello.c\" that does not have any magic, and\nyou would need to quote it, i.e. \"\\:top:hello.c\" or \":::top:hello.c\", if\nyou mean \":top:hello.c\" is the pattern, I think.\n\nI have considered to tweak the syntax definition further to allow you to\nsay \":Makefile\" (which would parse as a pattern \"Makefile\" without any\nmagic with yesterday's patch) and automagically add \"top\" magic to it,\nbecause \"top\" would be the most common thing you would want to say, and\nbecause it would make it look similar to \"HEAD:Makefile\" (the blob object\nin HEAD tree) or \":Makefile\" (the blob object in the index). A three\nliner patch is attached.\n\nBut that would mean that we are not just saying \"a colon ':' followed by a\nnon-alphanumeric is an unlikely sequence\". We would be saying \"a colon\n':' is an unlikely character to begin a pathname\". I think that is going\na bit too far. So we probably would not want to do this.\n\n setup.c | 3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 820ed05..4ebedd4 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -213,6 +213,9 @@ const char *prefix_pathspec(const char *prefix, int prefixlen, const char *elt)\n \t\t}\n \t\tif (*copyfrom == ':')\n \t\t\tcopyfrom++;\n+\t\telse if (copyfrom == elt + 1)\n+\t\t\t/* Special case: \":Makefile\" -> \":/Makefile\" */\n+\t\t\tmagic = PATHSPEC_FROMTOP;\n \t}\n \n \tif (magic & PATHSPEC_FROMTOP)\n"},{"id":"165379","messageId":"7vr59detcu.fsf@alter.siamese.dyndns.org","threadId":"27000","inReplyTo":"7vvcypeti4.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-07T19:47:45Z","receivedAt":"2011-04-07T19:47:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Because I doubt that the major restructuring we discussed earlier won't be\n> ready within the 1.7.6 timeframe,...\n\nI think people involved in this thread have dealt with my bad writing long\nenough and understood what I meant, but just in case, I meant to say that\nI do not think restructuring would be ready to ship with 1.7.6,\nin other words, s/won't/will/ is needed above.\n\nSorry for a noise.\n"},{"id":"165400","messageId":"4D9EC75F.1060901@drmicha.warpmail.net","threadId":"27000","inReplyTo":"7vr59detcu.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2011-04-08T08:29:19Z","receivedAt":"2011-04-08T08:29:19Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 07.04.2011 21:47:\n> Junio C Hamano writes:\n> \n>> Because I doubt that the major restructuring we discussed earlier won't be\n>> ready within the 1.7.6 timeframe,...\n> \n> I think people involved in this thread have dealt with my bad writing long\n> enough and understood what I meant, but just in case, I meant to say that\n> I do not think restructuring would be ready to ship with 1.7.6,\n> in other words, s/won't/will/ is needed above.\n> \n> Sorry for a noise.\n\nNo problem, that was clear from the context. \"Doubt\" is such a weak form\nof negation that many people miss that possible double negation. In\nseveral languages (or even dialects) a double negation is a strong form\nof negation.\n\nThe time line sounds very sane, and I'm with the parentheses. What I'm\nwondering about are :foo and :foo:bar (which have no modifiers to\nparse). I thought you meant to eat the \":\" only in the former and not\nthe latter but I guess I was mistaken. Maybe we can make the \":\" literal\nwhen it can't be parsed, or make it do something (\":/\" as you\nsuggested), because swallowing it and then doing nothing seems doubly bad.\n\nMichael\n"},{"id":"165442","messageId":"7v62qo4jmp.fsf@alter.siamese.dyndns.org","threadId":"27000","inReplyTo":"4D9EC75F.1060901@drmicha.warpmail.net","subject":"Re: [PATCH] pathspec: rename per-item field has_wildcard to use_wildcard","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-08T19:40:14Z","receivedAt":"2011-04-08T19:40:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Maybe we can make the \":\" literal\n> when it can't be parsed, or make it do something (\":/\" as you\n> suggested), because swallowing it and then doing nothing seems doubly bad.\n\nAnother possible tweak would be to make \":\" without even any \"pattern\"\nafter it to become a no-element, in order to allow the same difference\nbetween \"git log\" and \"git log .\" to be expressed from within a\nsubdirectory.\n\nThe difference matters when you have commits created with --allow-empty\nthat are discarded by path based history simplification.\n"}]}