bug#34720: 26.1; Reverting a GPG buffer moves all markers to the end of the file

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> 1. Open a GnuPG encrypted file
>> 2. Run the following:
>>    (setq test-marker (make-marker))
>>    (move-marker test-marker (point-marker))
>>    ;; test-marker points to `point'
>> 3. Revert the buffer
>>    ;; test-marker now points to the end of the buffer

I took a stab at this with the patch below (which is the wrong solution
because it doesn't clear the undo list etc).

But I don't quite understand why this is so much work.  :-)

The problem is twofold: The first is in epa.el itself, which deletes and
then inserts, which destroys the markers.

The second is the call to `decode-coding-region' in
`decode-coding-inserted-region', which also destroys the markers.

The third is that the interface to the new replace-buffer-contents
function is really awkward -- it only takes a buffer as its SOURCE,
which means that if you want to feed it something, you have to skip
around to temporary buffers instead of feeding it a string, which would
be natural.  You have to be a with-temp-buffer/with-current-buffer/let
contortionist to get it to be safe.

And replace-buffer-contents does so much more than just restoring
markers, anyway, so I'm not sure it's the right solution here, anyway.

Would anybody mind if I just write a `with-saved-markers' macro in
subr-x, which would make all these problems to away and make the
solution a two-liner in epa itself?

(Unless such a macro exists somewhere already.)

diff --git a/lisp/epa-file.el b/lisp/epa-file.el
index d9886d3d67..03f53c40a9 100644
--- a/lisp/epa-file.el
+++ b/lisp/epa-file.el
@@ -24,6 +24,7 @@
 
 (require 'epa)
 (require 'epa-hook)
+(require 'mule)
 
 (defcustom epa-file-cache-passphrase-for-symmetric-encryption nil
   "If non-nil, cache passphrase for symmetric encryption.
@@ -102,16 +103,21 @@ epa-file-run-real-handler
     (apply operation args)))
 
 (defun epa-file-decode-and-insert (string file visit beg end replace)
-  (if (fboundp 'decode-coding-inserted-region)
-      (save-restriction
-       (narrow-to-region (point) (point))
-       (insert string)
-       (decode-coding-inserted-region
-        (point-min) (point-max)
-        (substring file 0 (string-match epa-file-name-regexp file))
-        visit beg end replace))
-    (insert (epa-file--decode-coding-string string (or coding-system-for-read
-                                                      'undecided)))))
+  (let ((buffer (current-buffer)))
+    (with-temp-buffer
+      (insert string)
+      (let ((coding (decode-coding-inserted-region-coding-system
+                     (substring
+                      file 0 (string-match epa-file-name-regexp file))
+                     visit beg end replace)))
+        (if coding
+           (decode-coding-region (point-min) (point-max) coding)
+         (setq last-coding-system-used coding))
+        (let ((temp (current-buffer)))
+          (with-current-buffer buffer
+            (if replace
+                (replace-buffer-contents temp)
+              (insert-buffer-substring temp))))))))
 
 (defvar epa-file-error nil)
 (defun epa-file--find-file-not-found-function ()
@@ -147,8 +153,6 @@ epa-file-insert-file-contents
           (format "Decrypting %s" file)))
     (unwind-protect
        (progn
-         (if replace
-             (goto-char (point-min)))
          (condition-case error
              (setq string (epg-decrypt-file context local-file nil))
            (error
@@ -187,12 +191,9 @@ epa-file-insert-file-contents
            ;; really edit the buffer.
            (let ((buffer-file-name
                   (if visit nil buffer-file-name)))
-             (save-restriction
-               (narrow-to-region (point) (point))
-               (epa-file-decode-and-insert string file visit beg end replace)
-               (setq length (- (point-max) (point-min))))
-             (if replace
-                 (delete-region (point) (point-max))))
+             (setq length
+                    (epa-file-decode-and-insert
+                     string file visit beg end replace)))
            (if visit
                (set-visited-file-modtime))))
       (if (and local-copy
diff --git a/lisp/international/mule.el b/lisp/international/mule.el
index ec6f647688..5338e54cf6 100644
--- a/lisp/international/mule.el
+++ b/lisp/international/mule.el
@@ -2244,6 +2244,25 @@ modify-coding-system-alist
                   (cons (cons regexp coding-system)
                         network-coding-system-alist)))))))
 
+(defun decode-coding-inserted-region-coding-system (filename
+                                                    visit beg end replace)
+  "Return a coding system for the current buffer as if it is read from 
FILENAME."
+  (let ((coding coding-system-for-read))
+    (unless coding
+      (setq coding (funcall set-auto-coding-function
+                           filename (- (point-max) (point-min)))))
+    (unless coding
+      (setq coding (car (find-operation-coding-system
+                        'insert-file-contents
+                        (cons filename (current-buffer))
+                        visit beg end replace))))
+    (if (coding-system-p coding)
+        (or enable-multibyte-characters
+           (setq coding
+                 (coding-system-change-text-conversion coding 'raw-text)))
+      (setq coding nil))
+    coding))
+
 (defun decode-coding-inserted-region (from to filename
                                           &optional visit beg end replace)
   "Decode the region between FROM and TO as if it is read from file FILENAME.
@@ -2253,8 +2272,7 @@ decode-coding-inserted-region
 Part of the job of this function is setting `buffer-undo-list' appropriately."
   (save-excursion
     (save-restriction
-      (let ((coding coding-system-for-read)
-           undo-list-saved)
+      (let (coding undo-list-saved)
        (if visit
            ;; Temporarily turn off undo recording, if we're decoding the
            ;; text of a visited file.
@@ -2268,19 +2286,8 @@ decode-coding-inserted-region
                      buffer-undo-list t))))
        (narrow-to-region from to)
        (goto-char (point-min))
-       (or coding
-           (setq coding (funcall set-auto-coding-function
-                                 filename (- (point-max) (point-min)))))
-       (or coding
-           (setq coding (car (find-operation-coding-system
-                              'insert-file-contents
-                              (cons filename (current-buffer))
-                              visit beg end replace))))
-       (if (coding-system-p coding)
-           (or enable-multibyte-characters
-               (setq coding
-                     (coding-system-change-text-conversion coding 'raw-text)))
-         (setq coding nil))
+        (setq coding (decode-coding-inserted-region-coding-system
+                      filename visit beg end replace))
        (if coding
            (decode-coding-region (point-min) (point-max) coding)
          (setq last-coding-system-used coding))

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