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

[Lars Ingebrigtsen <larsi@gnus.org>]

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