bug#21262: 25.0.50; Diff-mode gets confused about its narrowing if hunk source not found

[charles@aurox.ch (Charles A. Roelli)]

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

> From: Dima Kogan <dima@secretsauce.net>
> Date: Fri, 14 Aug 2015 22:12:09 -0700
> 
> Hi. This bug has been there since at least emacs23 it seems like. To
> reproduce:
> 
> 1. emacs -Q
> 
> 2. Open any patch file that has hunks from more than one file in it. For
> instance, the attached patch file works, but it really doesn't matter
> 
> 3. Narrow to just the second file's diffs. For instance: M-N M-N C-SPC
> C-> C-x n n SPC (last SPC to confirm the narrowing)
> 
> 4. Try to see the source of the hunk. Move the point to the guts of the
> hunk, and M-enter. Emacs asks for the directory where the sources live.
> In the process, it changes the narrowing to the wrong hunk. This is
> wrong. The narrowing shouldn't change.
> 
> 5. If we cancel with C-g, the wrong narrowing persists
> 
> 
> 
> diff --git a/src/charset.c b/src/charset.c
> index b19e344..eeebf17 100644
> --- a/src/charset.c
> +++ b/src/charset.c
> @@ -555,7 +555,7 @@ load_charset_map_from_vector (struct charset *charset, Lisp_Object vec, int cont
>  
>    if (len % 2 == 1)
>      {
> -      add_to_log ("Failure in loading charset map: %V", vec, Qnil);
> +      add_to_log ("Failure in loading charset map: %V", vec);
>        return;
>      }
>  
> diff --git a/src/image.c b/src/image.c
> index 066db74..313419b 100644
> --- a/src/image.c
> +++ b/src/image.c
> @@ -629,16 +629,19 @@ valid_image_p (Lisp_Object object)
>  }
>  
>  
> -/* Log error message with format string FORMAT and argument ARG.
> +/* Log error message with format string FORMAT and trailing arguments.
>     Signaling an error, e.g. when an image cannot be loaded, is not a
>     good idea because this would interrupt redisplay, and the error
>     message display would lead to another redisplay.  This function
>     therefore simply displays a message.  */
>  
>  static void
> -image_error (const char *format, Lisp_Object arg1, Lisp_Object arg2)
> +image_error (const char *format, ...)
>  {
> -  add_to_log (format, arg1, arg2);
> +  va_list ap;
> +  va_start (ap, format);
> +  vadd_to_log (format, ap);
> +  va_end (ap);
>  }
>  
>  

This still affects 26.0.50.

