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 thanks John
