Hi Junio, On 14 Mar 2023, at 16:37, Junio C Hamano wrote: > John Cai <johncai86@xxxxxxxxx> 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? > > 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. > > 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
