Subject: bug#33567: Syntactic fontification of diff hunks

From: Juri Linkov <[email-masked]>
Date: Thu, 06 Dec 2018 01:25:46 +0200

>> vc-git-find-revision binds coding-system-for-read to `binary'.
>
> I see that vc-hg-find-revision does the same.  Sigh.  I guess the
> find-revision API was never meant to process the resulting buffer
> normally.  My advice would be to reimplement your
> vc-find-revision-no-save function differently, without trying to
> piggy-back the fact that vc-find-revision inserts the contents into a
> buffer.  That is, let the code write the contents to a temporary file,
> like vc-find-revision does, then call insert-file-contents to re-read
> that file normally.  It would be slightly less efficient, but I think
> the result will be much simpler, so a net win.

The whole purpose of creating vc-find-revision-no-save function was
to improve the performance of vc-find-revision-save to avoid the need
to write files.  It would significantly degrade performance of
diff syntax fontification if it will write files for every hunk.

> If you still want to reuse the literal contents of the file, as
> inserted by vc-git-find-revision etc., then you will have to duplicate
> what insert-file-contents does internally.  I suggest to look at how
> this is done in archive-set-buffer-as-visiting-file.

I see it does something like I was trying to do.  I will use it
when failing to use the third possible solution I proposed below.

>> > How do you know vc-git-find-revision doesn't have a subtle bug as
>> > well, e.g. when file names in the repository are encoded in some
>> > non-trivial, non-UTF-8 encoding?
>>
>> This is why vc-git-find-revision does nothing with its output
>> when it binds coding-system-for-read to `binary',
>> and doesn't try to encode/decode the git output.
>
> vc-git-find-revision does _something_ with Git's output: it uses the
> file name returned by Git.  That file name could have a non-trivial
> encoding.

I'm thinking about overriding coding in vc-git-find-revision like

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index f317400530..e5f44524df 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -838,8 +838,8 @@ vc-git-checkin
 
 (defun vc-git-find-revision (file rev buffer)
   (let* (process-file-side-effects
-	 (coding-system-for-read 'binary)
-	 (coding-system-for-write 'binary)
+	 (coding-system-for-read (or coding-system-for-read 'binary))
+	 (coding-system-for-write (or coding-system-for-write 'binary))
 	 (fullname
 	  (let ((fn (vc-git--run-command-string
 		     file "ls-files" "-z" "--full-name" "--")))

then a caller function could set its own value of this dynamic binding.
But I haven't tested yet if it works with UTF-8 file names.