Re: BUG: which-func-mode

[Stefan Monnier]

> 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)