{"thread":{"id":"38875","subject":"[PATCH/RFC][GSoC] diff-no-index: transform \"$directory $file\" args to \"$directory/$file $file\"","startedAt":"2015-03-21T12:50:32Z","lastAt":"2015-03-22T02:39:02Z","messageCount":4,"participants":["Yurii Shevtsov","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"258240","messageId":"CAHLaBN+x3SVL9+jDzeSEMapVd2BVrwQuVx_7ENspjbUPrium_A@mail.gmail.com","threadId":"38875","inReplyTo":null,"subject":"[PATCH/RFC][GSoC] diff-no-index: transform \"$directory $file\" args to \"$directory/$file $file\"","fromName":"Yurii Shevtsov","fromEmail":"ungetch@gmail.com","sentAt":"2015-03-21T12:50:32Z","receivedAt":"2015-03-21T12:50:32Z","isPatch":true,"sender":{"key":"ungetch@gmail.com","avatar":null},"body":"git diff --no-index refuses to compare if args are directory and file,\ninstead of usual diff.\n\nNow git diff --no-index compares args' target names and, if they are\nequal, refuses to compare. If args differ, it transforms args and\ndiffs files, as usual diff does.\nThis is the least expensive way in terms of amount of new code and\nalso doesn't affect usual behaviour.\n\nChanges are done in queue_diff(). Adding args transformation logic to\ntop-level is irrational,\nbecause requires writing additional code to define args' types, which\nis already done in\nqueue_diff(). The error message about file\\directory conflict left as\nis until further guidence.\n\nSigned-off-by: Yurii Shevtsov \n---\n diff-no-index.c | 35 +++++++++++++++++++++++++++++++++--\n 1 files changed, 33 insertions(+), 2 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 265709b..9a3439a 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -97,8 +97,39 @@ static int queue_diff(struct diff_options *o,\n if (get_mode(name1, &mode1) || get_mode(name2, &mode2))\n return -1;\n\n- if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))\n- return error(\"file/directory conflict: %s, %s\", name1, name2);\n+ if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {\n+ struct strbuf path;\n+ const char *dir, *file;\n+ char *filename, *dirname = 0;\n+ int i, ret = 0;\n+\n+ dir = S_ISDIR(mode1) ? name1 : name2;\n+ file = (dir == name1) ? name2 : name1;\n+ strbuf_init(&path, strlen(name1) + strlen(name2) + 1);\n+ strbuf_addstr(&path, dir);\n+ filename = strrchr(file, '/');\n+ if (path.len && path.buf[path.len - 1] != '/')\n+ strbuf_addch(&path, '/');\n+ for (i = path.len - 2; i >= 0; i--)\n+ if (path.buf[i] == '/') {\n+ dirname = &path.buf[i];\n+ break;\n+ }\n+ if (dirname == 0)\n+ dirname = path.buf;\n+\n+ if (!strncmp(dirname, filename, strlen(filename)))\n+ return error(\"file/directory conflict: %s, %s\", name1, name2);\n+\n+ strbuf_addstr(&path, filename ? (filename + 1) : file);\n+ if (file == name1)\n+ ret = queue_diff(o, file, path.buf);\n+ else\n+ ret = queue_diff(o, path.buf, file);\n+ strbuf_release(&path);\n+\n+ return ret;\n+ }\n\n if (S_ISDIR(mode1) || S_ISDIR(mode2)) {\n struct strbuf buffer1 = STRBUF_INIT;\n--\n"},{"id":"258252","messageId":"xmqqsicyciz2.fsf@gitster.dls.corp.google.com","threadId":"38875","inReplyTo":"CAHLaBN+x3SVL9+jDzeSEMapVd2BVrwQuVx_7ENspjbUPrium_A@mail.gmail.com","subject":"Re: [PATCH/RFC][GSoC] diff-no-index: transform \"$directory $file\" args to \"$directory/$file $file\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-21T18:18:25Z","receivedAt":"2015-03-21T18:18:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yurii Shevtsov writes:\n\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 265709b..9a3439a 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -97,8 +97,39 @@ static int queue_diff(struct diff_options *o,\n> if (get_mode(name1, &mode1) || get_mode(name2, &mode2))\n> return -1;\n>\n> - if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))\n> - return error(\"file/directory conflict: %s, %s\", name1, name2);\n> + if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {\n> + struct strbuf path;\n> + const char *dir, *file;\n> + char *filename, *dirname = 0;\n> + int i, ret = 0;\n\nIf you have two directories, a and b, under which there are two\nfiles a/sub/file and b/sub (i.e. 'sub' in a/ is a directory and b/\nis a file), and if you say \"git diff --no-index a b\", what happens?\n\n - the caller of this function gives a and b in name1 and name2;\n\n - we do not come in this codepath as both are directories;\n\n - we read from a/ and b/ and fill p1 and p2 with names of paths in\n the directories -- p1 and p2 will both have 'sub';\n\n - queue_diff() is recursively called to compare a/sub and b/sub;\n\n - now we have name1 = a/sub and name2 = b/sub;\n\n - we come in this codepath, and they are turned into comparison\n between a/sub/sub and b/sub.\n\nThe last step is simply crazy.\n\nHmmmm, is vger reinjecting an old message, or you sent an older and\nwrong version of a patch by mistake? We discussed why doing this in\nqueue_diff() is wrong in the thread that has $gmane/265543 in it,\nand I was expecting to see a logic like this in the caller.\n\nPuzzled...\n"},{"id":"258254","messageId":"CAHLaBN+q8T6XJ-aJNu1HsNdnKDrR_5PFeY20vmj8RUDTX8PNew@mail.gmail.com","threadId":"38875","inReplyTo":"xmqqsicyciz2.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH/RFC][GSoC] diff-no-index: transform \"$directory $file\" args to \"$directory/$file $file\"","fromName":"Yurii Shevtsov","fromEmail":"ungetch@gmail.com","sentAt":"2015-03-21T19:08:26Z","receivedAt":"2015-03-21T19:08:26Z","isPatch":true,"sender":{"key":"ungetch@gmail.com","avatar":null},"body":">> diff --git a/diff-no-index.c b/diff-no-index.c\n>> index 265709b..9a3439a 100644\n>> --- a/diff-no-index.c\n>> +++ b/diff-no-index.c\n>> @@ -97,8 +97,39 @@ static int queue_diff(struct diff_options *o,\n>> if (get_mode(name1, &mode1) || get_mode(name2, &mode2))\n>> return -1;\n>>\n>> - if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))\n>> - return error(\"file/directory conflict: %s, %s\", name1, name2);\n>> + if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {\n>> + struct strbuf path;\n>> + const char *dir, *file;\n>> + char *filename, *dirname = 0;\n>> + int i, ret = 0;\n>\n> If you have two directories, a and b, under which there are two\n> files a/sub/file and b/sub (i.e. 'sub' in a/ is a directory and b/\n> is a file), and if you say \"git diff --no-index a b\", what happens?\n>\n> - the caller of this function gives a and b in name1 and name2;\n>\n> - we do not come in this codepath as both are directories;\n>\n> - we read from a/ and b/ and fill p1 and p2 with names of paths in\n> the directories -- p1 and p2 will both have 'sub';\n>\n> - queue_diff() is recursively called to compare a/sub and b/sub;\n>\n> - now we have name1 = a/sub and name2 = b/sub;\n>\n> - we come in this codepath, and they are turned into comparison\n> between a/sub/sub and b/sub.\n>\n> The last step is simply crazy.\n\nIt won't try to compare a/sub/sub and b/sub, since git diff with this\npatch falls before with error message, as I planned (if it did compare\nI would get 'Couldn't access a/sub/sub'). I tested my code! Original\ngit diff also stops comparing when hits folder and file with same\nnames. So my patch allows to pass 'D F' args, and doesn't affect other\nlogic.\n\n> Hmmmm, is vger reinjecting an old message, or you sent an older and\n> wrong version of a patch by mistake? We discussed why doing this in\n> queue_diff() is wrong in the thread that has $gmane/265543 in it,\n> and I was expecting to see a logic like this in the caller.\n>\n> Puzzled...\n\nYes, we discussed it, but, as I mentioned in commit message, I think\nmy solution much better fits in queue_diff()\n"},{"id":"258262","messageId":"CAPig+cShOv0TQ2x_22j8rXyp+kNbBVwAs8E1U==9qkijvffxoA@mail.gmail.com","threadId":"38875","inReplyTo":"CAHLaBN+x3SVL9+jDzeSEMapVd2BVrwQuVx_7ENspjbUPrium_A@mail.gmail.com","subject":"Re: [PATCH/RFC][GSoC] diff-no-index: transform \"$directory $file\" args to \"$directory/$file $file\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-22T02:39:02Z","receivedAt":"2015-03-22T02:39:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Mar 21, 2015 at 8:50 AM, Yurii Shevtsov wrote:\n> Signed-off-by: Yurii Shevtsov \n> ---\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 265709b..9a3439a 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -97,8 +97,39 @@ static int queue_diff(struct diff_options *o,\n> if (get_mode(name1, &mode1) || get_mode(name2, &mode2))\n> return -1;\n\nSomehow, you lost all the tabs in the patch, and everything is instead\nindented with spaces (including context lines).\n\n> - if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))\n> - return error(\"file/directory conflict: %s, %s\", name1, name2);\n> + if (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {\n> + struct strbuf path;\n> + const char *dir, *file;\n> + char *filename, *dirname = 0;\n> + int i, ret = 0;\n> +\n> + dir = S_ISDIR(mode1) ? name1 : name2;\n> + file = (dir == name1) ? name2 : name1;\n> + strbuf_init(&path, strlen(name1) + strlen(name2) + 1);\n> + strbuf_addstr(&path, dir);\n> + filename = strrchr(file, '/');\n> + if (path.len && path.buf[path.len - 1] != '/')\n> + strbuf_addch(&path, '/');\n> + for (i = path.len - 2; i >= 0; i--)\n> + if (path.buf[i] == '/') {\n> + dirname = &path.buf[i];\n> + break;\n> + }\n> + if (dirname == 0)\n> + dirname = path.buf;\n> +\n> + if (!strncmp(dirname, filename, strlen(filename)))\n> + return error(\"file/directory conflict: %s, %s\", name1, name2);\n\nLeaking 'path' strbuf.\n\n> +\n> + strbuf_addstr(&path, filename ? (filename + 1) : file);\n> + if (file == name1)\n> + ret = queue_diff(o, file, path.buf);\n> + else\n> + ret = queue_diff(o, path.buf, file);\n> + strbuf_release(&path);\n> +\n> + return ret;\n> + }\n>\n> if (S_ISDIR(mode1) || S_ISDIR(mode2)) {\n> struct strbuf buffer1 = STRBUF_INIT;\n> --\n"}]}