patchgit-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 <worley@ariadne.com>
---

This clarification is to avoid a problem I ran into.  I executed 'git
diff' in the remote working tree of a repository, and not in the
repository directory itself.  Because of that, git-diff assumed
git-diff --no-index, and executed diff-no-index.  Since I hadn't
provided paths, diff-no-index produced an error message.
Unfortunately, the error message presupposes that the decision to
execute diff-no-index reflects the user's intention, thus leaving me
confused, as the error message is only:
    usage: git diff [--no-index] <path> <path>
and does not cover the case I intended.  This patch changes the
message to notify the user that he is getting --no-index semantics
because he is outside of a repository:
    Not within a git repository:
    usage: git diff [--no-index] <path> <path>
The additional line is suppressed if the user specified --no-index.

The documentation is expanded to state that execution outside of a repository forces --no-index behavior. Previously, the manual implied this but did not state it, making it easy for the user to overlook that it's possible to run git-diff outside of a repository.

Dale

 Documentation/git-diff.txt |    3 ++-
 diff-no-index.c            |    6 +++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

worley@alum.mit.edu (Dale R. Worley) writes:

It makes perfect sense for your situation, I think.

Do we say "within" in other error messages for similar situations? Many commands require you to be in a working tree---the ones marked as NEED_WORK_TREE in git.c call setup.c::setup_work_tree() to do this check---and the error message phrases "run in a work tree". We would want to use the matching phrasing here, too.

For that matter, as no_index variable knows we didn't get an explicit "--no-index", we can be even more explicit, e.g.

	fatal: If you want to compare two files outside a working
        tree, use "git diff <fileA> <fileB>".

which hopefully will also clarify the consequence of the command, i.e. compares two files that are _outside_ a working tree.

I am not sure which one is better, though. Just a random thought that came to my mind while reading your error message.

I am not sure if "forced" is a good description here. An explicit "--no-index" does force the command to ignore the fact that the command is run inside a working tree and compare two paths without involving Git at all, but the behaviour you saw was to fall back to the no-index hack instead of failing (the latter of which is a logical but unfriendly thing to do, as Git is about data managed by Git, and running Git command that wants a working tree without having a working tree). It feels that it is more like "Also, this mode is used when the command is run outside a working tree" to me.

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 <worley@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(-)

worley@alum.mit.edu (Dale R. Worley) writes:

The above result looks good and I find the reasoning stated here very sound.

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.

Thanks.

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

worley@alum.mit.edu (Dale R. Worley) writes:

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(-)

That does break out the --no-index case in a clearer way.

Dale