Re: [PATCH] diff: fix up SHA-1 abbreviations outside of repository

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 5, 2016, 22:43 UTC
Message-ID: <xmqqmvga2dzx.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20161205072614.zg6yglqnznna65vf@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> I agree that it may be an accident waiting to happen, though, as soon as
> some buried sub-function needs to care about the distinction.

Yes to that.

Show 7 quoted lines

>> I wonder if we're better off if we made sure that diff_no_index()
>> works the same way regardless of the value of "have_repository"
>> field?
>
> If you mean adding a diffopt flag like "just abbreviate everything to
> FALLBACK_DEFAULT_ABBREV even if we're in a repository", and then setting
> that in diff_no_index(), I agree that is a lot cleaner.

I am not sure if that is what I meant (I no longer sure what exactly I meant to say there TBH), but this is probably not limited to the default abbrev length aka core.abbrev configuration. Don't we have other configuration settings we may read from $HOME/.gitconfig (and possibly per-repository .git/config, if we did discovery but were explicitly given "--no-index") that want to affect the behaviour of the command?

I guess what I wanted, with "the same way", to see happen was that "have_repository" should be only controling how and from what files the configuration is read, and the behaviour of the command should be controlled by the values read from the configuration after that.

Specifically, even if we were running with "--no-index", if we know we have access to the current repository discovered by setting it up gently, I do not think it is bad to ask find_unique_abbrev() to come up with an appropriate abbreviation. So the fact that patch in question has to flip the have_repository bit off, if it is done in order to affect what diff_abbrev_oid() does, smells quite fishy from that point of view.