{"thread":{"id":"6960","subject":"[PATCH 4/8] diff: support reading a file from stdin via \"-\"","startedAt":"2007-02-25T22:36:10Z","lastAt":"2007-02-26T00:44:02Z","messageCount":6,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"35466","messageId":"Pine.LNX.4.63.0702252335410.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6960","inReplyTo":null,"subject":"[PATCH 4/8] diff: support reading a file from stdin via \"-\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-25T22:36:10Z","receivedAt":"2007-02-25T22:36:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis allows you to say\n\n\techo Hello World | git diff x -\n\nto compare the contents of file \"x\" with the line \"Hello World\".\nThis automatically switches to --no-index mode.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tSince the revision machinery checks for the presence of files,\n\tdiff_populate_filespec() will only change behaviour when there\n\tis a file \"-\"... I have yet to think of an elegant fix for that.\n\n diff-lib.c |   16 +++++++++++-----\n diff.c     |   20 ++++++++++++++++++++\n 2 files changed, 31 insertions(+), 5 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 6678e22..089c94c 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -37,14 +37,20 @@ static int queue_diff(struct diff_options *o,\n \tint mode1 = 0, mode2 = 0;\n \n \tif (name1) {\n-\t\tif (stat(name1, &st))\n+\t\tif (!strcmp(name1, \"-\"))\n+\t\t\tmode1 = 0644;\n+\t\telse if (stat(name1, &st))\n \t\t\treturn error(\"Could not access '%s'\", name1);\n-\t\tmode1 = st.st_mode;\n+\t\telse\n+\t\t\tmode1 = st.st_mode;\n \t}\n \tif (name2) {\n-\t\tif (stat(name2, &st))\n+\t\tif (!strcmp(name2, \"-\"))\n+\t\t\tmode2 = 0644;\n+\t\telse if (stat(name2, &st))\n \t\t\treturn error(\"Could not access '%s'\", name2);\n-\t\tmode2 = st.st_mode;\n+\t\telse\n+\t\t\tmode2 = st.st_mode;\n \t}\n \n \tif (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))\n@@ -224,7 +230,7 @@ int setup_diff_no_index(struct rev_info *revs,\n {\n \tint i;\n \tfor (i = 1; i < argc; i++)\n-\t\tif (argv[i][0] != '-')\n+\t\tif (argv[i][0] != '-' || argv[i][1] == '\\0')\n \t\t\tbreak;\n \t\telse if (!strcmp(argv[i], \"--\")) {\n \t\t\ti++;\ndiff --git a/diff.c b/diff.c\nindex 5651152..31129d1 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1389,6 +1389,22 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)\n \t\tchar *buf;\n \t\tunsigned long size;\n \n+\t\tif (!strcmp(s->path, \"-\")) {\n+#define INCREMENT 1024\n+\t\t\tint i = INCREMENT;\n+\t\t\tsize = 0;\n+\t\t\tbuf = NULL;\n+\t\t\twhile (i == INCREMENT) {\n+\t\t\t\tbuf = xrealloc(buf, size + INCREMENT);\n+\t\t\t\ti = xread(0, buf + size, INCREMENT);\n+\t\t\t\tsize += i;\n+\t\t\t}\n+\t\t\ts->should_munmap = 0;\n+\t\t\ts->data = buf;\n+\t\t\ts->size = size;\n+\t\t\ts->should_free = 1;\n+\t\t\treturn 0;\n+\t\t}\n \t\tif (lstat(s->path, &st) < 0) {\n \t\t\tif (errno == ENOENT) {\n \t\t\terr_empty:\n@@ -1688,6 +1704,10 @@ static void diff_fill_sha1_info(struct diff_filespec *one)\n \tif (DIFF_FILE_VALID(one)) {\n \t\tif (!one->sha1_valid) {\n \t\t\tstruct stat st;\n+\t\t\tif (!strcmp(one->path, \"-\")) {\n+\t\t\t\thashcpy(one->sha1, null_sha1);\n+\t\t\t\treturn;\n+\t\t\t}\n \t\t\tif (lstat(one->path, &st) < 0)\n \t\t\t\tdie(\"stat %s\", one->path);\n \t\t\tif (index_path(one->sha1, one->path, &st, 0))\n-- \n1.5.0.1.788.g8ca52\n"},{"id":"35472","messageId":"7vvehprgl7.fsf@assigned-by-dhcp.cox.net","threadId":"6960","inReplyTo":"Pine.LNX.4.63.0702252335410.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 4/8] diff: support reading a file from stdin via \"-\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-25T22:57:08Z","receivedAt":"2007-02-25T22:57:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> This allows you to say\n>\n> \techo Hello World | git diff x -\n>\n> to compare the contents of file \"x\" with the line \"Hello World\".\n> This automatically switches to --no-index mode.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>\n> \tSince the revision machinery checks for the presence of files,\n> \tdiff_populate_filespec() will only change behaviour when there\n> \tis a file \"-\"... I have yet to think of an elegant fix for that.\n\nAnother thing is that at some point diff_populate_filespec()\nneeds to have a way to discard what was cached if memory\npressure gets tight, and we would want to keep this data read\nfrom the standard input.\n\nOne solution would be to add a \"const char *stdin_data\" to diffopts\nand read the data from stdin when you parse the options, and\nhave populate_filespec point at that with s->data (setting\nshould_free and should_munmap both to 0).\n"},{"id":"35474","messageId":"Pine.LNX.4.63.0702260009530.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6960","inReplyTo":"7vvehprgl7.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/8] diff: support reading a file from stdin via \"-\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-25T23:11:38Z","receivedAt":"2007-02-25T23:11:38Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Feb 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > This allows you to say\n> >\n> > \techo Hello World | git diff x -\n> >\n> > to compare the contents of file \"x\" with the line \"Hello World\".\n> > This automatically switches to --no-index mode.\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >\n> > \tSince the revision machinery checks for the presence of files,\n> > \tdiff_populate_filespec() will only change behaviour when there\n> > \tis a file \"-\"... I have yet to think of an elegant fix for that.\n> \n> Another thing is that at some point diff_populate_filespec()\n> needs to have a way to discard what was cached if memory\n> pressure gets tight, and we would want to keep this data read\n> from the standard input.\n\nI see this purely for purposes of --no-index diff. And in that case, we \nonly compare one file pair. Either one of them is from stdin, or it is \nnot. Therefore, if memory gets tight, we cannot compare that file pair \nanyway, and have to error out.\n\nMaybe I missed something obvious?\n\nCiao,\nDscho\n"},{"id":"35476","messageId":"7virdprejh.fsf@assigned-by-dhcp.cox.net","threadId":"6960","inReplyTo":"Pine.LNX.4.63.0702260009530.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH 4/8] diff: support reading a file from stdin via \"-\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-25T23:41:22Z","receivedAt":"2007-02-25T23:41:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> I see this purely for purposes of --no-index diff. And in that case, we \n> only compare one file pair. Either one of them is from stdin, or it is \n> not. Therefore, if memory gets tight, we cannot compare that file pair \n> anyway, and have to error out.\n>\n> Maybe I missed something obvious?\n\nI think the responsibility to mark if a path whose name is \"-\"\nis a filesystem object or stdin lies within the option parser.\nIf the user says \"diff - a\" or diff a -\" then the user clearly\nmeans the stdin, while if the user says \"diff ./- a\" it is a\npath (and the later stages like populate_filespec() would see\ncanonicalized \"-\" as the filename).  The point is that the\ndisambiguation must be done by somebody, and I think the option\nparser, who calls get_pathspec(), should be the one that does\nit.\n\nSo we at least need a bit in diff_options that lets the option\nparser to tell the later stages \"if you see - in pathspec, the\nuser means stdin\".  And my suggestion was that if you are going\nto do that in the option parser anyway, you could read stdin in\nthe option parser and hang that data in diff_options, and the\npresence of that stdin data (pointer being non NULL) can be used\nas that signal.\n\nI think the callers of populate_filespec() may need to pass\naround diff_options as a parameter for the above to work, but\nhopefully that should not be a rocket surgery.\n"},{"id":"35478","messageId":"7vejodrdi4.fsf@assigned-by-dhcp.cox.net","threadId":"6960","inReplyTo":"7virdprejh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/8] diff: support reading a file from stdin via \"-\"","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-26T00:03:47Z","receivedAt":"2007-02-26T00:03:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Another hack just occurred to me... get_pathspec() could\ntranslate '-' to /dev/stdin while leaving './-' to '-'.\n\nIt could be rather too ugly to the taste of some people.\nMyself, I dunno...\n"},{"id":"35480","messageId":"Pine.LNX.4.63.0702260139290.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6960","inReplyTo":"7virdprejh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 4/8] diff: support reading a file from stdin via \"-\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-26T00:44:02Z","receivedAt":"2007-02-26T00:44:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Feb 2007, Junio C Hamano wrote:\n\n> I think the callers of populate_filespec() may need to pass around \n> diff_options as a parameter for the above to work, but hopefully that \n> should not be a rocket surgery.\n\nThat's a good idea! Actually, that pointer can be the pointer in the \ndiff_filespec itself!\n\nThis is my current version of [4/8] (Gosh, I really _love_ rebase -i ;-):\n\n-- snipsnap --\n[PATCH] diff: support reading a file from stdin via \"-\"\n\nThis allows you to say\n\n\techo Hello World | git diff x -\n\nto compare the contents of file \"x\" with the line \"Hello World\".\nThis automatically switches to --no-index mode.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tThis gives revs->max_count yet another meaning, oh well...\n\n\tThe diff is a bit borked, because get_mode() and the first part \n\tof the old queue_diff() are apparently a little similar...\n\n diff-lib.c |   92 ++++++++++++++++++++++++++++++++++++++++++-----------------\n 1 files changed, 65 insertions(+), 27 deletions(-)\n\ndiff --git a/diff-lib.c b/diff-lib.c\nindex 6678e22..9302d2b 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -30,22 +30,61 @@ static int read_directory(const char *path, struct path_list *list)\n \treturn 0;\n }\n \n-static int queue_diff(struct diff_options *o,\n-\t\tconst char *name1, const char *name2)\n+static int get_mode(const char *path, int *mode, int seen_dashdash)\n {\n \tstruct stat st;\n-\tint mode1 = 0, mode2 = 0;\n \n-\tif (name1) {\n-\t\tif (stat(name1, &st))\n-\t\t\treturn error(\"Could not access '%s'\", name1);\n-\t\tmode1 = st.st_mode;\n+\tif (!path)\n+\t\t*mode = 0;\n+\telse if (!seen_dashdash && !strcmp(path, \"-\"))\n+\t\t*mode = 020600;\n+\telse if (stat(path, &st))\n+\t\treturn error(\"Could not access '%s'\", path);\n+\telse\n+\t\t*mode = st.st_mode;\n+\treturn 0;\n+}\n+\n+static void read_stdin(void **buffer, unsigned long *size)\n+{\n+\tchar *buf = NULL;\n+#define INCREMENT 1024\n+\tint i = INCREMENT;\n+\n+\t*size = 0;\n+\tbuf = NULL;\n+\twhile (i == INCREMENT) {\n+\t\tbuf = xrealloc(buf, *size + INCREMENT);\n+\t\ti = xread(0, buf + *size, INCREMENT);\n+\t\t*size += i;\n \t}\n-\tif (name2) {\n-\t\tif (stat(name2, &st))\n-\t\t\treturn error(\"Could not access '%s'\", name2);\n-\t\tmode2 = st.st_mode;\n+\t*buffer = buf;\n+}\n+\n+static struct diff_filespec *make_diff_filespec(const char *path, int mode)\n+{\n+\tstruct diff_filespec *result;\n+\tif (!path)\n+\t\tpath = \"/dev/null\";\n+\tresult = alloc_filespec(path);\n+\tfill_filespec(result, null_sha1, mode);\n+\tif (mode == 020600) {\n+\t\tread_stdin(&result->data, &result->size);\n+\t\tresult->mode = 0100644;\n+\t\tresult->should_munmap = 0;\n+\t\tresult->should_free = 1;\n \t}\n+\treturn result;\n+}\n+\n+static int queue_diff(struct diff_options *o,\n+\t\tconst char *name1, const char *name2, int seen_dashdash)\n+{\n+\tint mode1 = 0, mode2 = 0;\n+\n+\tif (get_mode(name1, &mode1, seen_dashdash) ||\n+\t\t\tget_mode(name2, &mode2, seen_dashdash))\n+\t\treturn -1;\n \n \tif (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2))\n \t\treturn error(\"file/directory conflict: %s, %s\", name1, name2);\n@@ -106,7 +145,7 @@ static int queue_diff(struct diff_options *o,\n \t\t\t\t\t\tPATH_MAX - len2);\n \t\t\t}\n \n-\t\t\tret = queue_diff(o, n1, n2);\n+\t\t\tret = queue_diff(o, n1, n2, 1);\n \t\t}\n \t\tpath_list_clear(&p1, 0);\n \t\tpath_list_clear(&p2, 0);\n@@ -122,14 +161,8 @@ static int queue_diff(struct diff_options *o,\n \t\t\ttmp_c = name1; name1 = name2; name2 = tmp_c;\n \t\t}\n \n-\t\tif (!name1)\n-\t\t\tname1 = \"/dev/null\";\n-\t\tif (!name2)\n-\t\t\tname2 = \"/dev/null\";\n-\t\td1 = alloc_filespec(name1);\n-\t\td2 = alloc_filespec(name2);\n-\t\tfill_filespec(d1, null_sha1, mode1);\n-\t\tfill_filespec(d2, null_sha1, mode2);\n+\t\td1 = make_diff_filespec(name1, mode1);\n+\t\td2 = make_diff_filespec(name2, mode2);\n \n \t\tdiff_queue(&diff_queued_diff, d1, d2);\n \t\treturn 0;\n@@ -222,11 +255,12 @@ static int is_outside_repo(const char *path, int nongit, const char *prefix)\n int setup_diff_no_index(struct rev_info *revs,\n \t\tint argc, const char ** argv, int nongit, const char *prefix)\n {\n-\tint i;\n+\tint i, seen_dashdash = 0;\n \tfor (i = 1; i < argc; i++)\n-\t\tif (argv[i][0] != '-')\n+\t\tif (argv[i][0] != '-' || argv[i][1] == '\\0')\n \t\t\tbreak;\n \t\telse if (!strcmp(argv[i], \"--\")) {\n+\t\t\tseen_dashdash = 1;\n \t\t\ti++;\n \t\t\tbreak;\n \t\t} else if (i < argc - 3 && !strcmp(argv[i], \"--no-index\")) {\n@@ -238,7 +272,7 @@ int setup_diff_no_index(struct rev_info *revs,\n \t\treturn -1;\n \n \tdiff_setup(&revs->diffopt);\n-\tfor (i = 1; i < argc - 2; )\n+\tfor (i = 1; i < argc - 2 - seen_dashdash; )\n \t\tif (!strcmp(argv[i], \"--no-index\"))\n \t\t\ti++;\n \t\telse {\n@@ -250,22 +284,26 @@ int setup_diff_no_index(struct rev_info *revs,\n \t\t}\n \trevs->diffopt.paths = argv + argc - 2;\n \trevs->diffopt.nr_paths = 2;\n-\trevs->max_count = -2;\n+\trevs->max_count = -2 - seen_dashdash;\n \treturn 0;\n }\n \n int run_diff_files_cmd(struct rev_info *revs, int argc, const char **argv)\n {\n-\tint silent_on_removed;\n+\tint silent_on_removed, seen_dashdash = 0;\n \n-\tif (handle_diff_files_args(revs, argc, argv, &silent_on_removed))\n+\tif (revs->max_count == -3) {\n+\t\tseen_dashdash = 1;\n+\t\trevs->max_count = -2;\n+\t} else if (handle_diff_files_args(revs, argc, argv,\n+\t\t\t\t&silent_on_removed))\n \t\treturn -1;\n \n \tif (revs->max_count == -2) {\n \t\tif (revs->diffopt.nr_paths != 2)\n \t\t\treturn error(\"need two files/directories with --no-index\");\n \t\tif (queue_diff(&revs->diffopt, revs->diffopt.paths[0],\n-\t\t\t\trevs->diffopt.paths[1]))\n+\t\t\t\trevs->diffopt.paths[1], seen_dashdash))\n \t\t\treturn -1;\n \t\tdiffcore_std(&revs->diffopt);\n \t\tdiff_flush(&revs->diffopt);\n-- \n1.5.0.1.784.g105d9-dirty\n"}]}