{"thread":{"id":"32524","subject":"[PATCH] attr: allow pattern escape using backslashes","startedAt":"2013-01-04T14:46:49Z","lastAt":"2013-02-27T15:42:20Z","messageCount":4,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"205968","messageId":"1357310809-4771-1-git-send-email-pclouds@gmail.com","threadId":"32524","inReplyTo":null,"subject":"[PATCH] attr: allow pattern escape using backslashes","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-01-04T14:46:49Z","receivedAt":"2013-01-04T14:46:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Patterns in .gitattributes are separated by whitespaces, which makes\nit impossible to specify exact spaces in the pattern. '?' can be used\nas a workaround, but it matches other characters too. This patch makes\na space following a backslash part of the pattern, not a pattern\nseparator.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n A similar patch was posted twice (during freeze time iirc). I think\n this is a good change, so I will keep reposting until someone turns\n it down.\n\n We could use wildmatch for parsing here, which would support patterns\n like \"Hello[ ]world\". But that's not going to happen until wildmatch\n graduates and somebody brings it up again.\n\n Documentation/gitattributes.txt | 6 +++---\n attr.c | 8 +++++++-\n t/t0003-attributes.sh | 5 +++++\n 3 files changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 2698f63..113b1f8 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -20,9 +20,9 @@ Each line in `gitattributes` file is of form:\n \n \tpattern\tattr1 attr2 ...\n \n-That is, a pattern followed by an attributes list,\n-separated by whitespaces. When the pattern matches the\n-path in question, the attributes listed on the line are given to\n+That is, a pattern followed by an attributes list, separated by\n+whitespaces that are not quoted by a backslash. When the pattern matches\n+the path in question, the attributes listed on the line are given to\n the path.\n \n Each attribute can be in one of these states for a given path:\ndiff --git a/attr.c b/attr.c\nindex 097ae87..776f34e 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -209,7 +209,13 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,\n \tif (!*cp || *cp == '#')\n \t\treturn NULL;\n \tname = cp;\n-\tnamelen = strcspn(name, blank);\n+\tnamelen = 0;\n+\twhile (name[namelen] != '\\0' && !strchr(blank, name[namelen])) {\n+\t\tif (name[namelen] == '\\\\' && name[namelen + 1] != '\\0')\n+\t\t\tnamelen += 2;\n+\t\telse\n+\t\t\tnamelen++;\n+\t}\n \tif (strlen(ATTRIBUTE_MACRO_PREFIX) < namelen &&\n \t !prefixcmp(name, ATTRIBUTE_MACRO_PREFIX)) {\n \t\tif (!macro_ok) {\ndiff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\nindex 807b8b8..6a5d8ab 100755\n--- a/t/t0003-attributes.sh\n+++ b/t/t0003-attributes.sh\n@@ -24,6 +24,7 @@ test_expect_success 'setup' '\n \t\techo \"offon -test test\"\n \t\techo \"no notest\"\n \t\techo \"A/e/F test=A/e/F\"\n+\t\techo \"A\\\\ b test=space\"\n \t) >.gitattributes &&\n \t(\n \t\techo \"g test=a/g\" &&\n@@ -196,6 +197,10 @@ test_expect_success 'root subdir attribute test' '\n \tattr_check subdir/a/i unspecified\n '\n \n+test_expect_success 'quoting in pattern' '\n+\tattr_check \"A b\" space\n+'\n+\n test_expect_success 'negative patterns' '\n \techo \"!f test=bar\" >.gitattributes &&\n \ttest_must_fail git check-attr test -- f\n-- \n1.8.0.rc2.23.g1fb49df\n"},{"id":"210176","messageId":"7v7gly5az0.fsf@alter.siamese.dyndns.org","threadId":"32524","inReplyTo":"1357310809-4771-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] attr: allow pattern escape using backslashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-24T09:15:31Z","receivedAt":"2013-02-24T09:15:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> Patterns in .gitattributes are separated by whitespaces, which makes\n> it impossible to specify exact spaces in the pattern. '?' can be used\n> as a workaround, but it matches other characters too. This patch makes\n> a space following a backslash part of the pattern, not a pattern\n> separator.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n>\n> A similar patch was posted twice (during freeze time iirc). I think\n> this is a good change, so I will keep reposting until someone turns\n> it down.\n>\n> We could use wildmatch for parsing here, which would support patterns\n> like \"Hello[ ]world\". But that's not going to happen until wildmatch\n> graduates and somebody brings it up again.\n\nDo people have any comment on this? Do we do something similar in\nthe .gitignore files already?\n\nSpeaking of .gitignore, I recall that there was a hanging discussion\non allowing a pattern to name the directory that the .gitignore file\nappears in, which I do not think we currently support. With such a\nfeature, instead of listing \"/junk\" in the .gitignore file at the\ntop-level to say that everything inside the \"junk\" directory is\nignored by default, we could instead say \"\" at the beginning\nof the .gitignore file in the \"junk\" directory. I think \"* \"\nin the .gitattributes file in a directory causes \"git check-attr .\"\nin that directory to report the , and we may want to have some\nway to allow \"git check-ignore .\" to be affected by the .gitignore\nfile in the same directory in a similar way, but I do not have a\nvery strong opinion. Do people have any comment on this?\n\nIt is obviously too late for this cycle to do anything on the\n'master' front on this topic (or the .gitignore), but it would be\ngood to reach concensus early so that we know what we want to do in\nthe next cycle.\n\n> Documentation/gitattributes.txt | 6 +++---\n> attr.c | 8 +++++++-\n> t/t0003-attributes.sh | 5 +++++\n> 3 files changed, 15 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\n> index 2698f63..113b1f8 100644\n> --- a/Documentation/gitattributes.txt\n> +++ b/Documentation/gitattributes.txt\n> @@ -20,9 +20,9 @@ Each line in `gitattributes` file is of form:\n> \n> \tpattern\tattr1 attr2 ...\n> \n> -That is, a pattern followed by an attributes list,\n> -separated by whitespaces. When the pattern matches the\n> -path in question, the attributes listed on the line are given to\n> +That is, a pattern followed by an attributes list, separated by\n> +whitespaces that are not quoted by a backslash. When the pattern matches\n> +the path in question, the attributes listed on the line are given to\n> the path.\n> \n> Each attribute can be in one of these states for a given path:\n> diff --git a/attr.c b/attr.c\n> index 097ae87..776f34e 100644\n> --- a/attr.c\n> +++ b/attr.c\n> @@ -209,7 +209,13 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,\n> \tif (!*cp || *cp == '#')\n> \t\treturn NULL;\n> \tname = cp;\n> -\tnamelen = strcspn(name, blank);\n> +\tnamelen = 0;\n> +\twhile (name[namelen] != '\\0' && !strchr(blank, name[namelen])) {\n> +\t\tif (name[namelen] == '\\\\' && name[namelen + 1] != '\\0')\n> +\t\t\tnamelen += 2;\n> +\t\telse\n> +\t\t\tnamelen++;\n> +\t}\n> \tif (strlen(ATTRIBUTE_MACRO_PREFIX) < namelen &&\n> \t !prefixcmp(name, ATTRIBUTE_MACRO_PREFIX)) {\n> \t\tif (!macro_ok) {\n> diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\n> index 807b8b8..6a5d8ab 100755\n> --- a/t/t0003-attributes.sh\n> +++ b/t/t0003-attributes.sh\n> @@ -24,6 +24,7 @@ test_expect_success 'setup' '\n> \t\techo \"offon -test test\"\n> \t\techo \"no notest\"\n> \t\techo \"A/e/F test=A/e/F\"\n> +\t\techo \"A\\\\ b test=space\"\n> \t) >.gitattributes &&\n> \t(\n> \t\techo \"g test=a/g\" &&\n> @@ -196,6 +197,10 @@ test_expect_success 'root subdir attribute test' '\n> \tattr_check subdir/a/i unspecified\n> '\n> \n> +test_expect_success 'quoting in pattern' '\n> +\tattr_check \"A b\" space\n> +'\n> +\n> test_expect_success 'negative patterns' '\n> \techo \"!f test=bar\" >.gitattributes &&\n> \ttest_must_fail git check-attr test -- f\n"},{"id":"210389","messageId":"CACsJy8Ag8rJ4P6w6ZCB0vBp6Hzqr_G5vWS_Jt=RNQT8+hF_w_A@mail.gmail.com","threadId":"32524","inReplyTo":"7v7gly5az0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] attr: allow pattern escape using backslashes","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-02-27T12:23:57Z","receivedAt":"2013-02-27T12:23:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Feb 24, 2013 at 4:15 PM, Junio C Hamano wrote:\n> Speaking of .gitignore, I recall that there was a hanging discussion\n> on allowing a pattern to name the directory that the .gitignore file\n> appears in, which I do not think we currently support. With such a\n> feature, instead of listing \"/junk\" in the .gitignore file at the\n> top-level to say that everything inside the \"junk\" directory is\n> ignored by default, we could instead say \"\" at the beginning\n> of the .gitignore file in the \"junk\" directory.\n\nShouldn't \"/\" alone in junk/.gitignore express that? It does not work,\nbut I think it's a natural interpretation of the syntax.\n\n> I think \"* \"\n> in the .gitattributes file in a directory causes \"git check-attr .\"\n> in that directory to report the , and we may want to have some\n> way to allow \"git check-ignore .\" to be affected by the .gitignore\n> file in the same directory in a similar way, but I do not have a\n> very strong opinion. Do people have any comment on this?\n-- \nDuy\n"},{"id":"210400","messageId":"7vlia9kbkz.fsf@alter.siamese.dyndns.org","threadId":"32524","inReplyTo":"CACsJy8Ag8rJ4P6w6ZCB0vBp6Hzqr_G5vWS_Jt=RNQT8+hF_w_A@mail.gmail.com","subject":"Re: [PATCH] attr: allow pattern escape using backslashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-27T15:42:20Z","receivedAt":"2013-02-27T15:42:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Sun, Feb 24, 2013 at 4:15 PM, Junio C Hamano wrote:\n>> Speaking of .gitignore, I recall that there was a hanging discussion\n>> on allowing a pattern to name the directory that the .gitignore file\n>> appears in, which I do not think we currently support. With such a\n>> feature, instead of listing \"/junk\" in the .gitignore file at the\n>> top-level to say that everything inside the \"junk\" directory is\n>> ignored by default, we could instead say \"\" at the beginning\n>> of the .gitignore file in the \"junk\" directory.\n>\n> Shouldn't \"/\" alone in junk/.gitignore express that? It does not work,\n> but I think it's a natural interpretation of the syntax.\n\nYup, there is nothing that you can plug to the \"\" in the above\nto make it mean \"junk\" directory is ignored. A trailing \"/\" is\nremoved after noting the fact that the entry is about a directory\nand leaves an empty string, and it would be OK to define that an\nempty string matches the directory the gitignore file appears in.\n"}]}