On Tue, Feb 27, 2018 at 10:05:17PM +0100, Torsten Bögershausen wrote:
> The other question is:
> Would this help showing diffs of UTF-16 encoded files on a "git hoster",
> github/bitbucket/.... ?
Almost. There's probably one more thing needed. We don't currently read
in-tree .gitattributes when doing a diff in a bare repository. And most
hosting sites will store bare repositories.
And of course it would require the users to actually set the attributes
themselves.
> Or would the auto-magic UTF-16 avoid binary patch that I send out be more
> helpful ?
> Or both ?
> Or the w-t-e encoding ?
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.
-Peff
