{"thread":{"id":"27430","subject":"[PATCH] Improve errors from 'git diff --no-index'.","startedAt":"2011-05-22T19:17:21Z","lastAt":"2011-05-23T21:24:50Z","messageCount":7,"participants":["Anthony Foiani","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"168459","messageId":"4dd98da1.1bf98e0a.4eb4.6fc5@mx.google.com","threadId":"27430","inReplyTo":null,"subject":"[PATCH] Improve errors from 'git diff --no-index'.","fromName":"Anthony Foiani","fromEmail":"anthony.foiani@gmail.com","sentAt":"2011-05-22T19:17:21Z","receivedAt":"2011-05-22T19:17:21Z","isPatch":true,"sender":{"key":"anthony.foiani@gmail.com","avatar":"https://gravatar.com/avatar/dc2ca9a2ec133021e09ad6a322d4951c7355aefcf12889c5ac837eb16187c2b9?d=mp&s=160"},"body":"I accidentally tried to use \"git diff\" when I wasn't within a git\nrepository, only to be confused by getting a usage message with no\nparticular error output.\n\nThis patch tries to provide better explanations to the user when the diff\ncannot be done.\n\nSigned-off-by: Anthony Foiani <anthony.foiani@gmail.com>\n---\n diff-no-index.c |  122 ++++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 95 insertions(+), 27 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 3a36144..3172788 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -149,21 +149,56 @@ static int queue_diff(struct diff_options *o,\n \t}\n }\n \n-static int path_outside_repo(const char *path)\n+/**\n+ * path_in_repo() - Determine whether @path is within a git directory/repo.\n+ * @path: pathspec to test\n+ *\n+ * Returns true if @path is equal to, or lies within, the git tree\n+ * returned by get_git_work_tree.\n+ */\n+static int path_in_repo(const char *path)\n {\n-\tconst char *work_tree;\n-\tsize_t len;\n+\tconst char *tree;\n+\tconst char *abs_path;\n+\tconst char *abs_tree;\n+\tchar norm_path[PATH_MAX+1];\n+\tchar norm_tree[PATH_MAX+1];\n+\tsize_t tree_len;\n+\tsize_t path_len;\n+\n+\t/* Paranoia. */\n+\tif (!path)\n+\t\treturn 0;\n \n-\tif (!is_absolute_path(path))\n+\t/* If there's no tree, then there's no repo to be in. */\n+\ttree = get_git_work_tree();\n+\tif (!tree)\n \t\treturn 0;\n-\twork_tree = get_git_work_tree();\n-\tif (!work_tree)\n-\t\treturn 1;\n-\tlen = strlen(work_tree);\n-\tif (strncmp(path, work_tree, len) ||\n-\t    (path[len] != '\\0' && path[len] != '/'))\n-\t\treturn 1;\n-\treturn 0;\n+\n+\t/* Get the full paths and normalize them.. */\n+\tabs_path = real_path(path);\n+\tif (normalize_path_copy(norm_path, abs_path))\n+\t\tdie(\"could not normalize path '%s'\", abs_path);\n+\tabs_tree = real_path(tree);\n+\tif (normalize_path_copy(norm_tree, abs_tree))\n+\t\tdie(\"could not normalize tree '%s'\", abs_tree);\n+\n+\t/* See if the work tree is a prefix *directory* of the given path. */\n+\tpath_len = strlen(norm_path);\n+\ttree_len = strlen(norm_tree);\n+\n+\tif (path_len < tree_len)\n+\t\treturn 0; /* Can't be a subdir if it's shorter. */\n+\n+\tif (strncmp(norm_path, norm_tree, tree_len))\n+\t\treturn 0; /* Nor if the first 'tree_len' bytes don't match. */\n+\n+\tif (norm_path[tree_len] == '\\0')\n+\t\treturn 1; /* The path and tree are the same. */\n+\tif (norm_path[tree_len] == '/')\n+\t\treturn 1; /* The path is a subdir of the tree. */\n+\n+\treturn 0; /* Nope. */\n }\n \n void diff_no_index(struct rev_info *revs,\n@@ -186,22 +221,55 @@ void diff_no_index(struct rev_info *revs,\n \t\t\tbreak;\n \t}\n \n-\tif (!no_index && !nongit) {\n-\t\t/*\n-\t\t * Inside a git repository, without --no-index.  Only\n-\t\t * when a path outside the repository is given,\n-\t\t * e.g. \"git diff /var/tmp/[12]\", or \"git diff\n-\t\t * Makefile /var/tmp/Makefile\", allow it to be used as\n-\t\t * a colourful \"diff\" replacement.\n-\t\t */\n-\t\tif ((argc != i + 2) ||\n-\t\t    (!path_outside_repo(argv[i]) &&\n-\t\t     !path_outside_repo(argv[i+1])))\n-\t\t\treturn;\n+\t/* How many pathspecs were given on the command line? */\n+\tconst int n_paths = (argc - i);\n+\n+\t/* If --no-index wasn't specified explicitly, check to see if\n+\t * we need to force it. */\n+\tif (!no_index) {\n+\n+\t\t/* The first pathspec is the \"left hand side\" */\n+\t\tconst char *lhs = (n_paths >= 1) ? argv[i] : NULL;\n+\t\tconst int lhs_in_repo = lhs && path_in_repo(lhs);\n+\n+\t\t/* And the second is the \"right hand side\" */\n+\t\tconst char *rhs = (n_paths >= 2) ? argv[i+1] : NULL;\n+\t\tconst int rhs_in_repo = rhs && path_in_repo(rhs);\n+\n+\t\tint force_no_index = 0;\n+\n+\t\tif (nongit) {\n+\t\t\twarning(\"'git diff' outside a repo, forcing --no-index\");\n+\t\t\tforce_no_index = 1;\n+\t\t}\n+\t\telse {\n+\t\t\tif (lhs_in_repo || rhs_in_repo)\n+\t\t\t\treturn; /* Normal git diff, let core handle it. */\n+\n+\t\t\tconst int lhs_untracked = (lhs && !lhs_in_repo);\n+\t\t\tconst int rhs_untracked = (rhs && !rhs_in_repo);\n+\n+\t\t\tif (lhs_untracked && rhs_untracked)\n+\t\t\t\twarning(\"neither '%s' nor '%s' are tracked,\"\n+\t\t\t\t\t\" forcing --no-index\", lhs, rhs );\n+\t\t\telse if (lhs_untracked)\n+\t\t\t\twarning(\"'%s' is not tracked,\"\n+\t\t\t\t\t\" forcing --no-index\", lhs );\n+\t\t\telse if (rhs_untracked)\n+\t\t\t\twarning(\"'%s' is not tracked,\"\n+\t\t\t\t\t\" forcing --no-index\", rhs );\n+\n+\t\t\tforce_no_index = lhs_untracked || rhs_untracked;\n+\t\t}\n+\n+\t\tif (!force_no_index) /* Impossible? */\n+\t\t\tdie(\"--no-index not set and not forced\");\n+\t}\n+\n+\tif (n_paths != 2) {\n+\t\twarning(\"no-index mode requires exactly two pathspecs\");\n+\t\tusage(\"git diff --no-index <path> <path>\");\n \t}\n-\tif (argc != i + 2)\n-\t\tusagef(\"git diff %s <path> <path>\",\n-\t\t       no_index ? \"--no-index\" : \"[--no-index]\");\n \n \tdiff_setup(&revs->diffopt);\n \tfor (i = 1; i < argc - 2; ) {\n-- \n1.7.4.4\n"},{"id":"168476","messageId":"BANLkTinKpSP5oEms914TWD7Tsjab1B87QQ@mail.gmail.com","threadId":"27430","inReplyTo":"7vlixyw4cx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Improve errors from 'git diff --no-index'.","fromName":"Anthony Foiani","fromEmail":"anthony.foiani@gmail.com","sentAt":"2011-05-23T02:35:40Z","receivedAt":"2011-05-23T02:35:40Z","isPatch":true,"sender":{"key":"anthony.foiani@gmail.com","avatar":"https://gravatar.com/avatar/dc2ca9a2ec133021e09ad6a322d4951c7355aefcf12889c5ac837eb16187c2b9?d=mp&s=160"},"body":"Junio --\n\nOn Sun, May 22, 2011 at 8:14 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Anthony Foiani <anthony.foiani@gmail.com> writes:\n>\n>> I accidentally tried to use \"git diff\" when I wasn't within a git\n>> repository, only to be confused by getting a usage message with no\n>> particular error output.\n>\n> I do not understand this at all.\n>\n>    $ cd /\n>    $ git diff\n>    usage: git diff [--no-index] <path> <path>\n>\n> What's unclear about it?\n\nIt's hard for me to explain right now, as I just spent a few hours\nfiguring out exactly what \"git diff\" assumed would happen.  And now\nthat I have done that study, I would not need anything beyond the\noriginal error message, either.\n\nBut when I screwed up earlier today, I was somewhat taken aback by the\nfact that I kept on getting the same usage message with no indication\nof what the heck I was doing wrong:\n\n$ git diff -b main.c\nusage: git diff [--no-index] <path> <path>\n$ git diff --cached -b main.c\nusage: git diff [--no-index] <path> <path>\n$ git diff -b HEAD -- main.c\nusage: git diff [--no-index] <path> <path>\n$ git diff -b -- main.c\nusage: git diff [--no-index] <path> <path>\n\nIt would have saved me a chunk of time to have one of my diagnostics\n(in this case, \"not in a git repo\") to knock some sense into me\nearlier.\n\n*shrug* Toss it in the bit-bucket if you like, but the work's already\nbeen done. And I happen to find the new code more readable than the\nprevious triple-negative and de Morgan's-expanded conditionals, but\nthat's of course personal taste.\n\nFor that matter, the existing code was already doing many of these\nchecks, but not communicating the results back in any way other than\npresenting a usage string. I apparently interpreted that as \"you're\ncalling me wrong\", not \"you're using me wrong\".\n\nThere might be a bug even in the previous code, though: there's an odd\nearly exit from 'path_outside_repo' for absolute pathnames, but I got\nlost in the multiple negations and could never quite convince myself\nof its correctness (or lack thereof).\n\nEither way, thanks for all your work on git!\n\nBest regards,\nTony\n"},{"id":"168477","messageId":"7vhb8mw1e6.fsf@alter.siamese.dyndns.org","threadId":"27430","inReplyTo":"BANLkTinKpSP5oEms914TWD7Tsjab1B87QQ@mail.gmail.com","subject":"Re: [PATCH] Improve errors from 'git diff --no-index'.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-23T03:18:09Z","receivedAt":"2011-05-23T03:18:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anthony Foiani <anthony.foiani@gmail.com> writes:\n\n> On Sun, May 22, 2011 at 8:14 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Anthony Foiani <anthony.foiani@gmail.com> writes:\n>>\n>>> I accidentally tried to use \"git diff\" when I wasn't within a git\n>>> repository, only to be confused by getting a usage message with no\n>>> particular error output.\n>>\n>> I do not understand this at all.\n>>\n>>    $ cd /\n>>    $ git diff\n>>    usage: git diff [--no-index] <path> <path>\n>>\n>> What's unclear about it?\n>\n> It's hard for me to explain right now,...\n\nNo, it was only your 'I tried to use \"git diff\"' that was confusing to me.\n\nWith pathspecs or any other options, it is understandable that you were\nconfused, for example from an output like this:\n\n> $ git diff -b main.c\n> usage: git diff [--no-index] <path> <path>\n\nAt that point we are committed to no-index codepath, so probably a good\nthing to do is to update that confusing usage message a bit better.\n\nIt already does change the behaviour when --no-index is not given by the\nend user, so perhaps it would be sufficient to change this part:\n\n\tif (argc != i + 2)\n        \tusagef(\"git diff %s <path> <path>\",\n                \tno_index ? \"--no-index\" : \"[--no-index]\");\n\nto something like (caution: I am not typing this in my MUA):\n\n\tif (argc != i + 2) {\n        \tif (no_index)\n\t\t\twarning(\"Assuming '--no-index'; you are in no repository\");\n        \tusagef(\"git diff %s <path> <path>\",\n                \tno_index ? \"--no-index\" : \"[--no-index]\");\n\t}\n\nwithout changing anything else.\n\nThat would be just a patch with 1 line removed, 4 lines added, no?\n"},{"id":"168479","messageId":"BANLkTi=Uf8X+Bkd+CZ9qz0wXhHn8wwYzrA@mail.gmail.com","threadId":"27430","inReplyTo":"7vhb8mw1e6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Improve errors from 'git diff --no-index'.","fromName":"Anthony Foiani","fromEmail":"anthony.foiani@gmail.com","sentAt":"2011-05-23T04:05:36Z","receivedAt":"2011-05-23T04:05:36Z","isPatch":true,"sender":{"key":"anthony.foiani@gmail.com","avatar":"https://gravatar.com/avatar/dc2ca9a2ec133021e09ad6a322d4951c7355aefcf12889c5ac837eb16187c2b9?d=mp&s=160"},"body":"Junio --\n\nOn Sun, May 22, 2011 at 9:18 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> No, it was only your 'I tried to use \"git diff\"' that was confusing to me.\n\nApologies!\n\n> With pathspecs or any other options, it is understandable that you were\n> confused, for example from an output like this:\n>\n>> $ git diff -b main.c\n>> usage: git diff [--no-index] <path> <path>\n>\n> At that point we are committed to no-index codepath, so probably a good\n> thing to do is to update that confusing usage message a bit better.\n>\n> It already does change the behaviour when --no-index is not given by the\n> end user, so perhaps it would be sufficient to change this part:\n>\n>        if (argc != i + 2)\n>                usagef(\"git diff %s <path> <path>\",\n>                        no_index ? \"--no-index\" : \"[--no-index]\");\n>\n> to something like (caution: I am not typing this in my MUA):\n>\n>        if (argc != i + 2) {\n>                if (no_index)\n>                        warning(\"Assuming '--no-index'; you are in no repository\");\n>                usagef(\"git diff %s <path> <path>\",\n>                        no_index ? \"--no-index\" : \"[--no-index]\");\n>        }\n>\n> without changing anything else.\n>\n> That would be just a patch with 1 line removed, 4 lines added, no?\n\nThe \"no_index\" variable purely indicates whether or not \"--no-index\"\nwas given on the command line; for this particular warning, you want\n\"nongit\" instead.\n\nAlternately, you could simply do this above the \"if (argc...\" condition:\n\nif (!no_index)\n        warning(\"Assuming '--no-index'\");\n\nIt wouldn't explain *why*, but would at least provide some sort of a\nhint to the user that they're maybe not doing what they thought they\nwere doing.\n\nIf that's more to your taste, then I'd rather see this than nothing.  :)\n\nYou might also want to account for the fact that, even within a git\nrepo, if \"git diff\" is invoked on two paths that are both outside the\nrepo, then --no-index is forced. E.g.:\n\n$ cd linux-2.6-stable/\n$ git log -1 --pretty=short\ncommit 831d52bc153971b70e64eccfbed2b232394f22f8\nAuthor: Suresh Siddha <suresh.b.siddha@intel.com>\n\n    x86, mm: avoid possible bogus tlb entries by clearing prev\nmm_cpumask after switching mm\n\n$ ls -al /tmp/{foo,bar}\n-rw-rw-r--. 1 tony tony 0 May 22 10:09 /tmp/bar\n-rw-rw-r--. 1 tony tony 0 May 22 10:00 /tmp/foo\n\n$ # current git tip:\n$ git diff /tmp/{foo,bar}\n\n$ # with my patch:\n$ ../git/git-diff /tmp/{foo,bar}\nwarning: neither '/tmp/foo' nor '/tmp/bar' are tracked, forcing --no-index\n\nAlthough I apparently did break something...\n\n$ touch bar\n$ git diff bar /tmp/foo\n$ ../git/git-diff bar /tmp/foo\nfatal: '/tmp/foo' is outside repository\n\nGiven that I broke things, and that a +100/-25 patch is a bit\nexcessive, consider it retracted.\n\nThanks once again!\n\nBest regards,\nTony\n"},{"id":"168518","messageId":"7vpqn9usqt.fsf@alter.siamese.dyndns.org","threadId":"27430","inReplyTo":"BANLkTi=Uf8X+Bkd+CZ9qz0wXhHn8wwYzrA@mail.gmail.com","subject":"Re: [PATCH] Improve errors from 'git diff --no-index'.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-23T19:22:34Z","receivedAt":"2011-05-23T19:22:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anthony Foiani <anthony.foiani@gmail.com> writes:\n\n> $ ls -al /tmp/{foo,bar}\n> -rw-rw-r--. 1 tony tony 0 May 22 10:09 /tmp/bar\n> -rw-rw-r--. 1 tony tony 0 May 22 10:00 /tmp/foo\n>\n> $ # current git tip:\n> $ git diff /tmp/{foo,bar}\n>\n> $ # with my patch:\n> $ ../git/git-diff /tmp/{foo,bar}\n> warning: neither '/tmp/foo' nor '/tmp/bar' are tracked, forcing --no-index\n\nI actually consider this a regression. We are giving an output that the\nuser wanted to see, and I do not see a reason why we need to warn.\n\nA tangent.\n\nOne thing that I have always been unhappy about --no-index is that it does\nnot really mesh well with the notion of what a \"git diff\" is. \"git diff\"\ninherently is an operation to take two collections of contents labeled\nwith paths, and show series of patch output between corresponding paths in\nthese collections, while rename/copy detection may affect the definition\nof \"correspoinding paths\".\n\nA typical \"I know 'git diff' has a lot more features like color-words that\nmy platform 'diff' does not support, so let me use 'git diff' instead\"\nsession does something like:\n\n\t$ git diff [--no-index] /tmp/foo /tmp/bar\n\nbut such a request does not compare \"two collections of paths that have\ncorresponding paths\" at all. We could say we are comparing a collection\nthat has tmp/foo with another that has tmp/bar, but then we should either\nemit a delete patch for tmp/foo and a create patch for tmp/bar, or emit a\nrename patch to create tmp/bar out of tmp/foo if we want to be consistent.\n\nBut that consistency goes totally against what the users would expect.\nThis inconsistency is not a fault of either the definition of \"git diff\"\nnor the user's expectation. They are fundamentally different and the root\ncause of it is that we support --no-index diff between randomly chosen two\nfiles. I am not saying we should drop that feature, but it does not change\nthe fact that we had to add extra code in the output codepath only to\nsupport this \"outside git\" use case to suppress rename information (and\nprobably other things I do not remember).\n\nThe _only_ use of --no-index that is in line with what \"git diff\" does is\nto compare two directories as the \"two collections of contents\" above,\ni.e.\n\n\t$ git diff --no-index old/ new/\n\nand then support pathspecs, like this:\n\n\t$ git diff --no-index old/ new/ -- Makefile '*.c'\n\nBut I do not think the current implementation does not even support this\nonly sane usecase.\n"},{"id":"168529","messageId":"20110523205038.GB6281@sigill.intra.peff.net","threadId":"27430","inReplyTo":"7vpqn9usqt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Improve errors from 'git diff --no-index'.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-23T20:50:38Z","receivedAt":"2011-05-23T20:50:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 23, 2011 at 12:22:34PM -0700, Junio C Hamano wrote:\n\n> > $ # with my patch:\n> > $ ../git/git-diff /tmp/{foo,bar}\n> > warning: neither '/tmp/foo' nor '/tmp/bar' are tracked, forcing --no-index\n> \n> I actually consider this a regression. We are giving an output that the\n> user wanted to see, and I do not see a reason why we need to warn.\n\nAgreed.\n\n> One thing that I have always been unhappy about --no-index is that it does\n> not really mesh well with the notion of what a \"git diff\" is. \"git diff\"\n> inherently is an operation to take two collections of contents labeled\n> with paths, and show series of patch output between corresponding paths in\n> these collections, while rename/copy detection may affect the definition\n> of \"correspoinding paths\".\n\nIt's not just --no-index that breaks this abstraction. How about:\n\n  git diff HEAD:Makefile v1.7.5:Makefile\n\nor even\n\n  git diff :1:Makefile :2:Makefile\n\n?\n\nSo I think even without --no-index, we have the concept that diff\ncompares two \"things\", and that only some of those things are a\ncollection of paths (either a tree-ish, or possibly a directory, but as\nyou note below, I suspect the latter is broken).\n\nAnd obviously the two \"things\" must either both be single items, or must\nboth be collections, as any other comparison doesn't really make sense.\nWe do catch this, but it's another place where it might be nice to have\nbetter error messages:\n\n  $ git diff HEAD:Makefile HEAD\n  usage: git diff [<options>] [<commit> [<commit>]] [--] [<path>...]\n\n> \t$ git diff [--no-index] /tmp/foo /tmp/bar\n> \n> but such a request does not compare \"two collections of paths that have\n> corresponding paths\" at all. We could say we are comparing a collection\n> that has tmp/foo with another that has tmp/bar, but then we should either\n> emit a delete patch for tmp/foo and a create patch for tmp/bar, or emit a\n> rename patch to create tmp/bar out of tmp/foo if we want to be consistent.\n> \n> But that consistency goes totally against what the users would expect.\n\nRight. If I give git two single items to compare, rather than\ncollections, I am not going to expect renames at all, which\nfundamentally involve path movement. Those are a collection thing, and I\nasked for a much narrower scope.\n\n> The _only_ use of --no-index that is in line with what \"git diff\" does is\n> to compare two directories as the \"two collections of contents\" above,\n> i.e.\n> \n> \t$ git diff --no-index old/ new/\n> \n> and then support pathspecs, like this:\n> \n> \t$ git diff --no-index old/ new/ -- Makefile '*.c'\n> \n> But I do not think the current implementation does not even support this\n> only sane usecase.\n\nYeah, I would expect git to support that, but knowing the builtin/diff.c\ncode, I am not too surprised if it doesn't. :)\n\nOn the subject of things git diff should probably do but I doubt anybody\ncares enough to implement, it would be nice to be able to do:\n\n  git diff $merge^1 $merge^2 $merge\n\nand get the combined diff that \"git show $merge\" would give you, but\nwithout the log cruft.\n\nI also wonder if the reverse would be useful:\n\n  git diff $ours $theirs `git merge-base $ours $theirs`\n\nwhich would produce something like a reverse combined diff, or \"how have\nwe diverged\". E.g.,:\n\n  diff --combined file\n  index $base..$ours,$theirs\n  --- a/file\n  --- b/file\n  @@@ -1,1 +1,1 +1,1 @@@\n  --what was in the base\n  + what is in ours\n   +what is in theirs\n\nBut that is more than just a git-diff interface problem; we would\nactually have to write the reverse-combined diff code (I _think_ it\nshould be a pretty simple reversal of the regular combined diff, but I\nhaven't really thought it through).\n\n-Peff\n"},{"id":"168536","messageId":"BANLkTinmtJXo4jcnB1TAFPPtcGqun5rfoA@mail.gmail.com","threadId":"27430","inReplyTo":"7vpqn9usqt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Improve errors from 'git diff --no-index'.","fromName":"Anthony Foiani","fromEmail":"anthony.foiani@gmail.com","sentAt":"2011-05-23T21:24:50Z","receivedAt":"2011-05-23T21:24:50Z","isPatch":true,"sender":{"key":"anthony.foiani@gmail.com","avatar":"https://gravatar.com/avatar/dc2ca9a2ec133021e09ad6a322d4951c7355aefcf12889c5ac837eb16187c2b9?d=mp&s=160"},"body":"Junio --\n\nThis fundamentally all started as me confusing myself, and the patch\nwas the result of me scratching that itch.  As such, I'm perfectly\nwilling to drop it, but I did want to address a few last things:\n\nOn Mon, May 23, 2011 at 1:22 PM, Junio C Hamano <gitster@pobox.com> wrote:\n\n>> $ # with my patch:\n>> $ ../git/git-diff /tmp/{foo,bar}\n>> warning: neither '/tmp/foo' nor '/tmp/bar' are tracked, forcing --no-index\n>\n> I actually consider this a regression. We are giving an output that the\n> user wanted to see, and I do not see a reason why we need to warn.\n\nNo, we're not: the user did *not* ask for a no-index diff, but we're\ngiving them one anyway. That warrants a warn (or other message; if\nthere's a less-severe version of \"warn\", then that's fine) to tell\nthem that the software is trying to \"do what you mean\".\n\nPlease also keep in mind that you live and breathe git, and have done\nso for years (and I'm very thankful for that!).  But not everyone has\nthe git mindset all the time; adding an extra message or two doesn't\nseem like too much pain to help novices and dilettantes.\n\n> But that consistency goes totally against what the users would expect.\n> This inconsistency is not a fault of either the definition of \"git diff\"\n> nor the user's expectation. They are fundamentally different and the root\n> cause of it is that we support --no-index diff between randomly chosen two\n> files.\n\nMaybe we *should* drop the feature: I would have been *happier* had\nthe error message from my initial attempt been simply: \"not in a git\nrepo, can't use git diff\". (Since that was, fundamentally, the mistake\nI made.)\n\nMaybe install \"diff-git\" or even \"gdiff\" (although maybe that's used\nby gnu diff on non-gnu platforms, dunno).\n\nBut I maintain that the current combination of options, assumptions,\nand error output are insufficient, and I still feel that my patch(es)\nimprove that matter.  *shrug*  Your call, of course.\n\nBest regards,\nTony\n"}]}