{"thread":{"id":"45426","subject":"[PATCH v3 0/2] diff --no-index: support symlinks and pipes","startedAt":"2017-03-18T21:26:49Z","lastAt":"2017-03-20T16:09:03Z","messageCount":8,"participants":["Dennis Kaarsemaker","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":2},"messages":[{"id":"314554","messageId":"20170318210038.22638-1-dennis@kaarsemaker.net","threadId":"45426","inReplyTo":null,"subject":"[PATCH v3 0/2] diff --no-index: support symlinks and pipes","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-03-18T21:00:36Z","receivedAt":"2017-03-18T21:26:49Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"git diff <(command1) <(command2) is less useful than it could be, all it outputs is:\n\ndiff --git a/dev/fd/63 b/dev/fd/62\nindex 9e6542b297..9f7b2c291b 120000\n--- a/dev/fd/63\n+++ b/dev/fd/62\n@@ -1 +1 @@\n-pipe:[464811685]\n\\ No newline at end of file\n+pipe:[464811687]\n\\ No newline at end of file\n\nNormal diff provides arguably better output: the diff of the output of the\ncommands. This series makes it possible for git diff --no-index to follow\nsymlinks and read from pipes, mimicking the behaviour of normal diff.\n\nv1: http://public-inbox.org/git/20161111201958.2175-1-dennis@kaarsemaker.net/\nv2: http://public-inbox.org/git/20170113102021.6054-1-dennis@kaarsemaker.net/\n\nChanges since v2, prompted by feedback from Junio:\n\n- A --derefence option was added and the default is no longer to dereference\n  symlinks.\n- Instead of looking at what canon_mode returns, use the original mode of \n  files to override behaviour for pipes.\n- Turn the !S_ISREG(...) check into a should_mmap_file_contents helper.\n\nDennis Kaarsemaker (2):\n  diff --no-index: optionally follow symlinks\n  diff --no-index: support reading from pipes\n\n Documentation/diff-options.txt |  9 +++++++\n diff-no-index.c                | 16 ++++++++++---\n diff.c                         | 30 +++++++++++++++++++----\n diff.h                         |  2 +-\n t/t4053-diff-no-index.sh       | 54 ++++++++++++++++++++++++++++++++++++++++++\n t/test-lib.sh                  |  4 ++++\n 6 files changed, 107 insertions(+), 8 deletions(-)\n\n-- \n2.12.0-437-g0cc2799\n\n"},{"id":"314555","messageId":"20170318210038.22638-3-dennis@kaarsemaker.net","threadId":"45426","inReplyTo":"20170318210038.22638-1-dennis@kaarsemaker.net","subject":"[PATCH v3 2/2] diff --no-index: support reading from pipes","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-03-18T21:00:38Z","receivedAt":"2017-03-18T21:26:50Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"diff <(command1) <(command2) provides useful output, let's make it\npossible for git to do the same.\n\nSigned-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n---\n diff-no-index.c          |  9 +++++++++\n diff.c                   | 18 ++++++++++++++++--\n t/t4053-diff-no-index.sh | 10 ++++++++++\n t/test-lib.sh            |  4 ++++\n 4 files changed, 39 insertions(+), 2 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex fe48f32ddd..1262a587e5 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -83,6 +83,15 @@ static struct diff_filespec *noindex_filespec(const char *name, int mode)\n \t\tname = \"/dev/null\";\n \ts = alloc_filespec(name);\n \tfill_filespec(s, null_sha1, 0, mode);\n+\t/*\n+\t * In --no-index mode, we support reading from pipes. canon_mode, called by\n+\t * fill_filespec, gets confused by this and thinks we now have subprojects.\n+\t * To help the rest of the diff machinery along, we now override what\n+\t * canon_mode says. This is done here instead of in canon_mode, because the\n+\t * rest of git does not (and should not) support pipes.\n+\t */\n+\tif (S_ISFIFO(mode))\n+\t\ts->mode = S_IFREG | ce_permissions(mode);\n \tif (name == file_from_standard_input)\n \t\tpopulate_from_stdin(s);\n \treturn s;\ndiff --git a/diff.c b/diff.c\nindex 2afecfb939..4f74a54d74 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2765,6 +2765,11 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)\n \treturn 0;\n }\n \n+static int should_mmap_file_contents(struct stat *st)\n+{\n+\treturn S_ISREG(st->st_mode);\n+}\n+\n /*\n  * While doing rename detection and pickaxe operation, we may need to\n  * grab the data for the blob (or file) for our own in-core comparison.\n@@ -2839,9 +2844,18 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\tfd = open(s->path, O_RDONLY);\n \t\tif (fd < 0)\n \t\t\tgoto err_empty;\n-\t\ts->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\tif (!should_mmap_file_contents(&st)) {\n+\t\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\t\tstrbuf_read(&sb, fd, 0);\n+\t\t\ts->size = sb.len;\n+\t\t\ts->data = strbuf_detach(&sb, NULL);\n+\t\t\ts->should_free = 1;\n+\t\t}\n+\t\telse {\n+\t\t\ts->data = xmmap(NULL, s->size, PROT_READ, MAP_PRIVATE, fd, 0);\n+\t\t\ts->should_munmap = 1;\n+\t\t}\n \t\tclose(fd);\n-\t\ts->should_munmap = 1;\n \n \t\t/*\n \t\t * Convert from working tree format to canonical git format\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 8c87bffb34..2d9b322315 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -171,4 +171,14 @@ test_expect_success SYMLINKS 'diff --no-index --no-dereference does not follow s\n \ttest_cmp expect actual\n '\n \n+test_expect_success PROCESS_SUBSTITUTION 'diff --no-index works on fifos' '\n+\tcat >expect <<-EOF &&\n+\t\t@@ -1 +1 @@\n+\t\t-1\n+\t\t+2\n+\tEOF\n+\ttest_expect_code 1 git diff --no-index --dereference <(echo 1) <(echo 2) | tail -n +5 > actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 11562bde10..78f3d24651 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1128,3 +1128,7 @@ build_option () {\n test_lazy_prereq LONG_IS_64BIT '\n \ttest 8 -le \"$(build_option sizeof-long)\"\n '\n+\n+test_lazy_prereq PROCESS_SUBSTITUTION '\n+\teval \"foo=<(echo test)\" 2>/dev/null\n+'\n-- \n2.12.0-437-g0cc2799\n\n"},{"id":"314557","messageId":"20170318210038.22638-2-dennis@kaarsemaker.net","threadId":"45426","inReplyTo":"20170318210038.22638-1-dennis@kaarsemaker.net","subject":"[PATCH v3 1/2] diff --no-index: optionally follow symlinks","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-03-18T21:00:37Z","receivedAt":"2017-03-18T21:26:55Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"Git's diff machinery does not follow symlinks, which makes sense as git\nitself also does not, but stores the symlink destination.\n\nIn --no-index mode however, it is useful for diff to be able to follow\nsymlinks, matching the behaviour of ordinary diff. A new --dereference\n(name copied from diff) option has been added to enable this behaviour.\n--no-dereference can be used to disable it again.\n\nSigned-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n---\n Documentation/diff-options.txt |  9 +++++++++\n diff-no-index.c                |  7 ++++---\n diff.c                         | 12 ++++++++++--\n diff.h                         |  2 +-\n t/t4053-diff-no-index.sh       | 44 ++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 68 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 2d77a19626..5a9d58b701 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -216,6 +216,15 @@ any of those replacements occurred.\n \tcommit range.  Defaults to `diff.submodule` or the 'short' format\n \tif the config option is unset.\n \n+ifdef::git-diff[]\n+--dereference::\n+--no-dereference::\n+\tNormally, \"git diff --no-index\" will compare symlinks by comparing what\n+\tthey point to. The `--dereference` option will make it compare the content\n+\tof the linked files. The `--no-dereference` option disables an earlier\n+\t`--dereference`.\n+endif::git-diff[]\n+\n --color[=<when>]::\n \tShow colored diff.\n \t`--color` (i.e. without '=<when>') is the same as `--color=always`.\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex f420786039..fe48f32ddd 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -40,7 +40,7 @@ static int read_directory_contents(const char *path, struct string_list *list)\n  */\n static const char file_from_standard_input[] = \"-\";\n \n-static int get_mode(const char *path, int *mode)\n+static int get_mode(const char *path, int *mode, int dereference)\n {\n \tstruct stat st;\n \n@@ -52,7 +52,7 @@ static int get_mode(const char *path, int *mode)\n #endif\n \telse if (path == file_from_standard_input)\n \t\t*mode = create_ce_mode(0666);\n-\telse if (lstat(path, &st))\n+\telse if (dereference ? stat(path, &st) : lstat(path, &st))\n \t\treturn error(\"Could not access '%s'\", path);\n \telse\n \t\t*mode = st.st_mode;\n@@ -93,7 +93,8 @@ static int queue_diff(struct diff_options *o,\n {\n \tint mode1 = 0, mode2 = 0;\n \n-\tif (get_mode(name1, &mode1) || get_mode(name2, &mode2))\n+\tif (get_mode(name1, &mode1, DIFF_OPT_TST(o, DEREFERENCE)) ||\n+\t\tget_mode(name2, &mode2, DIFF_OPT_TST(o, DEREFERENCE)))\n \t\treturn -1;\n \n \tif (mode1 && mode2 && S_ISDIR(mode1) != S_ISDIR(mode2)) {\ndiff --git a/diff.c b/diff.c\nindex be11e4ef2b..2afecfb939 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2815,7 +2815,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\ts->size = xsize_t(st.st_size);\n \t\tif (!s->size)\n \t\t\tgoto empty;\n-\t\tif (S_ISLNK(st.st_mode)) {\n+\t\tif (S_ISLNK(s->mode)) {\n \t\t\tstruct strbuf sb = STRBUF_INIT;\n \n \t\t\tif (strbuf_readlink(&sb, s->path, s->size))\n@@ -2825,6 +2825,10 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n \t\t\ts->should_free = 1;\n \t\t\treturn 0;\n \t\t}\n+\t\tif (S_ISLNK(st.st_mode)) {\n+\t\t\tstat(s->path, &st);\n+\t\t\ts->size = xsize_t(st.st_size);\n+\t\t}\n \t\tif (size_only)\n \t\t\treturn 0;\n \t\tif ((flags & CHECK_BINARY) &&\n@@ -3884,7 +3888,11 @@ int diff_opt_parse(struct diff_options *options,\n \telse if (!strcmp(arg, \"--no-follow\")) {\n \t\tDIFF_OPT_CLR(options, FOLLOW_RENAMES);\n \t\tDIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);\n-\t} else if (!strcmp(arg, \"--color\"))\n+\t} else if (!strcmp(arg, \"--dereference\"))\n+\t\tDIFF_OPT_SET(options, DEREFERENCE);\n+\telse if (!strcmp(arg, \"--no-dereference\"))\n+\t\tDIFF_OPT_CLR(options, DEREFERENCE);\n+\telse if (!strcmp(arg, \"--color\"))\n \t\toptions->use_color = 1;\n \telse if (skip_prefix(arg, \"--color=\", &arg)) {\n \t\tint value = git_config_colorbool(NULL, arg);\ndiff --git a/diff.h b/diff.h\nindex 25ae60d5ff..db33dc67f6 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -69,7 +69,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data)\n #define DIFF_OPT_FIND_COPIES_HARDER  (1 <<  6)\n #define DIFF_OPT_FOLLOW_RENAMES      (1 <<  7)\n #define DIFF_OPT_RENAME_EMPTY        (1 <<  8)\n-/* (1 <<  9) unused */\n+#define DIFF_OPT_DEREFERENCE         (1 <<  9)\n #define DIFF_OPT_HAS_CHANGES         (1 << 10)\n #define DIFF_OPT_QUICK               (1 << 11)\n #define DIFF_OPT_NO_INDEX            (1 << 12)\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 453e6c35eb..8c87bffb34 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -127,4 +127,48 @@ test_expect_success 'diff --no-index from repo subdir respects config (implicit)\n \ttest_cmp expect actual.head\n '\n \n+test_expect_success SYMLINKS 'diff --no-index does not follows symlinks' '\n+\techo a >1 &&\n+\techo b >2 &&\n+\tln -s 1 3 &&\n+\tln -s 2 4 &&\n+\tcat >expect <<-EOF &&\n+\t\t--- a/3\n+\t\t+++ b/4\n+\t\t@@ -1 +1 @@\n+\t\t-1\n+\t\t\\ No newline at end of file\n+\t\t+2\n+\t\t\\ No newline at end of file\n+\tEOF\n+\ttest_expect_code 1 git diff --no-index 3 4 | tail -n +3 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success SYMLINKS 'diff --no-index --dereference does follows symlinks' '\n+\tcat >expect <<-EOF &&\n+\t\t--- a/3\n+\t\t+++ b/4\n+\t\t@@ -1 +1 @@\n+\t\t-a\n+\t\t+b\n+\tEOF\n+\ttest_expect_code 1 git diff --no-index --dereference 3 4 | tail -n +3 >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success SYMLINKS 'diff --no-index --no-dereference does not follow symlinks' '\n+\tcat >expect <<-EOF &&\n+\t\t--- a/3\n+\t\t+++ b/4\n+\t\t@@ -1 +1 @@\n+\t\t-1\n+\t\t\\ No newline at end of file\n+\t\t+2\n+\t\t\\ No newline at end of file\n+\tEOF\n+\ttest_expect_code 1 git diff --no-index --no-dereference 3 4 | tail -n +3 > actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.12.0-437-g0cc2799\n\n"},{"id":"314613","messageId":"xmqqo9wwzzpl.fsf@gitster.mtv.corp.google.com","threadId":"45426","inReplyTo":"20170318210038.22638-1-dennis@kaarsemaker.net","subject":"Re: [PATCH v3 0/2] diff --no-index: support symlinks and pipes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-19T22:08:54Z","receivedAt":"2017-03-19T22:09:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:\n\n> Normal diff provides arguably better output: the diff of the output of the\n> commands. This series makes it possible for git diff --no-index to follow\n> symlinks and read from pipes, mimicking the behaviour of normal diff.\n>\n> v1: http://public-inbox.org/git/20161111201958.2175-1-dennis@kaarsemaker.net/\n> v2: http://public-inbox.org/git/20170113102021.6054-1-dennis@kaarsemaker.net/\n>\n> Changes since v2, prompted by feedback from Junio:\n>\n> - A --derefence option was added and the default is no longer to dereference\n>   symlinks.\n\nI do agree that it makes sense to have --[no-]dereference options,\nbut I do not think it was my feedback and suggestion to make it\noptional (not default) to dereference, so please do not blame me for\nthat choice.\n\n"},{"id":"314614","messageId":"xmqqk27kzzfm.fsf@gitster.mtv.corp.google.com","threadId":"45426","inReplyTo":"20170318210038.22638-2-dennis@kaarsemaker.net","subject":"Re: [PATCH v3 1/2] diff --no-index: optionally follow symlinks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-19T22:14:53Z","receivedAt":"2017-03-19T22:15:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:\n\n> diff --git a/diff.c b/diff.c\n> index be11e4ef2b..2afecfb939 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -2815,7 +2815,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n>  \t\ts->size = xsize_t(st.st_size);\n>  \t\tif (!s->size)\n>  \t\t\tgoto empty;\n> -\t\tif (S_ISLNK(st.st_mode)) {\n> +\t\tif (S_ISLNK(s->mode)) {\n>  \t\t\tstruct strbuf sb = STRBUF_INIT;\n>  \n>  \t\t\tif (strbuf_readlink(&sb, s->path, s->size))\n> @@ -2825,6 +2825,10 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n>  \t\t\ts->should_free = 1;\n>  \t\t\treturn 0;\n>  \t\t}\n> +\t\tif (S_ISLNK(st.st_mode)) {\n> +\t\t\tstat(s->path, &st);\n> +\t\t\ts->size = xsize_t(st.st_size);\n\nDoesn't this affect --no-index mode?  We never need to do a wasteful\nstat() after lstat() and we are penalizing the normal codepath with\nthis change, no?\n\n> @@ -3884,7 +3888,11 @@ int diff_opt_parse(struct diff_options *options,\n>  \telse if (!strcmp(arg, \"--no-follow\")) {\n>  \t\tDIFF_OPT_CLR(options, FOLLOW_RENAMES);\n>  \t\tDIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);\n> -\t} else if (!strcmp(arg, \"--color\"))\n> +\t} else if (!strcmp(arg, \"--dereference\"))\n> +\t\tDIFF_OPT_SET(options, DEREFERENCE);\n> +\telse if (!strcmp(arg, \"--no-dereference\"))\n> +\t\tDIFF_OPT_CLR(options, DEREFERENCE);\n> +\telse if (!strcmp(arg, \"--color\"))\n>  \t\toptions->use_color = 1;\n\nAlso shouldn't be some code to detect --[no-]dereference options\ngiven when --no-index is not in effect and error out?  As the patch\ntitle says, this change should be a no-op for normal codepath and\nonly affect the no-index hack.\n"},{"id":"314636","messageId":"1490007035.15470.14.camel@kaarsemaker.net","threadId":"45426","inReplyTo":"xmqqk27kzzfm.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 1/2] diff --no-index: optionally follow symlinks","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-03-20T10:50:35Z","receivedAt":"2017-03-20T10:50:52Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On Sun, 2017-03-19 at 15:14 -0700, Junio C Hamano wrote:\n> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:\n> \n> > diff --git a/diff.c b/diff.c\n> > index be11e4ef2b..2afecfb939 100644\n> > --- a/diff.c\n> > +++ b/diff.c\n> > @@ -2815,7 +2815,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n> >  \t\ts->size = xsize_t(st.st_size);\n> >  \t\tif (!s->size)\n> >  \t\t\tgoto empty;\n> > -\t\tif (S_ISLNK(st.st_mode)) {\n> > +\t\tif (S_ISLNK(s->mode)) {\n> >  \t\t\tstruct strbuf sb = STRBUF_INIT;\n> >  \n> >  \t\t\tif (strbuf_readlink(&sb, s->path, s->size))\n> > @@ -2825,6 +2825,10 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)\n> >  \t\t\ts->should_free = 1;\n> >  \t\t\treturn 0;\n> >  \t\t}\n> > +\t\tif (S_ISLNK(st.st_mode)) {\n> > +\t\t\tstat(s->path, &st);\n> > +\t\t\ts->size = xsize_t(st.st_size);\n> \n> Doesn't this affect --no-index mode?  We never need to do a wasteful\n> stat() after lstat() and we are penalizing the normal codepath with\n> this change, no?\n\nthe S_ISLNK(s->mode) conditional above is for the normal codepath,\nwhich returns early. So the stat I added is only done for symlinks in\nno_index mode.\n\n> > @@ -3884,7 +3888,11 @@ int diff_opt_parse(struct diff_options *options,\n> >  \telse if (!strcmp(arg, \"--no-follow\")) {\n> >  \t\tDIFF_OPT_CLR(options, FOLLOW_RENAMES);\n> >  \t\tDIFF_OPT_CLR(options, DEFAULT_FOLLOW_RENAMES);\n> > -\t} else if (!strcmp(arg, \"--color\"))\n> > +\t} else if (!strcmp(arg, \"--dereference\"))\n> > +\t\tDIFF_OPT_SET(options, DEREFERENCE);\n> > +\telse if (!strcmp(arg, \"--no-dereference\"))\n> > +\t\tDIFF_OPT_CLR(options, DEREFERENCE);\n> > +\telse if (!strcmp(arg, \"--color\"))\n> >  \t\toptions->use_color = 1;\n> \n> Also shouldn't be some code to detect --[no-]dereference options\n> given when --no-index is not in effect and error out?  As the patch\n> title says, this change should be a no-op for normal codepath and\n> only affect the no-index hack.\n\nBut erroring out isn't a no-op. With the current patch you can do \n--dereference without --no-index and it simply wouldn't affect\nanything. \n\nI don't mind either way, so I'll make it error out.\n-- \nDennis Kaarsemaker\nhttp://www.kaarsemaker.net\n"},{"id":"314643","messageId":"1490006404.15470.12.camel@kaarsemaker.net","threadId":"45426","inReplyTo":"xmqqo9wwzzpl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3 0/2] diff --no-index: support symlinks and pipes","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2017-03-20T10:40:04Z","receivedAt":"2017-03-20T12:36:23Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On Sun, 2017-03-19 at 15:08 -0700, Junio C Hamano wrote:\n> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:\n> \n> > Normal diff provides arguably better output: the diff of the output of the\n> > commands. This series makes it possible for git diff --no-index to follow\n> > symlinks and read from pipes, mimicking the behaviour of normal diff.\n> > \n> > v1: http://public-inbox.org/git/20161111201958.2175-1-dennis@kaarsemaker.net/\n> > v2: http://public-inbox.org/git/20170113102021.6054-1-dennis@kaarsemaker.net/\n> > \n> > Changes since v2, prompted by feedback from Junio:\n> > \n> > - A --derefence option was added and the default is no longer to dereference\n> >   symlinks.\n> \n> I do agree that it makes sense to have --[no-]dereference options,\n> but I do not think it was my feedback and suggestion to make it\n> optional (not default) to dereference, so please do not blame me for\n> that choice.\n\nThen I misinterpreted your message at \nhttp://public-inbox.org/git/xmqqk29yedkv.fsf@gitster.mtv.corp.google.com/\nNo blame inteded, my apologies for coming across as blaming.\n\n-- \nDennis Kaarsemaker\nhttp://www.kaarsemaker.net\n"},{"id":"314657","messageId":"xmqqa88gx7gj.fsf@gitster.mtv.corp.google.com","threadId":"45426","inReplyTo":"1490006404.15470.12.camel@kaarsemaker.net","subject":"Re: [PATCH v3 0/2] diff --no-index: support symlinks and pipes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-03-20T16:02:04Z","receivedAt":"2017-03-20T16:09:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:\n\n> On Sun, 2017-03-19 at 15:08 -0700, Junio C Hamano wrote:\n> ...\n>> > - A --derefence option was added and the default is no longer to dereference\n>> >   symlinks.\n>> \n>> I do agree that it makes sense to have --[no-]dereference options,\n>> but I do not think it was my feedback and suggestion to make it\n>> optional (not default) to dereference, so please do not blame me for\n>> that choice.\n>\n> Then I misinterpreted your message at \n> http://public-inbox.org/git/xmqqk29yedkv.fsf@gitster.mtv.corp.google.com/\n> No blame inteded, my apologies for coming across as blaming.\n\ns/blame/credit/ then.  I do not too deeply care which one is the\ndefault, and if we were adding --no-index without any existing users\ntoday, I probably would suggest making it deref by default (i.e. to\nmake \"diff --no-index\" match better what other peoples' diffs do),\nbut that would be a behaviour change to existing users if done today,\nso I think what you did probably is a good thing.\n\nThanks.\n"}]}