Thread (11 messages) 11 messages, 4 authors, 2018-03-04

Re: [PATCH v7 0/7] convert: add support for different encodings

flat view

From: Lars Schneider <hidden>
Date: 2018-02-28 20:46:51

Possibly related (same subject, not in this thread)

On 27 Feb 2018, at 22:25, Jeff King [off-list ref] 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

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help