bug#14516: 24.3.50; vc-diff ignores the encoding of its input

bug#14516: 24.3.50; vc-diff ignores the encoding of its input

[Dima Kogan]

[-- Attachment #1: Type: text/plain, Size: 1442 bytes --]

I'm using a very recent build of the emacs source repo on a Linux box.

I have a file that uses DOS-style line termination. This file is in git
and has some modifications. I use (vc-diff) to get a buffer that shows
those modifications. Currently this buffer is NOT set to use the DOS
encoding, and emacs does a conversion, so this buffer is missing the
extra CR characters the DOS encoding had. Because of this, if I save
this buffer to a file, external tools can no longer be used to apply the
patch.

I'm attaching a tiny git repository that demonstrates the issue:

 dima@shorty:/tmp$ tar xvfz vc_encoding_error_test_repo.tar.gz
 vc_encoding_error_test_repo/
 vc_encoding_error_test_repo/file
 <snip>

 dima@shorty:/tmp$ cd vc_encoding_error_test_repo

 dima@shorty:/tmp/vc_encoding_error_test_repo$ emacs --batch --eval '(progn (find-file "file") (vc-diff) (with-current-buffer "*vc-diff*" (write-file "/tmp/patch")))' -Q
 Finding changes in /tmp/vc_encoding_error_test_repo/file...
 Saving file /tmp/patch...
 Wrote /tmp/patch

 dima@shorty:/tmp/vc_encoding_error_test_repo$ git apply --cached /tmp/patch
 error: patch failed: file:1
 error: file: patch does not apply



Here we see git fail to apply the saved patch. This is due to the
stripped CR characters.

VC already has code in it to detect the encoding of its input. I'm
attaching a small patch that sets this detected encoding to the
*vc-diff* buffer. This fixes the issue.




[-- Attachment #2: vc_encoding_error_test_repo.tar.gz --]
[-- Type: application/octet-stream, Size: 6612 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0001-vc-buffers-now-use-the-encoding-of-their-input.patch --]
[-- Type: text/x-diff, Size: 860 bytes --]

From fd04b0d03f61344a4c9a37cafc86495c79a46d19 Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Thu, 30 May 2013 14:17:08 -0700
Subject: [PATCH] vc buffers now use the encoding of their input

---
 lisp/vc/vc-dispatcher.el |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index 309cf50..1bada72 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -178,7 +178,9 @@ Another is that undo information is not kept."
     (setq default-directory olddir)
     (let ((buffer-undo-list t)
           (inhibit-read-only t))
-      (erase-buffer))))
+      (erase-buffer))
+    (when coding-system-for-read
+      (setq buffer-file-coding-system coding-system-for-read))))
 
 (defvar vc-sentinel-movepoint)          ;Dynamically scoped.
 
-- 
1.7.10.4

bug#14516: 24.3.50; vc-diff ignores the encoding of its input

[Richard Stallman]

    I'm using a very recent build of the emacs source repo on a Linux box.

Does it run Linux alone, or does it run GNU and Linux?

-- 
Dr Richard Stallman
President, Free Software Foundation
51 Franklin St
Boston MA 02110
USA
www.fsf.org  www.gnu.org
Skype: No way! That's nonfree (freedom-denying) software.
  Use Ekiga or an ordinary phone call

bug#14516: 24.3.50; vc-diff ignores the encoding of its input

[Dmitry Gutov]

Dima Kogan <dima@secretsauce.net> writes:

> VC already has code in it to detect the encoding of its input. I'm
> attaching a small patch that sets this detected encoding to the
> *vc-diff* buffer. This fixes the issue.

I'd prefer a slightly different patch.  This way, you have smaller
probability of conflict when `vc-setup-buffer' is called from any
fuction other than `vc-diff-internal'.

And `coding-system-for-read' detection is only peformed for the diff
commands.

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el       2013-11-13 20:06:37 +0000
+++ lisp/vc/vc.el       2013-11-25 01:13:04 +0000
@@ -1642,6 +1642,8 @@
         (coding-system-for-read
          (if files (vc-coding-system-for-diff (car files)) 'undecided)))
     (vc-setup-buffer buffer)
+    (when coding-system-for-read
+      (setq buffer-file-coding-system coding-system-for-read))
     (message "%s" (car messages))
     ;; Many backends don't handle well the case of a file that has been
     ;; added but not yet committed to the repo (notably CVS and Subversion).

bug#14516: 24.3.50; vc-diff ignores the encoding of its input

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Dima Kogan <dima@secretsauce.net> writes:
>
>> VC already has code in it to detect the encoding of its input. I'm
>> attaching a small patch that sets this detected encoding to the
>> *vc-diff* buffer. This fixes the issue.
>
> I'd prefer a slightly different patch.  This way, you have smaller
> probability of conflict when `vc-setup-buffer' is called from any
> fuction other than `vc-diff-internal'.
>
> And `coding-system-for-read' detection is only peformed for the diff
> commands.
>
> === modified file 'lisp/vc/vc.el'
> --- lisp/vc/vc.el       2013-11-13 20:06:37 +0000
> +++ lisp/vc/vc.el       2013-11-25 01:13:04 +0000
> @@ -1642,6 +1642,8 @@
>          (coding-system-for-read
>           (if files (vc-coding-system-for-diff (car files)) 'undecided)))
>      (vc-setup-buffer buffer)
> +    (when coding-system-for-read
> +      (setq buffer-file-coding-system coding-system-for-read))
>      (message "%s" (car messages))
>      ;; Many backends don't handle well the case of a file that has been
>      ;; added but not yet committed to the repo (notably CVS and Subversion).

It looks like this was added instead:

    ;; On MS-Windows and MS-DOS, Diff is likely to produce DOS-style
    ;; EOLs, which will look ugly if (car files) happens to have Unix
    ;; EOLs.
    (if (memq system-type '(windows-nt ms-dos))
	(setq coding-system-for-read
	      (coding-system-change-eol-conversion coding-system-for-read
						   'dos)))

I guess that makes as much sense as anything.  (I mean, there may be
different coding systems in each file being diffed in the same
buffer...)

So I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no