From mboxrd@z Thu Jan 1 00:00:00 1970 Path: main.gmane.org!not-for-mail From: "Stefan Monnier" Newsgroups: gmane.emacs.devel Subject: Re: BUG: which-func-mode Date: Mon, 10 Mar 2003 13:28:18 -0500 Sender: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Message-ID: <200303101828.h2AISIrs027316@rum.cs.yale.edu> References: NNTP-Posting-Host: main.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: main.gmane.org 1047320954 11107 80.91.224.249 (10 Mar 2003 18:29:14 GMT) X-Complaints-To: usenet@main.gmane.org NNTP-Posting-Date: Mon, 10 Mar 2003 18:29:14 +0000 (UTC) Cc: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Mon Mar 10 19:29:13 2003 Return-path: Original-Received: from quimby.gnus.org ([80.91.224.244]) by main.gmane.org with esmtp (Exim 3.35 #1 (Debian)) id 18sS1R-0002su-00 for ; Mon, 10 Mar 2003 19:29:13 +0100 Original-Received: from monty-python.gnu.org ([199.232.76.173]) by quimby.gnus.org with esmtp (Exim 3.12 #1 (Debian)) id 18sSNo-0005aA-00 for ; Mon, 10 Mar 2003 19:52:20 +0100 Original-Received: from localhost ([127.0.0.1] helo=monty-python.gnu.org) by monty-python.gnu.org with esmtp (Exim 4.10.13) id 18sS0v-0000sZ-00 for emacs-devel@quimby.gnus.org; Mon, 10 Mar 2003 13:28:41 -0500 Original-Received: from list by monty-python.gnu.org with tmda-scanned (Exim 4.10.13) id 18sS0a-0000rT-00 for emacs-devel@gnu.org; Mon, 10 Mar 2003 13:28:20 -0500 Original-Received: from mail by monty-python.gnu.org with spam-scanned (Exim 4.10.13) id 18sS0Z-0000qQ-00 for emacs-devel@gnu.org; Mon, 10 Mar 2003 13:28:20 -0500 Original-Received: from rum.cs.yale.edu ([128.36.229.169]) by monty-python.gnu.org with esmtp (Exim 4.10.13) id 18sS0Z-0000qM-00 for emacs-devel@gnu.org; Mon, 10 Mar 2003 13:28:19 -0500 Original-Received: from rum.cs.yale.edu (localhost [127.0.0.1]) by rum.cs.yale.edu (8.12.8/8.12.8) with ESMTP id h2AISIj6027318; Mon, 10 Mar 2003 13:28:18 -0500 Original-Received: (from monnier@localhost) by rum.cs.yale.edu (8.12.8/8.12.8/Submit) id h2AISIrs027316; Mon, 10 Mar 2003 13:28:18 -0500 X-Mailer: exmh version 2.4 06/23/2000 with nmh-1.0.4 Original-To: Le Wang X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1b5 Precedence: list List-Id: Emacs development discussions. List-Help: List-Post: List-Subscribe: , List-Archive: List-Unsubscribe: , Errors-To: emacs-devel-bounces+emacs-devel=quimby.gnus.org@gnu.org Xref: main.gmane.org gmane.emacs.devel:12245 X-Report-Spam: http://spam.gmane.org/gmane.emacs.devel:12245 > I just spend a few hours debugging a situation where something was modifying > my buffer-list behind my back, whenever I had multiple windows in the same > frame. > > It turned out that `which-func-mode' was using `walk-windows' to update the > mode-lines in all windows. It selects each window and forces a mode-line > update in it. But by selecting the window, it silently disrupts the > `buffer-list'. This is the bug. Yes. We need a lower-level form of `select-window' for such uses. Actually I'd recommend naming it `with-selected-window'. > ! (save-selected-window > ! (select-window window) [...] > ! (set-buffer (window-buffer window)) I think the patch is basically more-or-less right, except for the following issues: - I'd use with-current-buffer over set-buffer as a matter of style. After all, the function's name and docstring doesn't suggest that it might change the current buffer. - I'm wondering whether it's good to update all the windows. After all, when you have a hundred windows, it's pretty silly to update them all all the time. - this brings up the old issue of buffers vs windows: the code obviously wants to treat each window independently, so that it does TRT if you have two windows showing the same buffer (at two different positions), but it actually odesn't work because it ends up relying on a buffer-local variable (there are no window-local variables in Emacs). Your patch thus doesn't really change the behavior but makes the code odd since it cycles through windows but only selects the corresponding buffers. The patch below fixes the buffer/window issue and removes the "update all" behavior. Maybe the "update all" behvaior should be restored, but it requires the lower-level select-window function mentioned above. Also I noticed while hacking it up that I need to set the risky-local-variable property on both which-func-format and which-func-current although it seems it should only be needed on which-func-current. What do people think ? Stefan --- which-func.el.~1.29.~ Wed Feb 5 10:55:11 2003 +++ which-func.el Mon Mar 10 13:22:41 2003 @@ -103,6 +103,7 @@ "Format for displaying the function in the mode line." :group 'which-func :type 'sexp) +(put 'which-func-format 'risky-local-variable t) (defvar which-func-cleanup-function nil "Function to transform a string before displaying it in the mode line. @@ -120,10 +121,11 @@ ;;; (require 'imenu) -(defvar which-func-current which-func-unknown) -(defvar which-func-previous which-func-unknown) -(make-variable-buffer-local 'which-func-current) -(make-variable-buffer-local 'which-func-previous) +(defvar which-func-table (make-hash-table :test 'eq :weakness 'key)) + +(defvar which-func-current + '(:eval (gethash (selected-window) which-func-table which-func-unknown))) +(put 'which-func-current 'risky-local-variable t) (defvar which-func-mode nil "Non-nil means display current function name in mode line. @@ -153,24 +155,19 @@ (setq which-func-mode nil)))) (defun which-func-update () - "Update the Which-Function mode display for all windows." - (walk-windows 'which-func-update-1 nil 'visible)) - -(defun which-func-update-1 (window) - "Update the Which-Function mode display for window WINDOW." - (save-selected-window - (select-window window) - ;; Update the string containing the current function. + ;; "Update the Which-Function mode display for all windows." + ;; (walk-windows 'which-func-update-1 nil 'visible)) + "Update the Which-Function mode display for the current window." (when which-func-mode (condition-case info - (progn - (setq which-func-current (or (which-function) which-func-unknown)) - (unless (string= which-func-current which-func-previous) - (force-mode-line-update) - (setq which-func-previous which-func-current))) + (let ((current (which-function))) + (unless (string= current + (gethash (selected-window) which-func-table)) + (puthash (selected-window) current which-func-table) + (force-mode-line-update))) (error (which-func-mode -1) - (error "Error in which-func-update: %s" info)))))) + (error "Error in which-func-update: %s" info))))) ;;;###autoload (defalias 'which-func-mode 'which-function-mode)