git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
John Cai <johncai86@gmail.com>
Date
Mar 16, 2023, 16:46 UTC
Message-ID
<9E479AA2-2D54-4CAF-899B-B25894B746AA@gmail.com>
In-Reply-To
<xmqqy1nzoyqr.fsf@gitster.g>
Hi Junio,
On 14 Mar 2023, at 16:37, Junio C Hamano wrote:
Show 15 quoted lines
> John Cai <johncai86@gmail.com> writes:
>
>>> 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.

Ah I see--in that case, what would be a good object to put this state into? Mabe repo_settings?

Show 28 quoted lines
>
> 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.
>
>>> 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.
Good point. I think to have a global flag would be a better user experience.
Show 8 quoted lines
>
> 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?

I think it works pretty smoothly. I adjusted my tests to work with this git flag, and I also tested git show and it works well.

In fact, your proposal would serve the needs of my patch independently of any of the diff code, which is nice.

>
> Thanks.

Thanks, John

Previous: Junio C HamanoNext: Junio C Hamano
Message 8 of 15 in “diff: support bare repositories when reading gitattributes for diff algorithm”
  1. 0/2 diff: support bare repositories when reading gitattributes for diff algorithmJohn Cai via GitGitGadget, Mar 14, 2023
  2. 2/2 diff: add --attr-source to read gitattributes from a commitJohn Cai via GitGitGadget, Mar 14, 2023
  3. 1/2 diff: use HEAD for attributes when using bare repositoryJohn Cai via GitGitGadget, Mar 14, 2023
  4. Junio C HamanoMar 14, 2023
  5. Junio C HamanoMar 14, 2023
  6. John CaiMar 14, 2023
  7. Junio C HamanoMar 14, 2023
  8. John CaiMar 16, 2023
  9. Junio C HamanoMar 16, 2023
  10. John CaiMar 17, 2023
  11. Philippe BlainMar 14, 2023
  12. John CaiMar 14, 2023
  13. Junio C HamanoMar 14, 2023
  14. Jeff KingMar 16, 2023
  15. Junio C HamanoMar 16, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.