{"thread":{"id":"35362","subject":"[PATCH] diff: restrict pathspec limitations to diff b/f case only","startedAt":"2013-11-20T01:26:41Z","lastAt":"2013-11-22T05:40:24Z","messageCount":8,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"230824","messageId":"1384910801-11125-1-git-send-email-pclouds@gmail.com","threadId":"35362","inReplyTo":null,"subject":"[PATCH] diff: restrict pathspec limitations to diff b/f case only","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-20T01:26:41Z","receivedAt":"2013-11-20T01:26:41Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"builtin_diff_b_f() needs a path, not pathspec. Other modes in diff\ncan deal with pathspec just fine. But because of the current\nGUARD_PATHSPEC() location, other modes also reject :(glob) and\n:(icase).\n\nMove GUARD_PATHSPEC(), and the \"path\" assignment statement, which is\nthe reason of this GUARD_PATHSPEC(), inside builtin_diff_b_f().\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/diff.c | 17 +++++++----------\n 1 file changed, 7 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex adb93a9..fe0cc7f 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -64,15 +64,18 @@ static void stuff_change(struct diff_options *opt,\n \n static int builtin_diff_b_f(struct rev_info *revs,\n \t\t\t int argc, const char **argv,\n-\t\t\t struct blobinfo *blob,\n-\t\t\t const char *path)\n+\t\t\t struct blobinfo *blob)\n {\n \t/* Blob vs file in the working tree*/\n \tstruct stat st;\n+\tconst char *path;\n \n \tif (argc > 1)\n \t\tusage(builtin_diff_usage);\n \n+\tGUARD_PATHSPEC(&revs->prune_data, PATHSPEC_FROMTOP | PATHSPEC_LITERAL);\n+\tpath = revs->prune_data.items[0].match;\n+\n \tif (lstat(path, &st))\n \t\tdie_errno(_(\"failed to stat '%s'\"), path);\n \tif (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))\n@@ -255,7 +258,6 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \tstruct rev_info rev;\n \tstruct object_array ent = OBJECT_ARRAY_INIT;\n \tint blobs = 0, paths = 0;\n-\tconst char *path = NULL;\n \tstruct blobinfo blob[2];\n \tint nongit;\n \tint result = 0;\n@@ -366,13 +368,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\t\tdie(_(\"unhandled object '%s' given.\"), name);\n \t\t}\n \t}\n-\tif (rev.prune_data.nr) {\n-\t\t/* builtin_diff_b_f() */\n-\t\tGUARD_PATHSPEC(&rev.prune_data, PATHSPEC_FROMTOP | PATHSPEC_LITERAL);\n-\t\tif (!path)\n-\t\t\tpath = rev.prune_data.items[0].match;\n+\tif (rev.prune_data.nr)\n \t\tpaths += rev.prune_data.nr;\n-\t}\n \n \t/*\n \t * Now, do the arguments look reasonable?\n@@ -385,7 +382,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\tcase 1:\n \t\t\tif (paths != 1)\n \t\t\t\tusage(builtin_diff_usage);\n-\t\t\tresult = builtin_diff_b_f(&rev, argc, argv, blob, path);\n+\t\t\tresult = builtin_diff_b_f(&rev, argc, argv, blob);\n \t\t\tbreak;\n \t\tcase 2:\n \t\t\tif (paths)\n-- \n1.8.2.82.gc24b958\n"},{"id":"230868","messageId":"xmqqvbzm66dt.fsf@gitster.dls.corp.google.com","threadId":"35362","inReplyTo":"1384910801-11125-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] diff: restrict pathspec limitations to diff b/f case only","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-20T22:43:58Z","receivedAt":"2013-11-20T22:43:58Z","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> builtin_diff_b_f() needs a path, not pathspec. Other modes in diff\n> can deal with pathspec just fine. But because of the current\n> GUARD_PATHSPEC() location, other modes also reject :(glob) and\n> :(icase).\n>\n> Move GUARD_PATHSPEC(), and the \"path\" assignment statement, which is\n> the reason of this GUARD_PATHSPEC(), inside builtin_diff_b_f().\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n\nMakes sense.\n\nLet me squash this in.\n\n\ndiff --git a/t/t6131-pathspec-icase.sh b/t/t6131-pathspec-icase.sh\nindex 8d4a7fc..a7c7ff5 100755\n--- a/t/t6131-pathspec-icase.sh\n+++ b/t/t6131-pathspec-icase.sh\n@@ -100,4 +100,10 @@ test_expect_success 'match_pathspec_depth matches :(icase)bar with empty prefix'\n \ttest_cmp expect actual\n '\n \n+test_expect_success '\"git diff\" can take magic :(icase) pathspec' '\n+\techo FOO/BAR >expect &&\n+\tgit diff --name-only HEAD^ HEAD -- \":(icase)foo/bar\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n\nThanks.\n"},{"id":"230874","messageId":"1385016260-9718-1-git-send-email-pclouds@gmail.com","threadId":"35362","inReplyTo":"1384910801-11125-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 1/2] glossary-content.txt: remove an obsolete paragrah","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-21T06:44:19Z","receivedAt":"2013-11-21T06:44:19Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"With the introduction of :(literal), :(glob) and :(icase), :(top) is\nno longer the only recognized magic signature.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n .. on top of nd/magic-pathspec..\n\n Documentation/glossary-content.txt | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\nindex e470661..e22b524 100644\n--- a/Documentation/glossary-content.txt\n+++ b/Documentation/glossary-content.txt\n@@ -379,10 +379,6 @@ full pathname may have special meaning:\n Glob magic is incompatible with literal magic.\n --\n +\n-Currently only the slash `/` is recognized as the \"magic signature\",\n-but it is envisioned that we will support more types of magic in later\n-versions of Git.\n-+\n A pathspec with only a colon means \"there is no pathspec\". This form\n should not be combined with other pathspec.\n \n-- \n1.8.2.82.gc24b958\n"},{"id":"230875","messageId":"1385016260-9718-2-git-send-email-pclouds@gmail.com","threadId":"35362","inReplyTo":"1385016260-9718-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 2/2] glossary-content.txt: fix documentation of \"**\" patterns","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-21T06:44:20Z","receivedAt":"2013-11-21T06:44:20Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\"**\" means bold in ASCIIDOC, so we need to escape it. This is similar\nto 8447dc8 (gitignore.txt: fix documentation of \"**\" patterns -\n2013-11-07)\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n There is another instance of '**' in RelNotes/1.8.5.txt. Junio you\n may want to fix it too.\n\n Documentation/glossary-content.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\nindex e22b524..1b56ba4 100644\n--- a/Documentation/glossary-content.txt\n+++ b/Documentation/glossary-content.txt\n@@ -362,12 +362,12 @@ full pathname may have special meaning:\n \n - A leading \"`**`\" followed by a slash means match in all\n directories. For example, \"`**/foo`\" matches file or directory\n- \"`foo`\" anywhere, the same as pattern \"`foo`\". \"**/foo/bar\"\n+ \"`foo`\" anywhere, the same as pattern \"`foo`\". \"`**/foo/bar`\"\n matches file or directory \"`bar`\" anywhere that is directly\n under directory \"`foo`\".\n \n- - A trailing \"/**\" matches everything inside. For example,\n- \"abc/**\" matches all files inside directory \"abc\", relative\n+ - A trailing \"`/**`\" matches everything inside. For example,\n+ \"`abc/**`\" matches all files inside directory \"abc\", relative\n to the location of the `.gitignore` file, with infinite depth.\n \n - A slash followed by two consecutive asterisks then a slash\n-- \n1.8.2.82.gc24b958\n"},{"id":"230894","messageId":"xmqqtxf54o83.fsf@gitster.dls.corp.google.com","threadId":"35362","inReplyTo":"1385016260-9718-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 1/2] glossary-content.txt: remove an obsolete paragrah","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-21T18:13:48Z","receivedAt":"2013-11-21T18:13:48Z","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> With the introduction of :(literal), :(glob) and :(icase), :(top) is\n> no longer the only recognized magic signature.\n\nYou need to re-read the message you took hints that led to this\npatch from (or, rather, the existing document around the area you\nare patching, which led to the wording in that message from me).\n\nThe above is a list of magic words, but until you add the magic\nsignature \":!not_in_this_directory\" for the \":(exclude)\" magic word,\n\":/\" still is the only magic signature.\n\nAnother moderately related tangent is this phrase in the existing\ndocument:\n\n In the short form, the leading colon `:` is followed by zero or\n more \"magic signature\" letters (which optionally is terminated\n by another colon `:`), and the remainder is the pattern to match\n against the path. The optional colon that terminates the \"magic\n signature\" can be omitted if the pattern begins with a character\n that cannot be a \"magic signature\" and is not a colon.\n\nWhile it is technically correct, the phrase \"a character that cannot\nbe\" is somewhat misleading, and I think it needs to be clarified by\nrephrasing.\n\nAs we can see in a later paragaph:\n\n The \"magic signature\" consists of an ASCII symbol that is not\n alphanumeric. Currently only the slash `/` is recognized as a\n \"magic signature\"...\n\nthe correct way to read that \"a character that cannot be a magic\nsignature\" is \"a character that is not 'an ASCII symbol that is not\nalphanumeric'\", which means you can do:\n\n\t:!/foo\t- with magic signatures ! and /, pattern begins at 'f'\n\n\t:/#abc - with magic signatures # and /, pattern begins at 'a',\n\t even if in a particular version of Git, '#' magic\n\t\t signature may be invalid and produce an error\n\n\t:/:#abc - with magic signature /, pattern is \"#abc\".\n\nbut because the definition of \"magic signature\" syntax comes later\nthan \"cannot be\", it is prone to be misinterpreted as \"anything that\nis not currently defined as a magic signature (namely, '/')\".\n\n\n\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> .. on top of nd/magic-pathspec..\n>\n> Documentation/glossary-content.txt | 4 ----\n> 1 file changed, 4 deletions(-)\n>\n> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt\n> index e470661..e22b524 100644\n> --- a/Documentation/glossary-content.txt\n> +++ b/Documentation/glossary-content.txt\n> @@ -379,10 +379,6 @@ full pathname may have special meaning:\n> Glob magic is incompatible with literal magic.\n> --\n> +\n> -Currently only the slash `/` is recognized as the \"magic signature\",\n> -but it is envisioned that we will support more types of magic in later\n> -versions of Git.\n> -+\n> A pathspec with only a colon means \"there is no pathspec\". This form\n> should not be combined with other pathspec.\n"},{"id":"230927","messageId":"xmqqy54h1f9q.fsf@gitster.dls.corp.google.com","threadId":"35362","inReplyTo":"xmqqtxf54o83.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] glossary-content.txt: remove an obsolete paragrah","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-21T23:55:29Z","receivedAt":"2013-11-21T23:55:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\nAn addendum.\n\n> As we can see in a later paragaph:\n>\n> The \"magic signature\" consists of an ASCII symbol that is not\n> alphanumeric. Currently only the slash `/` is recognized as a\n> \"magic signature\"...\n>\n> the correct way to read that \"a character that cannot be a magic\n> signature\" is \"a character that is not 'an ASCII symbol that is not\n> alphanumeric'\", which means you can do:\n>\n> \t:!/foo\t- with magic signatures ! and /, pattern begins at 'f'\n>\n> \t:/#abc - with magic signatures # and /, pattern begins at 'a',\n> \t even if in a particular version of Git, '#' magic\n> \t\t signature may be invalid and produce an error\n>\n> \t:/:#abc - with magic signature /, pattern is \"#abc\".\n>\n> but because the definition of \"magic signature\" syntax comes later\n> than \"cannot be\", it is prone to be misinterpreted as \"anything that\n> is not currently defined as a magic signature (namely, '/')\".\n\n... and that misinterpretation may give a false impression that\n\n\t\":/#abc\"\n\nis with the magic signature '/' (i.e. match from top) for a pattern\n\"#abc\".\n\nDoing so would mean that a version of Git that does not support a\nmagic signature '#' will allow people and scripts to use \":/#abc\"\nwith such a meaning, and will prevent us from using '#' as a new\nmagic signature with some useful meaning in the future. Rather, we\nneed to force them to always spell it as \":/:#abc\" even in the\nversion of Git before the magic signature '#'.\n\nAnd the phrasing 'if the pattern begins with a character that cannot\nbe a \"magic signature\" and is not a colon' needs to be clarified to\navoid that misinterpretation for that reason.\n"},{"id":"230930","messageId":"CACsJy8C38PUCYMSdEJ1-S5HSNdJO7S24mhkEryFrSGfwKuaU0A@mail.gmail.com","threadId":"35362","inReplyTo":"xmqqy54h1f9q.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/2] glossary-content.txt: remove an obsolete paragrah","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-11-22T01:36:46Z","receivedAt":"2013-11-22T01:36:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Nov 22, 2013 at 6:55 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n> An addendum.\n>\n>> As we can see in a later paragaph:\n>>\n>> The \"magic signature\" consists of an ASCII symbol that is not\n>> alphanumeric. Currently only the slash `/` is recognized as a\n>> \"magic signature\"...\n>>\n>> the correct way to read that \"a character that cannot be a magic\n>> signature\" is \"a character that is not 'an ASCII symbol that is not\n>> alphanumeric'\", which means you can do:\n>>\n>> :!/foo - with magic signatures ! and /, pattern begins at 'f'\n>>\n>> :/#abc - with magic signatures # and /, pattern begins at 'a',\n>> even if in a particular version of Git, '#' magic\n>> signature may be invalid and produce an error\n>>\n>> :/:#abc - with magic signature /, pattern is \"#abc\".\n>>\n>> but because the definition of \"magic signature\" syntax comes later\n>> than \"cannot be\", it is prone to be misinterpreted as \"anything that\n>> is not currently defined as a magic signature (namely, '/')\".\n>\n> ... and that misinterpretation may give a false impression that\n>\n> \":/#abc\"\n>\n> is with the magic signature '/' (i.e. match from top) for a pattern\n> \"#abc\".\n>\n> Doing so would mean that a version of Git that does not support a\n> magic signature '#' will allow people and scripts to use \":/#abc\"\n> with such a meaning, and will prevent us from using '#' as a new\n> magic signature with some useful meaning in the future. Rather, we\n> need to force them to always spell it as \":/:#abc\" even in the\n> version of Git before the magic signature '#'.\n\nCurrent version does force that.\n\n$ git log -- ':/#aa'\nfatal: Unimplemented pathspec magic '#' in ':/#aa'\n\nNot sure if there are tests for it though. I'll add an advice to do \":/:#aa\".\n\n> And the phrasing 'if the pattern begins with a character that cannot\n> be a \"magic signature\" and is not a colon' needs to be clarified to\n> avoid that misinterpretation for that reason.\n-- \nDuy\n"},{"id":"230932","messageId":"xmqqpppt0zav.fsf@gitster.dls.corp.google.com","threadId":"35362","inReplyTo":"CACsJy8C38PUCYMSdEJ1-S5HSNdJO7S24mhkEryFrSGfwKuaU0A@mail.gmail.com","subject":"Re: [PATCH 1/2] glossary-content.txt: remove an obsolete paragrah","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-22T05:40:24Z","receivedAt":"2013-11-22T05:40:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> Current version does force that.\n>\n> $ git log -- ':/#aa'\n> fatal: Unimplemented pathspec magic '#' in ':/#aa'\n>\n> Not sure if there are tests for it though. I'll add an advice to do \":/:#aa\".\n\nIt is good that we already do the right thing, but actually I was\nnot questioning the implementation. I was talking about the fact\nthat the documentation is open to misinterpretation that contradicts\nthe correct behaviour, as if it were buggy.\n"}]}