bug#12586: 24.2.50; "q" in empty vc-diff buffer causes error

bug#12586: 24.2.50; "q" in empty vc-diff buffer causes error

[Eric Hanchrow]

This bug report will be sent to the Bug-GNU-Emacs mailing list
and the GNU bug tracker at debbugs.gnu.org.  Please check that
the From: line contains a valid email address.  After a delay of up
to one day, you should receive an acknowledgment at that address.

Please write in English if possible, as the Emacs maintainers
usually do not have translators for other languages.

Please describe exactly what actions triggered the bug, and
the precise symptoms of the bug.  If you can, give a recipe
starting from `emacs -Q':


* Prepare a version-controlled file

These instructions use git, but I've found that the analagous steps
using RCS also reproduce the problem.

At your shell:

#+BEGIN_EXAMPLE
$ cd /tmp
$ rm -rf x
$ mkdir x
$ cd x
$ git init
$ echo Hey baby > frotz
$ git add .
$ git commit -m "A commit"
#+END_EXAMPLE

Now we have a git repo that contains a single file.

Now visit the file with emacs

- C-x C-f /tmp/frotz RET

Change the file:

- w a s s u p RET C-x C-s

Create a *vc-diff* buffer that shows the changes:

- C-x v =

Restore the file to its checked-in state

- C-x v u y e s RET

Refresh the *vc-diff* buffer

- g

At this point, everything is as it should be: we're looking at the
file in the upper window, and a (now-empty) *vc-diff* buffer in the
lower window.

Now try to quit the *vc-diff* buffer:

- q

I hear the bell ring, and see a message in the echo area:

	Buffer is read-only: #<buffer *vc-diff*>

I was expecting the *vc-diff* window to close, and the *vc-diff*
buffer to get buried, which is what _normally_ happens when I type
"q".


If Emacs crashed, and you have the Emacs process in the gdb debugger,
please include the output from the following gdb commands:
    `bt full' and `xbacktrace'.
For information about debugging Emacs, please read the file
/Applications/Emacs.app/Contents/Resources/etc/DEBUG.


In GNU Emacs 24.2.50.1 (x86_64-apple-darwin12.1.0, NS apple-appkit-1187.00)
 of 2012-09-20 on Eric-Hanchrows-MacBook-Air.local
Windowing system distributor `Apple', version 10.3.1187
Configured using:
 `configure '--with-ns' 'CFLAGS=-O2 -arch x86_64'
 'LDFLAGS=-L/opt/local/lib' 'CPPFLAGS=-I/opt/local/include''

Important settings:
  locale-coding-system: nil
  default enable-multibyte-characters: t

Major mode: Fundamental

Minor modes in effect:
  diff-auto-refine-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-x C-f / t m p / f r t o z <backspace> <backspace>
<backspace> <tab> / / t m p / f r <tab> <M-backspace>
x <tab> f <tab> <return> C-M-o W a s s u p C-x C-s
C-x v = q C-x v u y e s <return> C-x v = C-M-o C-w
W a s s u p C-x C-s C-x v = C-x o C-x v u y e s <return>
C-x o g q C-x 0 C-x C-f <backspace> <backspace> <return>
C-u C-n C-u C-n C-n C-n a y C-x C-f w h e e C-j W a
s s u p <return> C-x C-s C-x v v r c <tab> <backspace>
<backspace> R <tab> <return> <return> C-x v = H e y
C-x C-q H e y SPC b a b y <return> C-x C-s C-x y e
<backspace> y e s <return> C-x v = C-x o C-x v u y
e s <return> y e s <return> C-x o g q C-x 0 C-x k <return>
C-x k <return> M-> Y o SPC m o m m a <return> C-x C-s
C-x v = C-x o C-x o C-x v u y e s <return> g q C-x
1 C-x b m e s s <M-backspace> * m e <tab> <backspace>
<backspace> M e <tab> <return> C-p M-f M-f M-f M-b
C-SPC C-e M-w <f5> C-x k <return> C-x k <return> H
e y SPC c u z <return> C-x C-s C-x v = q M-x r e p
o r t - e m <tab> <return>

Recent messages:
Saving file /tmp/x/frotz...
Wrote /tmp/x/frotz
Finding changes in /tmp/x/frotz...done

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils vc-arch vc-mtn vc-hg vc-bzr vc-sccs vc-svn vc-cvs
vc-rcs help-mode novice dired diff-mode easymenu easy-mmode vc
vc-dispatcher vc-git time-date tooltip ediff-hook vc-hooks
lisp-float-type mwheel ns-win tool-bar dnd fontset image regexp-opt
fringe tabulated-list newcomment lisp-mode register page menu-bar
rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax
facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak
czech european ethiopic indian cyrillic chinese case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
make-network-process dbusbind ns multi-tty emacs)

bug#12586: Acknowledgement (24.2.50; "q" in empty vc-diff buffer causes error)

[Eric Hanchrow]

This patch seems to fix the problem.

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 47800bd..c66c4db 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1590,21 +1590,22 @@ Return t if the buffer had changes, nil otherwise."
     (let ((vc-disable-async-diff (not async)))
       (vc-call-backend (car vc-fileset) 'diff files rev1 rev2 buffer))
     (set-buffer buffer)
-    (if (and (zerop (buffer-size))
-             (not (get-buffer-process (current-buffer))))
+    (let ((pop (not
+
         ;; Treat this case specially so as not to pop the buffer.
-        (progn
-          (message "%s" (cdr messages))
-          nil)
-      (diff-mode)
+                (and (zerop (buffer-size))
+                     (not (get-buffer-process (current-buffer)))))))
+
       (set (make-local-variable 'diff-vc-backend) (car vc-fileset))
       (set (make-local-variable 'revert-buffer-function)
 	   `(lambda (ignore-auto noconfirm)
 	      (vc-diff-internal ,async ',vc-fileset ,rev1 ,rev2 ,verbose)))
-      ;; Make the *vc-diff* buffer read only, the diff-mode key
-      ;; bindings are nicer for read only buffers. pcl-cvs does the
-      ;; same thing.
-      (setq buffer-read-only t)
+
+      (let ((result (if (not pop)
+                        (progn
+                          (message "%s" (cdr messages))
+                          nil)
+
       ;; Display the buffer, but at the end because it can change point.
       (pop-to-buffer (current-buffer))
       ;; The diff process may finish early, so call `vc-diff-finish'
@@ -1616,6 +1617,14 @@ Return t if the buffer had changes, nil otherwise."
       ;; because we don't know that yet.
       t)))

+        ;; Make the *vc-diff* buffer read only, the diff-mode key
+        ;; bindings are nicer for read only buffers. pcl-cvs does the
+        ;; same thing.
+        (diff-mode)
+        (setq buffer-read-only t)
+
+        result))))
+
 (defun vc-read-revision (prompt &optional files backend default initial-input)
   (cond
    ((null files)

bug#12586: Acknowledgement (24.2.50; "q" in empty vc-diff buffer causes error)

[Chong Yidong]

Eric Hanchrow <eric.hanchrow@gmail.com> writes:

> This patch seems to fix the problem.

Thanks.  I committed a slightly simpler one to trunk.