Re: [PATCHv2] git-diff: Clarify operation when not inside a repository.
From: Junio C Hamano gits...@pobox.com diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt index b1630ba..33fbd8c 100644 --- a/Documentation/git-diff.txt +++ b/Documentation/git-diff.txt @@ -28,11 +28,15 @@ two blob objects, or changes between two files on disk. words, the differences are what you _could_ tell Git to further add to the index but you still haven't. You can stage these changes by using linkgit:git-add[1]. -+ -If exactly two paths are given and at least one points outside -the current repository, 'git diff' will compare the two files / -directories. This behavior can be forced by --no-index or by -executing 'git diff' outside of a working tree. + +'git diff' --no-index [--options] [--] [path...]:: + + This form is to compare the given two paths on the + filesystem. You can omit the `--no-index` option when + running the command in a working tree controlled by Git and + at least one of the paths points outside the working tree, + or when running the command outside a working tree + controlled by Git. That does break out the --no-index case in a clearer way. Dale -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] git-diff: Clarify operation when not inside a repository.
wor...@alum.mit.edu (Dale R. Worley) writes: For now, I'll queue your version as-is modulo style fixes, while waiting for others to help polishing the documentation better. It'd difficult to figure out how to describe it well. In my opinion, the problem here is the DWIM nature of the command, ... My preference is ... But that's not how git-diff works. So given the constraints, I think this is the best we can do. As nobody seems to be helping to polish the text, here is my attempt, on top of your patch. Documentation/git-diff.txt | 14 +- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt index b1630ba..33fbd8c 100644 --- a/Documentation/git-diff.txt +++ b/Documentation/git-diff.txt @@ -28,11 +28,15 @@ two blob objects, or changes between two files on disk. words, the differences are what you _could_ tell Git to further add to the index but you still haven't. You can stage these changes by using linkgit:git-add[1]. -+ -If exactly two paths are given and at least one points outside -the current repository, 'git diff' will compare the two files / -directories. This behavior can be forced by --no-index or by -executing 'git diff' outside of a working tree. + +'git diff' --no-index [--options] [--] [path...]:: + + This form is to compare the given two paths on the + filesystem. You can omit the `--no-index` option when + running the command in a working tree controlled by Git and + at least one of the paths points outside the working tree, + or when running the command outside a working tree + controlled by Git. 'git diff' [--options] --cached [commit] [--] [path...]:: -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] git-diff: Clarify operation when not inside a repository.
From: Junio C Hamano gits...@pobox.com I suspect that it may be a good idea to split the section altogether to reduce confusion like what triggered this thread, e.g. 'git diff' [--options] [--] [path...]:: This form is to view the changes you made relative to the index (staging area for the next commit). In other words, the differences are what you _could_ tell Git to further add to the index but you still haven't. You can stage these changes by using linkgit:git-add[1]. 'git diff' --no-index [--options] [--] path path:: This form is to compare the given two paths on the filesystem. When run in a working tree controlled by Git, if at least one of the paths points outside the working tree, or when run outside a working tree controlled by Git, you can omit the `--no-index` option. For now, I'll queue your version as-is modulo style fixes, while waiting for others to help polishing the documentation better. It'd difficult to figure out how to describe it well. In my opinion, the problem here is the DWIM nature of the command, which means that there is a lot of interaction between the options that are specified, the number of path arguments, and the circumstances. My preference is for do what I say, that the options restrict the command to operate in exactly one way, which determines the way the paths are used (and thus their number) and the context in which it can be used. But that's not how git-diff works. Dale -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] git-diff: Clarify operation when not inside a repository.
wor...@alum.mit.edu (Dale R. Worley) writes: The error message has been updated from [PATCH]. git diff outside a repository now produces: Not a git repository To compare two paths outside a working tree: usage: git diff [--no-index] path path This should inform the user of his error regardless of whether he intended to perform a within-repository git diff or an out-of-repository git diff. This message is closer to the message that other Git commands produce: fatal: Not a git repository (or any parent up to mount parent ) Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set). git diff --no-index produces the same message as before (since the user is clearly invoking the non-repository behavior): usage: git diff --no-index path path The above result looks good and I find the reasoning stated here very sound. Regarding the change to git-diff.txt, perhaps forced ... by executing 'git diff' outside of a working tree is not the best wording, but it should be clear to the reader that (1) it is possible to execute 'git diff' outside of a working tree, and (2) when doing so, the behavior will be as if '--no-index' was specified. Then perhaps we can avoid the confusing forced by phrasing it like so? This behaviour can be forced by --no-index. Also 'git diff path path' outside of a working tree can be used to compare two named paths. Let's step back a bit, though. The original text is: 'git diff' [--options] [--] [path...]:: This form is to view the changes you made relative to the index (staging area for the next commit). In other words, the differences are what you _could_ tell Git to further add to the index but you still haven't. You can stage these changes by using linkgit:git-add[1]. + If exactly two paths are given and at least one points outside the current repository, 'git diff' will compare the two files / directories. This behavior can be forced by --no-index. which _primarily_ explains how the index and the working tree contents are compared, but also mixes the description of the --no-index hack, which is quite different. As its name suggests, it is not about comparing with the index---in fact, it is not even about Git at all. Just a pair of random paths that do not have anything to do with Git are compared. I suspect that it may be a good idea to split the section altogether to reduce confusion like what triggered this thread, e.g. 'git diff' [--options] [--] [path...]:: This form is to view the changes you made relative to the index (staging area for the next commit). In other words, the differences are what you _could_ tell Git to further add to the index but you still haven't. You can stage these changes by using linkgit:git-add[1]. 'git diff' --no-index [--options] [--] path path:: This form is to compare the given two paths on the filesystem. When run in a working tree controlled by Git, if at least one of the paths points outside the working tree, or when run outside a working tree controlled by Git, you can omit the `--no-index` option. For now, I'll queue your version as-is modulo style fixes, while waiting for others to help polishing the documentation better. I've also added some comments for the new code. Thanks. Documentation/git-diff.txt |3 ++- diff-no-index.c| 12 +++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt index 78d6d50..9f74989 100644 --- a/Documentation/git-diff.txt +++ b/Documentation/git-diff.txt @@ -31,7 +31,8 @@ two blob objects, or changes between two files on disk. + If exactly two paths are given and at least one points outside the current repository, 'git diff' will compare the two files / -directories. This behavior can be forced by --no-index. +directories. This behavior can be forced by --no-index or by +executing 'git diff' outside of a working tree. 'git diff' [--options] --cached [commit] [--] [path...]:: diff --git a/diff-no-index.c b/diff-no-index.c index e66fdf3..9734ec3 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -215,9 +215,19 @@ void diff_no_index(struct rev_info *revs, path_inside_repo(prefix, argv[i+1]))) return; } - if (argc != i + 2) + if (argc != i + 2) { + if (!no_index) { + /* There was no --no-index and there were not two + * paths. It is possible that the user intended + * to do an inside-repository operation. */ + fprintf(stderr, Not a git repository\n); + fprintf(stderr, + To compare two