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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 28, 2018, 17:42 UTC
Message-ID: <xmqqfu5lm2nw.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180228132116.GA32272@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

>> The binary patch is still supported, but that detail may need some more explanation
>> in the commit message. Please see  t4066-diff-encoding.sh
>
> Yeah, but if you don't have binary-patches enabled we'd generate a bogus
> patch. Which, granted, without that you wouldn't be able to apply the
> patch either. But somehow it feels funny to me to generate something
> that _looks_ like a patch but you can't actually apply.

True. And at least you _could_ apply a properly formatted binary patch to the original.

> I also think we'd want a plan for this to be used consistently in other
> diff-like tools. E.g., "git blame" uses textconv for the starting file
> content, and it would be nice for this to kick in then, too. Ditto for
> things like grep, pickaxe, etc.

You probably do not want to limit your thinking to the generation side. It is entirely plausible to have "we deal with diff in this encoding X" in addition to "the in-repo encoding for this project is this encoding Y" and "the working tree encoding for this path is Z" and allow them to interact in "git diff | git apply" pipeline.

"diff/format-patch --stdout/etc." on the upstream would first iconv Y to X and feed the contents in X to xdiff machinery, which is sent down the pipe and received by apply, which reads the preimage from the disk or from the repository. If doing "apply" without "--cached/--index", the preimage data from the disk would go through iconv Z to X. If doing "apply --cached/--index", the preimage data from the repo would go through iconv Y to X. The incoming patch is in X, so we apply, and the resulting postimage will be re-encoded in Z in the working tree and Y in the repository.