The attached change fixes it -- but I know nothing of diff-mode, so
better ideas are welcome.  Note that the buffer will be widened while
Emacs asks where to look for the sources, but it will return to the
original narrowing after the user has selected a file or hit C-g.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 01-emacs-bug-21262-draft.patch --]
[-- Type: text/x-patch, Size: 4937 bytes --]

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index aa8d778..b470406 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -874,52 +874,54 @@ diff-find-file-name
     ;; Flush diff-remembered-files-alist if the default-directory is changed.
     (set (make-local-variable 'diff-remembered-defdir) default-directory)
     (set (make-local-variable 'diff-remembered-files-alist) nil))
-  (save-excursion
-    (unless (looking-at diff-file-header-re)
-      (or (ignore-errors (diff-beginning-of-file))
-	  (re-search-forward diff-file-header-re nil t)))
-    (let ((fs (diff-hunk-file-names old)))
-      (if prefix (setq fs (mapcar (lambda (f) (concat prefix f)) fs)))
-      (or
-       ;; use any previously used preference
-       (cdr (assoc fs diff-remembered-files-alist))
-       ;; try to be clever and use previous choices as an inspiration
-       (cl-dolist (rf diff-remembered-files-alist)
-	 (let ((newfile (diff-merge-strings (caar rf) (car fs) (cdr rf))))
-	   (if (and newfile (file-exists-p newfile)) (cl-return newfile))))
-       ;; look for each file in turn.  If none found, try again but
-       ;; ignoring the first level of directory, ...
-       (cl-do* ((files fs (delq nil (mapcar 'diff-filename-drop-dir files)))
-                (file nil nil))
-	   ((or (null files)
-		(setq file (cl-do* ((files files (cdr files))
-                                    (file (car files) (car files)))
-			       ;; Use file-regular-p to avoid
-			       ;; /dev/null, directories, etc.
-			       ((or (null file) (file-regular-p file))
-				file))))
-	    file))
-       ;; <foo>.rej patches implicitly apply to <foo>
-       (and (string-match "\\.rej\\'" (or buffer-file-name ""))
-	    (let ((file (substring buffer-file-name 0 (match-beginning 0))))
-	      (when (file-exists-p file) file)))
-       ;; If we haven't found the file, maybe it's because we haven't paid
-       ;; attention to the PCL-CVS hint.
-       (and (not prefix)
-	    (boundp 'cvs-pcl-cvs-dirchange-re)
-	    (save-excursion
-	      (re-search-backward cvs-pcl-cvs-dirchange-re nil t))
-	    (diff-find-file-name old noprompt (match-string 1)))
-       ;; if all else fails, ask the user
-       (unless noprompt
-         (let ((file (expand-file-name (or (car fs) ""))))
-	   (setq file
-		 (read-file-name (format "Use file %s: " file)
-				 (file-name-directory file) file t
-				 (file-name-nondirectory file)))
-           (set (make-local-variable 'diff-remembered-files-alist)
-                (cons (cons fs file) diff-remembered-files-alist))
-           file))))))
+  (save-restriction
+    (widen)
+    (save-excursion
+      (unless (looking-at diff-file-header-re)
+        (or (ignore-errors (diff-beginning-of-file))
+	    (re-search-forward diff-file-header-re nil t)))
+      (let ((fs (diff-hunk-file-names old)))
+        (if prefix (setq fs (mapcar (lambda (f) (concat prefix f)) fs)))
+        (or
+         ;; use any previously used preference
+         (cdr (assoc fs diff-remembered-files-alist))
+         ;; try to be clever and use previous choices as an inspiration
+         (cl-dolist (rf diff-remembered-files-alist)
+	   (let ((newfile (diff-merge-strings (caar rf) (car fs) (cdr rf))))
+	     (if (and newfile (file-exists-p newfile)) (cl-return newfile))))
+         ;; look for each file in turn.  If none found, try again but
+         ;; ignoring the first level of directory, ...
+         (cl-do* ((files fs (delq nil (mapcar 'diff-filename-drop-dir files)))
+                  (file nil nil))
+	     ((or (null files)
+		  (setq file (cl-do* ((files files (cdr files))
+                                      (file (car files) (car files)))
+			         ;; Use file-regular-p to avoid
+			         ;; /dev/null, directories, etc.
+			         ((or (null file) (file-regular-p file))
+				  file))))
+	      file))
+         ;; <foo>.rej patches implicitly apply to <foo>
+         (and (string-match "\\.rej\\'" (or buffer-file-name ""))
+	      (let ((file (substring buffer-file-name 0 (match-beginning 0))))
+	        (when (file-exists-p file) file)))
+         ;; If we haven't found the file, maybe it's because we haven't paid
+         ;; attention to the PCL-CVS hint.
+         (and (not prefix)
+	      (boundp 'cvs-pcl-cvs-dirchange-re)
+	      (save-excursion
+	        (re-search-backward cvs-pcl-cvs-dirchange-re nil t))
+	      (diff-find-file-name old noprompt (match-string 1)))
+         ;; if all else fails, ask the user
+         (unless noprompt
+           (let ((file (expand-file-name (or (car fs) ""))))
+	     (setq file
+		   (read-file-name (format "Use file %s: " file)
+				   (file-name-directory file) file t
+				   (file-name-nondirectory file)))
+             (set (make-local-variable 'diff-remembered-files-alist)
+                  (cons (cons fs file) diff-remembered-files-alist))
+             file)))))))
 
 
 (defun diff-ediff-patch ()