{"thread":{"id":"31739","subject":"[PATCH/RFC] attr: allow pattern escape using backslash","startedAt":"2012-10-06T07:22:05Z","lastAt":"2012-10-06T08:02:53Z","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":"200654","messageId":"1349508125-24775-1-git-send-email-pclouds@gmail.com","threadId":"31739","inReplyTo":null,"subject":"[PATCH/RFC] attr: allow pattern escape using backslash","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-10-06T07:22:05Z","receivedAt":"2012-10-06T07:22:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":".gitattributes pattern syntax is supposed to be the same as .gitignore\n(except a few things that do not make sense in attr context, but\nthat's a different issue). .gitignore uses fnmatch() as the matching\nmachinery and \"\\\" is accepted as an escape code. In theory the pattern\n'foo\\ bar' should match path 'foo bar' in .gitignore. Granted, no one\nwould write 'foo\\ bar' in .gitignore when 'foo bar' should\nsuffice.\n\nRegardless, 'foo\\ bar attr' does not (but should) attach \"attr\" to\npath \"foo bar\" because pattern/attr parse code does not understand\nbackslash escape. It parses the line as path 'foo\\' and attributes\n'bar' and 'attr'. This patch teaches attr code to recognize the\nbackslash in patterns (not macro names) and pass 'foo\\ bar' down to\nfnmatch().\n\nThis changes the attr behavior. \"foo\\ attr\", if exists in the field,\nwould match nothing because path \"foo\\\" is invalid in UNIX and is a\ndirectory in Windows, which we do not accept attaching attributes\nto. With this patch, that line becomes invalid and is rejected. So\nit's not really bad (i.e. no silent changes in behavior).\n\nOther subtle behavioral changes may happen. Still, I think we should\ndo this as it seems like a correct thing to do.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n We discussed the \"spaces in path names in attr\" before and I remember\n using quotes or double quotes, even substituting spaces with\n dots, were raised. I don't remember if backslashes were mentioned.\n\n attr.c                | 12 +++++++++++-\n t/t0003-attributes.sh |  5 +++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/attr.c b/attr.c\nindex 887a9ae..173d51d 100644\n--- a/attr.c\n+++ b/attr.c\n@@ -221,8 +221,18 @@ static struct match_attr *parse_attr_line(const char *line, const char *src,\n \t\t\treturn NULL;\n \t\t}\n \t}\n-\telse\n+\telse {\n \t\tis_macro = 0;\n+\t\tnamelen = 0;\n+\t\twhile (name[namelen] != '\\0' &&\n+\t\t       !strchr(blank, name[namelen])) {\n+\t\t\tif (name[namelen] == '\\\\' &&\n+\t\t\t    name[namelen + 1] != '\\0')\n+\t\t\t\tnamelen += 2;\n+\t\t\telse\n+\t\t\t\tnamelen++;\n+\t\t}\n+\t}\n \n \tstates = name + namelen;\n \tstates += strspn(states, blank);\ndiff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh\nindex febc45c..16b419e 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 'setup bare' '\n \tgit clone --bare . bare.git &&\n \tcd bare.git\n-- \n1.7.12.1.406.g6ab07c4\n"},{"id":"200655","messageId":"7vzk40qbm1.fsf@alter.siamese.dyndns.org","threadId":"31739","inReplyTo":"1349508125-24775-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH/RFC] attr: allow pattern escape using backslash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-06T07:54:30Z","receivedAt":"2012-10-06T07:54:30Z","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> .gitattributes pattern syntax is supposed to be the same as .gitignore\n> (except a few things that do not make sense in attr context, but\n> that's a different issue). .gitignore uses fnmatch() as the matching\n> machinery and \"\\\" is accepted as an escape code. In theory the pattern\n> 'foo\\ bar' should match path 'foo bar' in .gitignore. Granted, no one\n> would write 'foo\\ bar' in .gitignore when 'foo bar' should\n> suffice.\n>\n> Regardless, 'foo\\ bar attr' does not (but should) attach \"attr\" to\n> path \"foo bar\" because pattern/attr parse code does not understand\n> backslash escape. It parses the line as path 'foo\\' and attributes\n> 'bar' and 'attr'. This patch teaches attr code to recognize the\n> backslash in patterns (not macro names) and pass 'foo\\ bar' down to\n> fnmatch().\n>\n> This changes the attr behavior. \"foo\\ attr\", if exists in the field,\n> would match nothing because path \"foo\\\" is invalid in UNIX and is a\n> directory in Windows, which we do not accept attaching attributes\n> to. With this patch, that line becomes invalid and is rejected. So\n> it's not really bad (i.e. no silent changes in behavior).\n>\n> Other subtle behavioral changes may happen. Still, I think we should\n> do this as it seems like a correct thing to do.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>  We discussed the \"spaces in path names in attr\" before and I remember\n>  using quotes or double quotes, even substituting spaces with\n>  dots, were raised. I don't remember if backslashes were mentioned.\n\nMy knee-jerk reaction, without thinking things through, is that this\nmakes sense and doing the same for .gitignore would, too (even\nthough the one-item-per-line nature of .gitignore makes \"\\ \" and \" \"\npractically equivalent).\n\nShouldn't we do the same for quoting fnmatch(3) metacharacters?  To\nmatch a path component 'a' followed by an asterisk followed by 'b',\nyou could then write 'a\\*b'.  Same for quoting the backslash itself.\n"},{"id":"200656","messageId":"CACsJy8BtfeJF=tov7nPMSjXuKjisJfNfHQ_eLjwYC=GTJP12rQ@mail.gmail.com","threadId":"31739","inReplyTo":"7vzk40qbm1.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] attr: allow pattern escape using backslash","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-10-06T08:02:53Z","receivedAt":"2012-10-06T08:02:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Oct 6, 2012 at 2:54 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Shouldn't we do the same for quoting fnmatch(3) metacharacters?  To\n> match a path component 'a' followed by an asterisk followed by 'b',\n> you could then write 'a\\*b'.  Same for quoting the backslash itself.\n\nI think my patch does that too. The thing it does not do is optimize\nthis case so that we can do strcmp() instead of fnmatch() if the\nentire pattern does not contain any metacharacters but backslashes. I\ndon't think it'll become popular enough to deserve an optimization.\n-- \nDuy\n"}]}