worley@xxxxxxxxxxxx (Dale R. Worley) writes:
> 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.
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.
> 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.
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.
> Documentation/git-diff.txt | 3 ++-
> diff-no-index.c | 6 +++++-
> 2 files changed, 7 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..98c5f76 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -215,9 +215,13 @@ 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) {
> + fprintf(stderr, "Not within a git repository:\n");
> + }
> usagef("git diff %s <path> <path>",
> no_index ? "--no-index" : "[--no-index]");
> + }
>
> diff_setup(&revs->diffopt);
> for (i = 1; i < argc - 2; ) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html