From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stefan Monnier Newsgroups: gmane.emacs.bugs Subject: bug#7277: 24.0.50; Can't revert diff-buffer-with-file Date: Mon, 22 Nov 2010 14:22:22 -0500 Message-ID: References: <19652.38745.217242.560815@rgr.rgrjr.com> NNTP-Posting-Host: lo.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: dough.gmane.org 1290456895 31817 80.91.229.12 (22 Nov 2010 20:14:55 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Mon, 22 Nov 2010 20:14:55 +0000 (UTC) To: rogers-emacs@rgrjr.dyndns.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Mon Nov 22 21:14:50 2010 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1PKcmi-0001J7-Oj for geb-bug-gnu-emacs@m.gmane.org; Mon, 22 Nov 2010 21:14:50 +0100 Original-Received: from localhost ([127.0.0.1]:36470 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PKcmh-0000oR-SD for geb-bug-gnu-emacs@m.gmane.org; Mon, 22 Nov 2010 15:14:43 -0500 Original-Received: from [140.186.70.92] (port=38529 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PKcmc-0000o5-D0 for bug-gnu-emacs@gnu.org; Mon, 22 Nov 2010 15:14:40 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PKcma-0003HY-Jv for bug-gnu-emacs@gnu.org; Mon, 22 Nov 2010 15:14:38 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:56020) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PKcma-0003HQ-Gr for bug-gnu-emacs@gnu.org; Mon, 22 Nov 2010 15:14:36 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.69) (envelope-from ) id 1PKcRh-0000Bw-TR for bug-gnu-emacs@gnu.org; Mon, 22 Nov 2010 14:53:01 -0500 Resent-From: Stefan Monnier Original-Sender: debbugs-submit-bounces@debbugs.gnu.org Resent-To: bug-gnu-emacs@gnu.org Resent-Date: Mon, 22 Nov 2010 19:53:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: cc-closed 7277 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Mail-Followup-To: 7277@debbugs.gnu.org, monnier@IRO.UMontreal.CA Original-Received: via spool by 7277-done@debbugs.gnu.org id=D7277.1290455539727 (code D ref 7277); Mon, 22 Nov 2010 19:53:01 +0000 Original-Received: (at 7277-done) by debbugs.gnu.org; 22 Nov 2010 19:52:19 +0000 Original-Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PKcR0-0000Bg-0j for submit@debbugs.gnu.org; Mon, 22 Nov 2010 14:52:19 -0500 Original-Received: from pruche.dit.umontreal.ca ([132.204.246.22]) by debbugs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1PKbt7-0008OL-8d for 7277-done@debbugs.gnu.org; Mon, 22 Nov 2010 14:17:19 -0500 Original-Received: from pastel.home (lechon.iro.umontreal.ca [132.204.27.242]) by pruche.dit.umontreal.ca (8.14.1/8.14.1) with ESMTP id oAMJMQed007573; Mon, 22 Nov 2010 14:22:27 -0500 Original-Received: by pastel.home (Postfix, from userid 20848) id 43474A83C8; Mon, 22 Nov 2010 14:22:22 -0500 (EST) In-Reply-To: <19652.38745.217242.560815@rgr.rgrjr.com> (rogers-emacs@rgrjr.dyndns.org's message of "Sun, 24 Oct 2010 16:30:17 -0400") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.0.50 (gnu/linux) X-NAI-Spam-Score: 0 X-NAI-Spam-Rules: 1 Rules triggered RV3687=0 X-Mailman-Approved-At: Mon, 22 Nov 2010 14:52:16 -0500 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.11 Precedence: list Resent-Date: Mon, 22 Nov 2010 14:53:01 -0500 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:41825 Archived-At: > The attached patch does a minimal refactoring of "diff" and > "diff-buffer-with-file" to do just that, while trying to DTRT if the > user renames the diff buffer or kills the original buffer. Thanks, installed. Actually, I then started to take a closer look at the code and ended up rewriting most of it to fix various problems. See patch below. Stefan === modified file 'lisp/ChangeLog' --- lisp/ChangeLog 2010-11-22 17:57:46 +0000 +++ lisp/ChangeLog 2010-11-22 19:19:24 +0000 @@ -1,3 +1,20 @@ +2010-11-22 Stefan Monnier + + * vc/diff.el (diff-old-temp-file, diff-new-temp-file): Remove. + (diff-sentinel): Get them as arguments instead. + (diff-old-file, diff-new-file, diff-extra-args): Remove. + (diff-file-local-copy, diff-better-file-name): New funs. + (diff-no-select): Rename from diff-into-buffer. + Support buffers additionally to files. Move `buf' arg. Don't display buf. + Prefer closures to buffer-local variables. + (diff): Adjust accordingly. + (diff-buffer-with-file): Move from files.el. + * files.el (diff-buffer-with-file): Move to vc/diff.el. + (diff-buffer-internal): Remove. + (diff-buffer-buffer): Remove. + (save-some-buffers-action-alist): Use diff-no-select so as not to guess + the buffer name used, and so as not to mess up windows and frames. + 2010-11-22 Bob Rogers * files.el: Make revert work with diff-buffer-with-file (bug#7277). === modified file 'lisp/files.el' --- lisp/files.el 2010-11-22 17:57:46 +0000 +++ lisp/files.el 2010-11-22 19:13:31 +0000 @@ -4487,47 +4487,6 @@ (setq buffer-backed-up nil)))))) setmodes)) -(defun diff-buffer-with-file (&optional buffer) - "View the differences between BUFFER and its associated file. -This requires the external program `diff' to be in your `exec-path'." - (interactive "bBuffer: ") - (diff-buffer-internal (get-buffer (or buffer (current-buffer))) - (get-buffer-create "*Diff*")) - ;; return always nil, so that save-buffers-kill-emacs will not move - ;; over to the next unsaved buffer when calling `d'. - nil) - -(defvar diff-buffer-buffer) ;; suppress compiler warnings. - -(defun diff-buffer-internal (buffer result-buffer) - (if (not (and buffer (buffer-name buffer))) - (error "Original buffer deleted.")) - (with-current-buffer buffer - (if (and buffer-file-name - (file-exists-p buffer-file-name)) - (let ((tempfile (make-temp-file "buffer-content-"))) - (unwind-protect - (progn - (write-region nil nil tempfile nil 'nomessage) - ;; No asynch so we don't delete the temp file prematurely. - (diff-into-buffer result-buffer buffer-file-name tempfile - nil t) - (sit-for 0) - ;; Now revise the revert-buffer-function, since the - ;; default will not be able to find the temp file. - (with-current-buffer result-buffer - (set (make-local-variable 'diff-buffer-buffer) buffer) - (setq revert-buffer-function - (lambda (ignore-auto noconfirm) - (diff-buffer-internal diff-buffer-buffer - (current-buffer)))))) - (when (file-exists-p tempfile) - (delete-file tempfile)))) - (message "Buffer %s has no associated file on disc" (buffer-name)) - ;; Display that message for 1 second so that user can read it - ;; in the minibuffer. - (sit-for 1)))) - (defvar save-some-buffers-action-alist `((?\C-r ,(lambda (buf) @@ -4542,13 +4501,14 @@ (?d ,(lambda (buf) (if (null (buffer-file-name buf)) (message "Not applicable: no file") - (save-window-excursion (diff-buffer-with-file buf)) + (require 'diff) ;for diff-no-select. + (let ((diffbuf (diff-no-select (buffer-file-name buf) buf + nil 'noasync))) (if (not enable-recursive-minibuffers) - (progn (display-buffer (get-buffer-create "*Diff*")) - (setq other-window-scroll-buffer "*Diff*")) - (view-buffer (get-buffer-create "*Diff*") - (lambda (_) (exit-recursive-edit))) - (recursive-edit))) + (progn (display-buffer diffbuf) + (setq other-window-scroll-buffer diffbuf)) + (view-buffer diffbuf (lambda (_) (exit-recursive-edit))) + (recursive-edit)))) ;; Return nil to ask about BUF again. nil) ,(purecopy "view changes in this buffer"))) === modified file 'lisp/vc/diff.el' --- lisp/vc/diff.el 2010-11-22 17:57:46 +0000 +++ lisp/vc/diff.el 2010-11-22 19:19:14 +0000 @@ -31,6 +31,8 @@ ;;; Code: +(eval-when-compile (require 'cl)) + (defgroup diff nil "Comparing files with `diff'." :group 'tools) @@ -47,11 +49,6 @@ :type 'string :group 'diff) -(defvar diff-old-temp-file nil - "This is the name of a temp file to be deleted after diff finishes.") -(defvar diff-new-temp-file nil - "This is the name of a temp file to be deleted after diff finishes.") - ;; prompt if prefix arg present (defun diff-switches () (if current-prefix-arg @@ -60,12 +57,12 @@ diff-switches (mapconcat 'identity diff-switches " "))))) -(defun diff-sentinel (code) +(defun diff-sentinel (code old-temp-file new-temp-file) "Code run when the diff process exits. CODE is the exit code of the process. It should be 0 only if no diffs were found." - (if diff-old-temp-file (delete-file diff-old-temp-file)) - (if diff-new-temp-file (delete-file diff-new-temp-file)) + (if old-temp-file (delete-file old-temp-file)) + (if new-temp-file (delete-file new-temp-file)) (save-excursion (goto-char (point-max)) (let ((inhibit-read-only t)) @@ -75,10 +72,6 @@ (t "")) (current-time-string)))))) -(defvar diff-old-file nil) -(defvar diff-new-file nil) -(defvar diff-extra-args nil) - ;;;###autoload (defun diff (old new &optional switches no-async) "Find and display the differences between OLD and NEW files. @@ -91,16 +84,15 @@ interactively for diff switches. Otherwise, the switches specified in `diff-switches' are passed to the diff command." (interactive - (let (oldf newf) - (setq newf (buffer-file-name) - newf (if (and newf (file-exists-p newf)) + (let ((newf (buffer-file-name)) + (oldf (file-newest-backup newf))) + (setq newf (if (and newf (file-exists-p newf)) (read-file-name (concat "Diff new file (default " (file-name-nondirectory newf) "): ") nil newf t) (read-file-name "Diff new file: " nil nil t))) - (setq oldf (file-newest-backup newf) - oldf (if (and oldf (file-exists-p oldf)) + (setq oldf (if (and oldf (file-exists-p oldf)) (read-file-name (concat "Diff original file (default " (file-name-nondirectory oldf) "): ") @@ -108,63 +100,82 @@ (read-file-name "Diff original file: " (file-name-directory newf) nil t))) (list oldf newf (diff-switches)))) - (diff-into-buffer nil old new switches no-async)) + (display-buffer + (diff-no-select old new switches no-async))) -(defun diff-into-buffer (buf old new &optional switches no-async) - ;; Noninteractive helper for creating and reverting diff buffers. - (setq new (expand-file-name new) - old (expand-file-name old)) +(defun diff-file-local-copy (file-or-buf) + (if (bufferp file-or-buf) + (with-current-buffer file-or-buf + (let ((tempfile (make-temp-file "buffer-content-"))) + (write-region nil nil tempfile nil 'nomessage) + tempfile)) + (file-local-copy file-or-buf))) + +(defun diff-better-file-name (file) + (if (bufferp file) file + (let ((rel (file-relative-name file)) + (abbr (abbreviate-file-name (expand-file-name file)))) + (if (< (length abbr) (length rel)) + abbr + rel)))) + +(defun diff-no-select (old new &optional switches no-async buf) + ;; Noninteractive helper for creating and reverting diff buffers + (setq new (diff-better-file-name new) + old (diff-better-file-name old)) (or switches (setq switches diff-switches)) ; If not specified, use default. + (unless (listp switches) (setq switches (list switches))) (or buf (setq buf (get-buffer-create "*Diff*"))) - (let* ((old-alt (file-local-copy old)) - (new-alt (file-local-copy new)) + (let* ((old-alt (diff-file-local-copy old)) + (new-alt (diff-file-local-copy new)) (command (mapconcat 'identity `(,diff-command ;; Use explicitly specified switches - ,@(if (listp switches) switches (list switches)) - ,@(if (or old-alt new-alt) - (list "-L" old "-L" new)) - ,(shell-quote-argument (or old-alt old)) - ,(shell-quote-argument (or new-alt new))) + ,@switches + ,@(mapcar #'shell-quote-argument + (nconc + (when (or old-alt new-alt) + (list "-L" (if (stringp old) + old (prin1-to-string old)) + "-L" (if (stringp new) + new (prin1-to-string new)))) + (list (or old-alt old) + (or new-alt new))))) " ")) - (thisdir default-directory) - proc) - (save-excursion - (display-buffer buf) - (set-buffer buf) - (setq buffer-read-only nil) + (thisdir default-directory)) + (with-current-buffer buf + (setq buffer-read-only t) (buffer-disable-undo (current-buffer)) (let ((inhibit-read-only t)) (erase-buffer)) (buffer-enable-undo (current-buffer)) (diff-mode) - ;; Use below 2 vars for backward-compatibility. - (set (make-local-variable 'diff-old-file) old) - (set (make-local-variable 'diff-new-file) new) - (set (make-local-variable 'diff-extra-args) (list switches no-async)) (set (make-local-variable 'revert-buffer-function) + (lexical-let ((old old) (new new) + (switches switches) + (no-async no-async)) (lambda (ignore-auto noconfirm) - (apply 'diff diff-old-file diff-new-file diff-extra-args))) - (set (make-local-variable 'diff-old-temp-file) old-alt) - (set (make-local-variable 'diff-new-temp-file) new-alt) + (diff-no-select old new switches no-async (current-buffer))))) (setq default-directory thisdir) (let ((inhibit-read-only t)) (insert command "\n")) (if (and (not no-async) (fboundp 'start-process)) - (progn - (setq proc (start-process "Diff" buf shell-file-name - shell-command-switch command)) + (let ((proc (start-process "Diff" buf shell-file-name + shell-command-switch command))) (set-process-filter proc 'diff-process-filter) + (lexical-let ((old-alt old-alt) (new-alt new-alt)) (set-process-sentinel proc (lambda (proc msg) (with-current-buffer (process-buffer proc) - (diff-sentinel (process-exit-status proc)))))) + (diff-sentinel (process-exit-status proc) + old-alt new-alt)))))) ;; Async processes aren't available. (let ((inhibit-read-only t)) (diff-sentinel (call-process shell-file-name nil buf nil - shell-command-switch command))))) + shell-command-switch command) + old-alt new-alt)))) buf)) (defun diff-process-filter (proc string) @@ -203,6 +214,14 @@ (funcall handler 'diff-latest-backup-file fn) (file-newest-backup fn)))) +;;;###autoload +(defun diff-buffer-with-file (&optional buffer) + "View the differences between BUFFER and its associated file. +This requires the external program `diff' to be in your `exec-path'." + (interactive "bBuffer: ") + (with-current-buffer (get-buffer (or buffer (current-buffer))) + (diff buffer-file-name (current-buffer) nil 'noasync))) + (provide 'diff) ;; arch-tag: 7de2c29b-7ea5-4b85-9b9d-72dd860de2bd