Re: [bug?] git diff --cached in a non-git directory

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 9, 2024, 20:26 UTC
Message-ID: <xmqqikv4po8v.fsf@gitster.g>
In-Reply-To: <xmqqy140pt67.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 20 quoted lines

> <rsbecker@nexbridge.com> writes:
>
>>>error: unknown option `cached'
>>>usage: git diff --no-index [<options>] <path> <path> [...]
>> ...
>> Because you are not in a git repository clone, the --cached option has no
>> meaning.
>
> Yes.  "git diff" unfortunately has a mode where a limited subset of
> its features is available and that is called the "--no-index" mode.
>
> Because the command is capable of working outside a repository, "You
> are not in a repository" is a not a good error message in this
> situation, either.  It is not that you stepped outside a repository
> that is your error.  It is not that you used "--cached" that is your
> error, either.  You used "--cached" outside a repository, that is
> where your error is.
>
> A patch to improve the error messages in such a situation is
> certainly welcome.

Having said that, there is already a warning() close by.

	argc = parse_options(argc, argv, revs->prefix, options,
			     diff_no_index_usage, 0);
	if (argc != 2) {
		if (implicit_no_index)
			warning(_("Not a git repository. Use --no-index to "
				  "compare two paths outside a working tree"));
		usage_with_options(diff_no_index_usage, options);
	}

and this gives

$ cd / && git diff foo 2>&1 | grep -e error: -e fatal: -e warning: warning: Not a git repository. Use --no-index to compare two paths outside a working tree

a relevant warning.

But this is a bit too short to be effective for all failure modes. parse_options() would die if it sees an invalid option. For that, I suspect that we'd need to pass the usage string conditionally, i.e.

    argc = parse_options(argc, argv, revs->prefix, options,
			 implicit_no_index 
			 ? diff_no_index_usage_with_non_repo_warning
			 : diff_no_index_usage, 0);

with

    static const char * const diff_no_index_usage_with_non_repo_warning[] = {
            N_("git diff --no-index [<options>] <path> <path>"),
            N_("Not in a git repository; some options may not make sense"),
            NULL,
    };

near where diff_no_index_usage[] is defined, or something silly like that.