Hi Elijah, On 20 Feb 2023, at 12:32, Elijah Newren wrote: > On Mon, Feb 20, 2023 at 8:49 AM John Cai <johncai86@xxxxxxxxx> wrote: >> >> Hi Elijah, >> >> On 20 Feb 2023, at 11:21, Elijah Newren wrote: >> >>> On Mon, Feb 20, 2023 at 7:32 AM John Cai <johncai86@xxxxxxxxx> wrote: >>> [...] >>>>> I'm still curious if this should this also include warnings/caveats, such as: >>>>> * The diff attribute specified in .gitattributes will be ignored in >>>>> a bare clone >>>>> * The diff attribute specified in .gitattributes will be ignored if >>>>> it is only specified in another branch (e.g. on a branch "special-file >>>>> diff=patience" recorded in .gitattributes, then checkout master but >>>>> run `git log -1 -p $branch`) >>>>> * When a file is renamed, the diff attribute for the pre-image name >>>>> is the only one the system pays attention to (thus adding "-R" can >>>>> flip which diff algorithm is run for the renamed file). >>>> >>>> I would be fine with adding that--though originally I was thinking that these >>>> can be inferred from the way that gitattributes are documented in [1]. Calling >>>> these out would make it more clear though, so I could go either way. >>>> >>>>> >>>>> Also, since I tested the three items above to verify they are valid >>>>> warnings, I'm a bit confused. I thought your intent was to use this >>>>> server-side[1], so isn't the bare clone aspect a deal-breaker for your >>>>> intended usecase? >>>>> >>>>> [1] https://lore.kernel.org/git/7852AC7B-7A4E-4DD0-ADEA-CFFD5D16C595@xxxxxxxxx/ >>>> >>>> yes, indeed. I was planning on adding bare repository support in a separate >>>> patch series, since the additions in [2] allows .gitattributes to be read from a >>>> bare repository. >>>> >>>> 1. https://git-scm.com/docs/gitattributes >>>> 2. https://lore.kernel.org/git/0ca8b2458921fc40269b0c43b5ec86eba77d6b54.1673684790.git.karthik.188@xxxxxxxxx/ >>>> >>>> thanks! >>>> John >>> >>> Oh, interesting, I didn't know about [2]. So, is the plan to take the >>> --source option from that series and add it to diff (perhaps with a >>> different name, since log tends to consume diff options and --source >>> is already taken)? >> >> Yep, that would be the general idea >> >>> >>> And do you expect to get the tree-ish from the two the users are >>> already specifying to diff? If so, which one do you use (the two >>> commits being diffed might have differing .gitattributes files)? If >>> not, what does that mean for users of e.g. the GitLab UI who have to >>> specify a third tree when diffing? >> >> Good question! Since it seems that when `git-diff(1)` considers diff.<driver>, >> it goes with the path of the first one. (might need some confirmation here) >> >> in diff.c: >> >> >> static void run_diff(struct diff_filepair *p, struct diff_options *o) >> { >> const char *pgm = external_diff(); >> struct strbuf msg; >> struct diff_filespec *one = p->one; >> struct diff_filespec *two = p->two; >> const char *name; >> const char *other; >> const char *attr_path; >> >> name = one->path; >> other = (strcmp(name, two->path) ? two->path : NULL); >> attr_path = name; >> if (o->prefix_length) >> >> I was thinking we would just use the tree-ish of the first one > > That would certainly simplify, but it'd be pretty important to > document. (Incidentally, this kind of decision was my reason for > asking about all those special cases earlier, i.e. how to handle diff > between different commits, how to handle renames, how to handle bare > repositories, etc.) Good point--that would be good to document. > > This kind of decision probably also means you'd need a variety of > testcases where .gitattributes is different in every commit & the > index & the working tree, and then you start testing several of the > possible pairings to make sure the right .gitattributes file is used > (e.g. (commit, commit), (commit, index), (index, commit), (worktree, > index), etc.) > > However, I'm curious again. You brought this up because you want to > use it in GitLab, yet configuration of using this option as it appears > in this series requires changing _both_ .gitattributes and > .git/config. How will users of the GitLab UI do the configuration > necessary for the git-config side to take effect? Good question--we would likely need to add some pre-baked configuration server side with a driver config that users could then tap into with their gitattributes files.
