> On 27 Feb 2018, at 22:25, Jeff King <peff@xxxxxxxx> wrote: > > On Tue, Feb 27, 2018 at 10:05:17PM +0100, Torsten Bögershausen wrote: > > Of the three solutions, I think the relative merits are something like > this: > > 1. baked-in textconv (my patch) > > - reuses an existing diff feature, so minimal code and not likely to > break things > > - requires people to add a .gitattributes entry > > - needs work to make bare-repo .gitattributes work (though I think > this would be useful for other features, too) > > - has a run-time cost at each diff to do the conversion > > - may sometimes annoy people when it doesn't kick in (e.g., > emailed patches from format-patch won't have a readable diff) > > - doesn't combine with other custom-diff config (e.g., utf-16 > storing C code should still use diff=c funcname rules, but > wouldn't with my patch) > > 2. auto-detect utf-16 (your patch) > - Just Works for existing repositories storing utf-16 > > - carries some risk of kicking in when people would like it not to > (e.g., when they really do want a binary patch that can be > applied). > > I think it would probably be OK if this kicked in only when > ALLOW_TEXTCONV is set (the default for porcelain), and --binary > is not (i.e., when we would otherwise just say "binary > files differ"). > > - similar to (1), carries a run-time cost for each diff, and users > may sometimes still see binary diffs > > 3. w-t-e (Lars's patch) > > - requires no server-side modifications; the diff is plain vanilla > > - works everywhere you diff, plumbing and porcelain > > - does require people to add a .gitattributes entry > > - run-time cost is per-checkout, not per-diff > > So I can see room for (3) to co-exist alongside the others. Between (1) > and (2), I think (2) is probably the better direction. Thanks for the great summary! I agree they could co-exist and people could use whatever works best for them. I'll incorporate Eric's feedback and send a w-t-e v9 soonish. - Lars
