Re: [PATCH 1/2] diff: use HEAD for attributes when using bare repository

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 14, 2023, 20:37 UTC
Message-ID: <xmqqy1nzoyqr.fsf@gitster.g>
In-Reply-To: <20230314193839.wevenngf324pijhb@pop-os>

John Cai <johncai86@gmail.com> writes:

Show 7 quoted lines

>> Just for illustration, here is one way to do so.
>> 
>> The implementation goes in the opposite direction from the more
>> recent trend, which is why I am not making it an official patch, but
>
> Could you explain why this goes against the "more recent trend" for my
> understanding?

The illustration uses a global state.

The recent trend is to reduce reliance on global states and use the repository object and others that hold such state through the callchain.

But a new global variable that holds the fallback tree-ish object name was a so convenient way to illustrate the core of the idea, without having to change many callchains.

Show 10 quoted lines

>> with this you can do things like:
>> 
>>   $ git --attr-source=e83c5163 check-attr whitespace cache.h
>>   cache.h: whitespace: unspecified
>>   $ git --attr-source=e2f6331a142^ check-attr whitespace cache.h
>>   cache.h: whitespace: set
>>   $ git --attr-source=HEAD check-attr whitespace cache.h
>>   cache.h: whitespace: indent,trail,space
>
> I like the idea of an option that is global. For git-check-attr however, we

I guess I shouldn't have used check-attr to avoid confusion.

The point is that the internal mechanisms introduced by 47cfc9bd (attr: add flag `--source` to work with tree-ish, 2023-01-14), which taught check-attr the --source option and is reused by this illustration patch, was a good idea, but its UI was a mistake. We do not need per-command --source option the commit adds if we did the global option from day one. Yes, I think we can deprecate the "--source" option from there, if we all prefer the global option avenue. I _think_ git_all_attrs() needs to be told about the default attr-source trick (which I didn't touch in my illustration patch) before it happens, though.

If you used the mechanism in the illustration patch I gave you, and adjusted the test part of your patch to match (i.e. "diff" does not learn "--attr-source" option, but "git --attr-source=... diff" is how you make it read attributes from a tree-ish), would the result work well, or do we need more work to make it usable? How well do other commands (e.g. "git show") work in the same test repository you created in your version?

Thanks.