git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: <rsbecker@nexbridge.com>
Cc: "'Okke Timm'" <okke@timm.sh>,  <git@vger.kernel.org>
Subject: Re: [bug?] git diff --cached in a non-git directory
Date: Mon, 09 Sep 2024 13:26:40 -0700	[thread overview]
Message-ID: <xmqqikv4po8v.fsf@gitster.g> (raw)
In-Reply-To: <xmqqy140pt67.fsf@gitster.g> (Junio C. Hamano's message of "Mon, 09 Sep 2024 11:40:16 -0700")

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

> <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.


  parent reply	other threads:[~2024-09-09 20:26 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-09 14:26 [bug?] git diff --cached in a non-git directory Okke Timm
2024-09-09 15:37 ` rsbecker
2024-09-09 15:51   ` Okke Timm
2024-09-09 18:40   ` Junio C Hamano
2024-09-09 19:28     ` rsbecker
2024-09-09 20:26     ` Junio C Hamano [this message]
  -- strict thread matches above, loose matches on Subject: below --
2024-09-09 14:48 Okke Timm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqikv4po8v.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=okke@timm.sh \
    --cc=rsbecker@nexbridge.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).