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
[PATCHv2] git-diff: Clarify operation when not inside a repository.
Clarify documentation for git-diff: State that when not inside a
repository, --no-index is implied (and thus two arguments are
mandatory).
Clarify error message from diff-no-index to inform user that CWD is
not inside a repository and thus two arguments are mandatory.
Signed-off-by: Dale Worley wor...@ariadne.com
---
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
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.
I've also added some comments for the new code.
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 paths outside a working
tree:\n);
+ }
+ /* Give the usage message for non-repository usage and exit. */
usagef(git diff %s path path,
no_index ? --no-index : [--no-index]);
+ }
diff_setup(revs-diffopt);
for (i = 1; i argc - 2; ) {
--
1.7.7.6
--
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
