{"thread":{"id":"42509","subject":"[RFC/PATCH] pathspec: allow escaped query values","startedAt":"2016-06-01T23:52:33Z","lastAt":"2016-06-02T19:53:57Z","messageCount":16,"participants":["Stefan Beller","Junio C Hamano","Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"288098","messageId":"20160601235233.21040-1-sbeller@google.com","threadId":"42509","inReplyTo":null,"subject":"[RFC/PATCH] pathspec: allow escaped query values","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-06-01T23:52:33Z","receivedAt":"2016-06-01T23:52:33Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In our own .gitattributes file we have attributes such as:\n\n *.[ch] whitespace=indent,trail,space\n\nWhen querying for attributes we want to be able to ask for the exact\nvalue, i.e.\n\n git ls-files :(attr:whitespace=indent,trail,space)\n\nshould work, but the commas are used in the attr magic to introduce\nthe next attr, such that this query currently fails with\n\nfatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'\n\nThis change allows escaping characters by a backslash, such that the query\n\n git ls-files :(attr:whitespace=indent\\,trail\\,space)\n\nwill match all path that have the value \"indent,trail,space\" for the\nwhitespace attribute. To accomplish this, we need to modify two places.\nFirst `eat_long_magic` needs to not stop early upon seeing a comma or\nclosing paren that is escaped. As a second step we need to remove any\nescaping from the attr value. For now we just remove any backslashes.\n\nCaveat: This doesn't allow for querying for values that have backslashes\nin them, e.g.\n\n git ls-files :(attr:backslashes=\\\\)\n\nthat would ask for matches that have the `backslashes` value set to '\\'.\n\nSigned-off-by: Stefan Beller \n---\n\n * This applies on top of sb/pathspec-label\n * Junio does this come close to what you imagine for escaped commas?\n \n Thanks,\n Stefan\n \n pathspec.c | 21 ++++++++++++++++++++-\n 1 file changed, 20 insertions(+), 1 deletion(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex 0a02255..925f949 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -89,6 +89,22 @@ static void prefix_short_magic(struct strbuf *sb, int prefixlen,\n \tstrbuf_addf(sb, \",prefix:%d)\", prefixlen);\n }\n \n+static char *attr_value_unquote(const char *value)\n+{\n+\tchar *ret = xstrdup(value);\n+\n+\tsize_t value_len = strlen(ret);\n+\tsize_t len = strcspn(ret, \"\\\\\");\n+\n+\twhile (len != value_len) {\n+\t\tmemmove(ret + len, ret + len + 1, value_len - len);\n+\t\tvalue_len--;\n+\t\tlen += strcspn(ret + len, \"\\\\\");\n+\t}\n+\n+\treturn ret;\n+}\n+\n static void parse_pathspec_attr_match(struct pathspec_item *item, const char *value)\n {\n \tstruct string_list_item *si;\n@@ -132,7 +148,7 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va\n \t\t\t\tam->match_mode = MATCH_SET;\n \t\t\telse {\n \t\t\t\tam->match_mode = MATCH_VALUE;\n-\t\t\t\tam->value = xstrdup(&attr[attr_len + 1]);\n+\t\t\t\tam->value = attr_value_unquote(&attr[attr_len + 1]);\n \t\t\t\tif (strchr(am->value, '\\\\'))\n \t\t\t\t\tdie(_(\"attr spec values must not contain backslashes\"));\n \t\t\t}\n@@ -167,6 +183,9 @@ static void eat_long_magic(struct pathspec_item *item, const char *elt,\n \t *copyfrom && *copyfrom != ')';\n \t copyfrom = nextat) {\n \t\tsize_t len = strcspn(copyfrom, \",)\");\n+\t\twhile (len > 0 && copyfrom[len - 1] == '\\\\'\n+\t\t && (copyfrom[len] == ',' || copyfrom[len] == ')'))\n+\t\t\tlen += strcspn(copyfrom + len + 1, \",)\") + 1;\n \t\tif (copyfrom[len] == ',')\n \t\t\tnextat = copyfrom + len + 1;\n \t\telse\n-- \n2.8.2.124.g24a9db3\n"},{"id":"288101","messageId":"xmqq37owwhti.fsf@gitster.mtv.corp.google.com","threadId":"42509","inReplyTo":"20160601235233.21040-1-sbeller@google.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-02T00:33:13Z","receivedAt":"2016-06-02T00:33:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> This change allows escaping characters by a backslash, such that the query\n>\n> git ls-files :(attr:whitespace=indent\\,trail\\,space)\n>\n> will match all path that have the value \"indent,trail,space\" for the\n> whitespace attribute. To accomplish this, we need to modify two places.\n> First `eat_long_magic` needs to not stop early upon seeing a comma or\n> closing paren that is escaped. As a second step we need to remove any\n> escaping from the attr value. For now we just remove any backslashes.\n>\n> Caveat: This doesn't allow for querying for values that have backslashes\n> in them, e.g.\n>\n> git ls-files :(attr:backslashes=\\\\)\n>\n> that would ask for matches that have the `backslashes` value set to '\\'.\n\n> Signed-off-by: Stefan Beller \n> ---\n>\n> * This applies on top of sb/pathspec-label\n> * Junio does this come close to what you imagine for escaped commas?\n\nI, being lazy, would have used %44 for comma, which would have\navoided touching higher level of the callchain. But \"a backslash\nalways quotes the next byte, if you want to express a backslash,\ndouble it\" would probably be a more end-user friendly quoting\nmechanism.\n\nI do not offhand understand why the second example does not show the\npaths with backslash attribute set to a single backslash, though.\n\n> -\t\t\t\tam->value = xstrdup(&attr[attr_len + 1]);\n> +\t\t\t\tam->value = attr_value_unquote(&attr[attr_len + 1]);\n> \t\t\t\tif (strchr(am->value, '\\\\'))\n> \t\t\t\t\tdie(_(\"attr spec values must not contain backslashes\"));\n\nIOW, the \"backslash is forbidden for now\" IIUC was added there so\nthat we can introduce a quoting like this--once we decided that\nquoting mechanism is via backslashes and have quoting support,\nshouldn't the \"values must not have backslash\" just go?\n\n> \t *copyfrom && *copyfrom != ')';\n> \t copyfrom = nextat) {\n> \t\tsize_t len = strcspn(copyfrom, \",)\");\n> +\t\twhile (len > 0 && copyfrom[len - 1] == '\\\\'\n> +\t\t && (copyfrom[len] == ',' || copyfrom[len] == ')'))\n> +\t\t\tlen += strcspn(copyfrom + len + 1, \",)\") + 1;\n\nGood that we can use ')' in values, too, but I think this gets this\ncase wrong:\n\n\t:(attr:foo=\\\\,icase)\n\nwhere the value for 'foo' wants to be a single backslash, and comma\nis to introduce another magic, not part of the value for 'foo'.\n\nIf you want to do the \"backslash unconditionally quotes the next\nbyte no matter what is quoted\", you'd need to lose the strcspn()\nand iterate over the string yourself, I would think.\n\n> \t\tif (copyfrom[len] == ',')\n> \t\t\tnextat = copyfrom + len + 1;\n> \t\telse\n"},{"id":"288102","messageId":"574F800D.6070107@ramsayjones.plus.com","threadId":"42509","inReplyTo":"20160601235233.21040-1-sbeller@google.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-02T00:38:37Z","receivedAt":"2016-06-02T00:38:37Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/06/16 00:52, Stefan Beller wrote:\n> In our own .gitattributes file we have attributes such as:\n> \n> *.[ch] whitespace=indent,trail,space\n> \n> When querying for attributes we want to be able to ask for the exact\n> value, i.e.\n> \n> git ls-files :(attr:whitespace=indent,trail,space)\n> \n> should work, but the commas are used in the attr magic to introduce\n> the next attr, such that this query currently fails with\n> \n> fatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'\n> \n> This change allows escaping characters by a backslash, such that the query\n> \n> git ls-files :(attr:whitespace=indent\\,trail\\,space)\n\nNot having given this much thought at all, but the question which comes\nto mind is: can you use some other separator for the -s rather than\na comma? That way you don't need to quote them in the part of the\n-spec.\n\n(I dunno, maybe use ; or : instead?)\n\nATB,\nRamsay Jones\n"},{"id":"288105","messageId":"CAGZ79kZSB-r-GzS1WVtLk-sX068PDXQ1h-1FV-8YZFGGsr3hWQ@mail.gmail.com","threadId":"42509","inReplyTo":"574F800D.6070107@ramsayjones.plus.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-06-02T02:20:39Z","receivedAt":"2016-06-02T02:20:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jun 1, 2016 at 5:38 PM, Ramsay Jones\n wrote:\n>\n>\n> On 02/06/16 00:52, Stefan Beller wrote:\n>> In our own .gitattributes file we have attributes such as:\n>>\n>> *.[ch] whitespace=indent,trail,space\n>>\n>> When querying for attributes we want to be able to ask for the exact\n>> value, i.e.\n>>\n>> git ls-files :(attr:whitespace=indent,trail,space)\n>>\n>> should work, but the commas are used in the attr magic to introduce\n>> the next attr, such that this query currently fails with\n>>\n>> fatal: Invalid pathspec magic 'trail' in ':(attr:whitespace=indent,trail,space)'\n>>\n>> This change allows escaping characters by a backslash, such that the query\n>>\n>> git ls-files :(attr:whitespace=indent\\,trail\\,space)\n>\n> Not having given this much thought at all, but the question which comes\n> to mind is: can you use some other separator for the -s rather than\n> a comma? That way you don't need to quote them in the part of the\n> -spec.\n>\n> (I dunno, maybe use ; or : instead?)\n\nThat was essentially my proposal as well (just that I said \"white\nspaces\" instead if ':' or ';'),\nbut it is not a good idea according to Junio as we don't want to need\nknowledge of the attr\nsyntax here in the pathspec matching IIUC. We rather want \"I want to\ntransport this exact\nstring for the match, but to go through the pathspec magic, we need to\napply escaping of\nthe exact string. That way we can change the attr system later without\nhaving to worry\nabout the syntax replacements done in the pathspecs.\n\n\n\n>\n> ATB,\n> Ramsay Jones\n>\n>\n>\n"},{"id":"288106","messageId":"CAGZ79kbpdgtt89Wm=YL0f+-zMnEhoa3xvWtXxFFw38tX8GpsaQ@mail.gmail.com","threadId":"42509","inReplyTo":"xmqq37owwhti.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-06-02T02:23:18Z","receivedAt":"2016-06-02T02:23:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Jun 1, 2016 at 5:33 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> This change allows escaping characters by a backslash, such that the query\n>>\n>> git ls-files :(attr:whitespace=indent\\,trail\\,space)\n>>\n>> will match all path that have the value \"indent,trail,space\" for the\n>> whitespace attribute. To accomplish this, we need to modify two places.\n>> First `eat_long_magic` needs to not stop early upon seeing a comma or\n>> closing paren that is escaped. As a second step we need to remove any\n>> escaping from the attr value. For now we just remove any backslashes.\n>>\n>> Caveat: This doesn't allow for querying for values that have backslashes\n>> in them, e.g.\n>>\n>> git ls-files :(attr:backslashes=\\\\)\n>>\n>> that would ask for matches that have the `backslashes` value set to '\\'.\n>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> * This applies on top of sb/pathspec-label\n>> * Junio does this come close to what you imagine for escaped commas?\n>\n> I, being lazy, would have used %44 for comma, which would have\n> avoided touching higher level of the callchain. But \"a backslash\n> always quotes the next byte, if you want to express a backslash,\n> double it\" would probably be a more end-user friendly quoting\n> mechanism.\n\nHeh, okay.\n\n>\n> I do not offhand understand why the second example does not show the\n> paths with backslash attribute set to a single backslash, though.\n>\n>> - am->value = xstrdup(&attr[attr_len + 1]);\n>> + am->value = attr_value_unquote(&attr[attr_len + 1]);\n>> if (strchr(am->value, '\\\\'))\n>> die(_(\"attr spec values must not contain backslashes\"));\n>\n> IOW, the \"backslash is forbidden for now\" IIUC was added there so\n> that we can introduce a quoting like this--once we decided that\n> quoting mechanism is via backslashes and have quoting support,\n> shouldn't the \"values must not have backslash\" just go?\n\nRight this can go now.\n\n>\n>> *copyfrom && *copyfrom != ')';\n>> copyfrom = nextat) {\n>> size_t len = strcspn(copyfrom, \",)\");\n>> + while (len > 0 && copyfrom[len - 1] == '\\\\'\n>> + && (copyfrom[len] == ',' || copyfrom[len] == ')'))\n>> + len += strcspn(copyfrom + len + 1, \",)\") + 1;\n>\n> Good that we can use ')' in values, too, but I think this gets this\n> case wrong:\n>\n> :(attr:foo=\\\\,icase)\n>\n> where the value for 'foo' wants to be a single backslash, and comma\n> is to introduce another magic, not part of the value for 'foo'.\n\nRight, with this patch this would be interpreted as\n\n foo equal to ',icase'\n\n>\n> If you want to do the \"backslash unconditionally quotes the next\n> byte no matter what is quoted\", you'd need to lose the strcspn()\n> and iterate over the string yourself, I would think.\n\nOkay, will do that instead.\n\n>\n>> if (copyfrom[len] == ',')\n>> nextat = copyfrom + len + 1;\n>> else\n"},{"id":"288108","messageId":"xmqqy46ouorn.fsf@gitster.mtv.corp.google.com","threadId":"42509","inReplyTo":"574F800D.6070107@ramsayjones.plus.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-02T05:46:04Z","receivedAt":"2016-06-02T05:46:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Not having given this much thought at all, but the question which comes\n> to mind is: can you use some other separator for the -s rather than\n> a comma? That way you don't need to quote them in the part of the\n> -spec.\n>\n> (I dunno, maybe use ; or : instead?)\n\nThere are two kinds of comma involved in this discussion.\n\n * Multiple pathspec magic can be attached to augment the way\n selects paths. \":(,,...)\" is\n the syntax, and are things like \"icase\" (select the path\n that matches case-insensitively), \"top\" ( must\n match from the top level of the working tree, even when you are\n running the command from a subdirectory). We added a new kind of\n whose syntax is \"attr:VAR=VAL\" recently, which says \"not\n only must match the path, in order to be selected, the\n path must have the attribute VAR with value VAL\".\n\n The comma that separates multiple magic is not something you can\n change now; it has been with us since v1.7.6 (Jun 2011)\n\n * My example wanted to use the attr:VAR=VAL form to select those\n paths that has one specific string as the value for whitespace\n attribute, i.e. VAR in this case is \"whitespace\". The value for\n whitespace attribute determines what kind of whitespace anomalies\n are considered as errors by \"git apply\" and \"git diff\", and it is\n formed by concatenating things like \"indent-with-non-tab\" (starts\n a line with more than 8 consecutive SPs), \"space-before-tab\" (a\n SP appears immediately before HT in the indent), etc., with a\n comma in between.\n\n The comma that separates various kinds of whitespace errors is\n not something you can change now; it has been with us since\n v1.5.4 (Feb 2008).\n\nSo using different separator is not a viable solution.\n"},{"id":"288129","messageId":"57505105.2000801@ramsayjones.plus.com","threadId":"42509","inReplyTo":"xmqqy46ouorn.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-02T15:30:13Z","receivedAt":"2016-06-02T15:30:13Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/06/16 06:46, Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n>> Not having given this much thought at all, but the question which comes\n>> to mind is: can you use some other separator for the -s rather than\n>> a comma? That way you don't need to quote them in the part of the\n>> -spec.\n>>\n>> (I dunno, maybe use ; or : instead?)\n> \n> There are two kinds of comma involved in this discussion.\n> \n> * Multiple pathspec magic can be attached to augment the way\n> selects paths. \":(,,...)\" is\n> the syntax, and are things like \"icase\" (select the path\n> that matches case-insensitively), \"top\" ( must\n> match from the top level of the working tree, even when you are\n> running the command from a subdirectory). We added a new kind of\n> whose syntax is \"attr:VAR=VAL\" recently, which says \"not\n> only must match the path, in order to be selected, the\n> path must have the attribute VAR with value VAL\".\n> \n> The comma that separates multiple magic is not something you can\n> change now; it has been with us since v1.7.6 (Jun 2011)\n> \n> * My example wanted to use the attr:VAR=VAL form to select those\n> paths that has one specific string as the value for whitespace\n> attribute, i.e. VAR in this case is \"whitespace\". The value for\n> whitespace attribute determines what kind of whitespace anomalies\n> are considered as errors by \"git apply\" and \"git diff\", and it is\n> formed by concatenating things like \"indent-with-non-tab\" (starts\n> a line with more than 8 consecutive SPs), \"space-before-tab\" (a\n> SP appears immediately before HT in the indent), etc., with a\n> comma in between.\n> \n> The comma that separates various kinds of whitespace errors is\n> not something you can change now; it has been with us since\n> v1.5.4 (Feb 2008).\n> \n> So using different separator is not a viable solution.\n\nAh, OK, makes sense. Note that I have not used 'pathspec magic' or\nthe attribute system in git (never felt/had the need)! ;-)\n\nSo, at risk of annoying you, let me continue in my ignorance a little\nlonger and ask: even if you have to protect all of this 'magic' from\nthe shell with '/\" quoting, could you not use (nested) quotes to\nprotect the part of an ? For example:\n\n git ls-files ':(attr:whitespace=\"indent,trail,space\",icase)'\n\n[Hmm, I don't seem to be able to find documentation on the syntax\nof these features (apart from the code, of course).]\n\nATB,\nRamsay Jones\n"},{"id":"288132","messageId":"xmqqoa7jvag0.fsf@gitster.mtv.corp.google.com","threadId":"42509","inReplyTo":"57505105.2000801@ramsayjones.plus.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-02T16:10:07Z","receivedAt":"2016-06-02T16:10:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> So, at risk of annoying you, let me continue in my ignorance a little\n> longer and ask: even if you have to protect all of this 'magic' from\n> the shell with '/\" quoting, could you not use (nested) quotes to\n> protect the part of an ? For example:\n>\n> git ls-files ':(attr:whitespace=\"indent,trail,space\",icase)'\n\nThat would be workable, I would think. Before attr:VAR=VAL\nextention, supported pathspec were only single lowercase-ascii\nalphabet tokens, so nobody would have used \" as a part of magic. So\nquting with double-quote pair would work.\n\nYou'd need to come up with a way to quote a double quote that\nhappens to be a part of VAL somehow, though. I think attribute\nvalue is limited to a string with non-whitespace letters; even\nthough the built-in attributes that have defined meaning to the Git\nitself may not use values with letters beyond [-a-zA-Z0-9,], end\nusers and projects can add arbitrary values within the allowed\nsyntax, so it is not unconceivable that some project may have a\ncustom attribute that lists forbidden characters in a path with\n\n\t=== .gitattributes ===\n *.txt forbidden=`\"\n\nthat tells their documentation cannot have these letters in it, or\nsomething like that.\n"},{"id":"288142","messageId":"57507DFD.6010800@ramsayjones.plus.com","threadId":"42509","inReplyTo":"xmqqoa7jvag0.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-02T18:42:05Z","receivedAt":"2016-06-02T18:42:05Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/06/16 17:10, Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n>> So, at risk of annoying you, let me continue in my ignorance a little\n>> longer and ask: even if you have to protect all of this 'magic' from\n>> the shell with '/\" quoting, could you not use (nested) quotes to\n>> protect the part of an ? For example:\n>>\n>> git ls-files ':(attr:whitespace=\"indent,trail,space\",icase)'\n> \n> That would be workable, I would think. Before attr:VAR=VAL\n> extention, supported pathspec were only single lowercase-ascii\n> alphabet tokens, so nobody would have used \" as a part of magic. So\n> quting with double-quote pair would work.\n\nI was thinking about both ' and \", so that you could do:\n\n $ ./args ':(attr:whitespace=\"indent,trail,space\",icase)'\n 1::(attr:whitespace=\"indent,trail,space\",icase)\n\n $ ./args \":(attr:whitespace='indent,trail,space',icase)\"\n 1::(attr:whitespace='indent,trail,space',icase)\n\n $ p=':(attr:whitespace=\"indent,trail,space\",icase)'\n $ ./args \"$p\"\n 1::(attr:whitespace=\"indent,trail,space\",icase)\n\n $ p=\":(attr:whitespace=\\\"indent,trail,space\\\",icase)\"\n $ ./args \"$p\"\n 1::(attr:whitespace=\"indent,trail,space\",icase)\n\nbut limiting it to \" would probably be OK too.\n\n> You'd need to come up with a way to quote a double quote that\n> happens to be a part of VAL somehow, though.\n\nYes I was assuming \\ quoting as well - I just want to reduce the\nneed for such quoting (especially on windows).\n\n> I think attribute\n> value is limited to a string with non-whitespace letters; even\n> though the built-in attributes that have defined meaning to the Git\n> itself may not use values with letters beyond [-a-zA-Z0-9,], end\n> users and projects can add arbitrary values within the allowed\n> syntax, so it is not unconceivable that some project may have a\n> custom attribute that lists forbidden characters in a path with\n> \n> \t=== .gitattributes ===\n> *.txt forbidden=`\"\n> \n> that tells their documentation cannot have these letters in it, or\n> something like that.\n\nHeh, yeah, that gets ugly:\n\n $ ./args \":(attr:*.txt forbidden=\\'\\\\\\\",icase)\"\n 1::(attr:*.txt forbidden=\\'\\\",icase)\n\n $ ./args ':(attr:*.txt forbidden=\\'\\''\\\",icase)'\n 1::(attr:*.txt forbidden=\\'\\\",icase)\n\n[Note the initial ' 1:' above is output from args]\n\nATB,\nRamsay Jones\n"},{"id":"288144","messageId":"CAPc5daWQYRbV_Oc-xCKE2R_O=aMBm_g_CLB=RPUam7EtbGPTgQ@mail.gmail.com","threadId":"42509","inReplyTo":"57507DFD.6010800@ramsayjones.plus.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-02T18:58:22Z","receivedAt":"2016-06-02T18:58:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Thu, Jun 2, 2016 at 11:42 AM, Ramsay Jones\n wrote:\n>>\n>> That would be workable, I would think. Before attr:VAR=VAL\n>> extention, supported pathspec were only single lowercase-ascii\n>> alphabet tokens, so nobody would have used \" as a part of magic. So\n>> quting with double-quote pair would work.\n>\n> I was thinking about both ' and \", so that you could do:\n\nYes, I understood your suggestion as such. Quoting like shells would work\nwithout breaking backward compatibility for the same reason quoting with\ndouble-quote and backslash only without supporting single-quotes would\nwork.\n"},{"id":"288145","messageId":"CAGZ79ka3ku7x7BhBZJ_uHjS0pNLT7NFACUVqNKi=DvtOkSN_bg@mail.gmail.com","threadId":"42509","inReplyTo":"57507DFD.6010800@ramsayjones.plus.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-06-02T19:04:29Z","receivedAt":"2016-06-02T19:04:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jun 2, 2016 at 11:42 AM, Ramsay Jones\n wrote:\n>\n>\n> On 02/06/16 17:10, Junio C Hamano wrote:\n>> Ramsay Jones writes:\n>>\n>>> So, at risk of annoying you, let me continue in my ignorance a little\n>>> longer and ask: even if you have to protect all of this 'magic' from\n>>> the shell with '/\" quoting, could you not use (nested) quotes to\n>>> protect the part of an ? For example:\n>>>\n>>> git ls-files ':(attr:whitespace=\"indent,trail,space\",icase)'\n>>\n>> That would be workable, I would think. Before attr:VAR=VAL\n>> extention, supported pathspec were only single lowercase-ascii\n>> alphabet tokens, so nobody would have used \" as a part of magic. So\n>> quting with double-quote pair would work.\n>\n> I was thinking about both ' and \", so that you could do:\n>\n> $ ./args ':(attr:whitespace=\"indent,trail,space\",icase)'\n> 1::(attr:whitespace=\"indent,trail,space\",icase)\n>\n> $ ./args \":(attr:whitespace='indent,trail,space',icase)\"\n> 1::(attr:whitespace='indent,trail,space',icase)\n>\n> $ p=':(attr:whitespace=\"indent,trail,space\",icase)'\n> $ ./args \"$p\"\n> 1::(attr:whitespace=\"indent,trail,space\",icase)\n>\n> $ p=\":(attr:whitespace=\\\"indent,trail,space\\\",icase)\"\n> $ ./args \"$p\"\n> 1::(attr:whitespace=\"indent,trail,space\",icase)\n>\n> but limiting it to \" would probably be OK too.\n>\n>> You'd need to come up with a way to quote a double quote that\n>> happens to be a part of VAL somehow, though.\n>\n> Yes I was assuming \\ quoting as well - I just want to reduce the\n> need for such quoting (especially on windows).\n>\n>> I think attribute\n>> value is limited to a string with non-whitespace letters; even\n>> though the built-in attributes that have defined meaning to the Git\n>> itself may not use values with letters beyond [-a-zA-Z0-9,], end\n>> users and projects can add arbitrary values within the allowed\n>> syntax, so it is not unconceivable that some project may have a\n>> custom attribute that lists forbidden characters in a path with\n>>\n>> === .gitattributes ===\n>> *.txt forbidden=`\"\n\nWe restrict the 'forbidden' to follow [-a-zA-Z0-9,], so we could enforce\nit for the values, too.\n\n\n>\n> $ ./args \":(attr:*.txt forbidden=\\'\\\\\\\",icase)\"\n> 1::(attr:*.txt forbidden=\\'\\\",icase)\n\nYou should lose the *.txt in there, but put it at the back\n\n> $ ./args \":(attr:forbidden=\\'\\\\\\\",icase)*.txt\"\n\n>\n> $ ./args ':(attr:*.txt forbidden=\\'\\''\\\",icase)'\n> 1::(attr:*.txt forbidden=\\'\\\",icase)\n\nI see, so quoting by \" or ' is preferred. What if the user\nwants to do a\n forbidden=',\"\n\nso we have to escape those in there, such as\n\n ./args ':(attr:\"forbidden=\\',\\\"\")'\n\nWe need to escape both ' and \", one for the outer\nshell and one for Git parsing.\n\nDoes that seem ok?\n"},{"id":"288147","messageId":"xmqqmvn3tmnm.fsf@gitster.mtv.corp.google.com","threadId":"42509","inReplyTo":"CAPc5daWQYRbV_Oc-xCKE2R_O=aMBm_g_CLB=RPUam7EtbGPTgQ@mail.gmail.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-02T19:29:17Z","receivedAt":"2016-06-02T19:29:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> On Thu, Jun 2, 2016 at 11:42 AM, Ramsay Jones\n> wrote:\n>>>\n>>> That would be workable, I would think. Before attr:VAR=VAL\n>>> extention, supported pathspec were only single lowercase-ascii\n>>> alphabet tokens, so nobody would have used \" as a part of magic. So\n>>> quting with double-quote pair would work.\n>>\n>> I was thinking about both ' and \", so that you could do:\n>\n> Yes, I understood your suggestion as such. Quoting like shells would work\n> without breaking backward compatibility for the same reason quoting with\n> double-quote and backslash only without supporting single-quotes would\n> work.\n\nHaving said that, \"It would work\" does not have to mean \"Hence we\nmust do it that way\" at all. Quoting character pairs make the\nparsing and unquoting significantly more complex.\n\nAs you said, not many people used attributes and pathspec magic, and\nI do not think those who want to use the new \"further limits with\nattributes\" magic, envisioned primarily to be those who want to give\nclasses to submodules, have compelling reason to name their classes\nwith anything but lowercase-ascii-alphabet tokens. So for a practical\npurposes, I'd rather see Stefan\n\n * just implement backquote-blindly-passes-the-next-byte and nothing\n more elaborate; and\n\n * forbid [^-a-z0-9,_] from being used in the value part in the\n attr:VAR=VAL magic.\n\nat least for now, and concentrate more on the other more important\nparts of the submodule enhancement topic.\n\nThat way, those who will start using attr:VAR=VAL magic will stick\nthemselves to lowercase-ascii-alphabet tokens for now (simply\nbecause an attribut that has the forbidden characters in its value\nwould not be usable with the magic), and we can later extend the\nmagic syntax parser in a backward compatible way to allow paired\nquotes and other \"more convenient\" syntax.\n\n\n[Footnote]\n\n*1* The reason I prefer to keep the initially allowed value\ncharacters narrow is because I envision that something like\n\n\t:(attr:VAR=())\n\nmay become necessary, and do not want to promise users that open or\nclose parentheses will forever be taken literally.\n"},{"id":"288149","messageId":"57508C85.1090503@ramsayjones.plus.com","threadId":"42509","inReplyTo":"CAGZ79ka3ku7x7BhBZJ_uHjS0pNLT7NFACUVqNKi=DvtOkSN_bg@mail.gmail.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-02T19:44:05Z","receivedAt":"2016-06-02T19:44:05Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/06/16 20:04, Stefan Beller wrote:\n> On Thu, Jun 2, 2016 at 11:42 AM, Ramsay Jones\n> wrote:\n>>\n>>\n>> On 02/06/16 17:10, Junio C Hamano wrote:\n>>> Ramsay Jones writes:\n>>>\n>>>> So, at risk of annoying you, let me continue in my ignorance a little\n>>>> longer and ask: even if you have to protect all of this 'magic' from\n>>>> the shell with '/\" quoting, could you not use (nested) quotes to\n>>>> protect the part of an ? For example:\n>>>>\n>>>> git ls-files ':(attr:whitespace=\"indent,trail,space\",icase)'\n>>>\n>>> That would be workable, I would think. Before attr:VAR=VAL\n>>> extention, supported pathspec were only single lowercase-ascii\n>>> alphabet tokens, so nobody would have used \" as a part of magic. So\n>>> quting with double-quote pair would work.\n>>\n>> I was thinking about both ' and \", so that you could do:\n>>\n>> $ ./args ':(attr:whitespace=\"indent,trail,space\",icase)'\n>> 1::(attr:whitespace=\"indent,trail,space\",icase)\n>>\n>> $ ./args \":(attr:whitespace='indent,trail,space',icase)\"\n>> 1::(attr:whitespace='indent,trail,space',icase)\n>>\n>> $ p=':(attr:whitespace=\"indent,trail,space\",icase)'\n>> $ ./args \"$p\"\n>> 1::(attr:whitespace=\"indent,trail,space\",icase)\n>>\n>> $ p=\":(attr:whitespace=\\\"indent,trail,space\\\",icase)\"\n>> $ ./args \"$p\"\n>> 1::(attr:whitespace=\"indent,trail,space\",icase)\n>>\n>> but limiting it to \" would probably be OK too.\n>>\n>>> You'd need to come up with a way to quote a double quote that\n>>> happens to be a part of VAL somehow, though.\n>>\n>> Yes I was assuming \\ quoting as well - I just want to reduce the\n>> need for such quoting (especially on windows).\n>>\n>>> I think attribute\n>>> value is limited to a string with non-whitespace letters; even\n>>> though the built-in attributes that have defined meaning to the Git\n>>> itself may not use values with letters beyond [-a-zA-Z0-9,], end\n>>> users and projects can add arbitrary values within the allowed\n>>> syntax, so it is not unconceivable that some project may have a\n>>> custom attribute that lists forbidden characters in a path with\n>>>\n>>> === .gitattributes ===\n>>> *.txt forbidden=`\"\n> \n> We restrict the 'forbidden' to follow [-a-zA-Z0-9,], so we could enforce\n> it for the values, too.\n> \n> \n>>\n>> $ ./args \":(attr:*.txt forbidden=\\'\\\\\\\",icase)\"\n>> 1::(attr:*.txt forbidden=\\'\\\",icase)\n> \n> You should lose the *.txt in there, but put it at the back\n\nAh, yes, just shows my ignorance of the attribute system!\n\n> \n>> $ ./args \":(attr:forbidden=\\'\\\\\\\",icase)*.txt\"\n> \n>>\n>> $ ./args ':(attr:*.txt forbidden=\\'\\''\\\",icase)'\n>> 1::(attr:*.txt forbidden=\\'\\\",icase)\n> \n> I see, so quoting by \" or ' is preferred. What if the user\n> wants to do a\n\nI think Junio wants to go with just \" quoting (see other thread).\n\n> forbidden=',\"\n> \n> so we have to escape those in there, such as\n> \n> ./args ':(attr:\"forbidden=\\',\\\"\")'\n\nNo, that won't work (\" is not terminated), try this:\n\n $ ./args ':(attr:\"forbidden='\\'',\\\"\")'\n 1::(attr:\"forbidden=',\\\"\")\n $ \n\n $ ./args \":(attr:\\\"forbidden=',\\\\\\\"\\\")\"\n 1::(attr:\"forbidden=',\\\"\")\n $ \n\n[half of the problem is just getting past the shell]\n\nATB,\nRamsay Jones\n"},{"id":"288150","messageId":"xmqqeg8ftlvk.fsf@gitster.mtv.corp.google.com","threadId":"42509","inReplyTo":"57508C85.1090503@ramsayjones.plus.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-02T19:46:07Z","receivedAt":"2016-06-02T19:46:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> I think Junio wants to go with just \" quoting (see other thread).\n\nNo. I meant just \\ quoting.\n"},{"id":"288151","messageId":"57508E75.8050501@ramsayjones.plus.com","threadId":"42509","inReplyTo":"xmqqmvn3tmnm.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-02T19:52:21Z","receivedAt":"2016-06-02T19:52:21Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/06/16 20:29, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> On Thu, Jun 2, 2016 at 11:42 AM, Ramsay Jones\n>> wrote:\n>>>>\n>>>> That would be workable, I would think. Before attr:VAR=VAL\n>>>> extention, supported pathspec were only single lowercase-ascii\n>>>> alphabet tokens, so nobody would have used \" as a part of magic. So\n>>>> quting with double-quote pair would work.\n>>>\n>>> I was thinking about both ' and \", so that you could do:\n>>\n>> Yes, I understood your suggestion as such. Quoting like shells would work\n>> without breaking backward compatibility for the same reason quoting with\n>> double-quote and backslash only without supporting single-quotes would\n>> work.\n> \n> Having said that, \"It would work\" does not have to mean \"Hence we\n> must do it that way\" at all. Quoting character pairs make the\n> parsing and unquoting significantly more complex.\n> \n> As you said, not many people used attributes and pathspec magic, and\n> I do not think those who want to use the new \"further limits with\n> attributes\" magic, envisioned primarily to be those who want to give\n> classes to submodules, have compelling reason to name their classes\n> with anything but lowercase-ascii-alphabet tokens. So for a practical\n> purposes, I'd rather see Stefan\n> \n> * just implement backquote-blindly-passes-the-next-byte and nothing\n> more elaborate; and\n> \n> * forbid [^-a-z0-9,_] from being used in the value part in the\n> attr:VAR=VAL magic.\n> \n> at least for now, and concentrate more on the other more important\n> parts of the submodule enhancement topic.\n\nOK, that reasonable. I didn't mean to derail Stefan's development!\n\nATB,\nRamsay Jones\n\n> \n> That way, those who will start using attr:VAR=VAL magic will stick\n> themselves to lowercase-ascii-alphabet tokens for now (simply\n> because an attribut that has the forbidden characters in its value\n> would not be usable with the magic), and we can later extend the\n> magic syntax parser in a backward compatible way to allow paired\n> quotes and other \"more convenient\" syntax.\n> \n> \n> [Footnote]\n> \n> *1* The reason I prefer to keep the initially allowed value\n> characters narrow is because I envision that something like\n> \n> \t:(attr:VAR=())\n> \n> may become necessary, and do not want to promise users that open or\n> close parentheses will forever be taken literally.\n> \n> \n"},{"id":"288152","messageId":"57508ED5.9020509@ramsayjones.plus.com","threadId":"42509","inReplyTo":"xmqqeg8ftlvk.fsf@gitster.mtv.corp.google.com","subject":"Re: [RFC/PATCH] pathspec: allow escaped query values","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-06-02T19:53:57Z","receivedAt":"2016-06-02T19:53:57Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/06/16 20:46, Junio C Hamano wrote:\n> Ramsay Jones writes:\n> \n>> I think Junio wants to go with just \" quoting (see other thread).\n> \n> No. I meant just \\ quoting.\n\nYes, sorry, I only just read your last email on the other thread.\n\nATB,\nRamsay Jones\n"}]}