bug#66283: 30.0.50; which-function-mode: When configured to display in header, and toggling off, then does not remove header

[sbaugh@catern.com]

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

Eli Zaretskii <eliz@gnu.org> writes:
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Cc: Mekeor Melire <mekeor@posteo.de>,  Author <sbaugh@catern.com>,
>>    66283@debbugs.gnu.org,  Committer <eliz@gnu.org>
>> Date: Wed, 04 Oct 2023 11:13:06 -0400
>> 
>> Two fixes to the previous patch:
>> 
>> - There was always a bug with which-function-mode where if you enabled
>>   it when there were already existing buffers, it would behave
>>   differently than if you enabled it and then created those buffers.
>>   Namely it would enable which-func-mode for every buffer, even if the
>>   buffer didn't support imenu.  This is especially noticeable when
>>   toggling which-function-mode and using the header line, so I fixed it.
>> 
>> - Also, I accidentally dropped a paren before submitting.  Now that's
>>   fixed :)
>
> Thanks, but how about some tests for these fixes?  It's high time
> which-mode had some test suite, I think.

Sure, here's a patch including a test.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Remove-the-header-line-after-disabling-which-functio.patch --]
[-- Type: text/x-patch, Size: 7150 bytes --]

From b0ae708df7cf81419fbbf6f81fcda31142626a27 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Sat, 21 Oct 2023 10:41:42 -0400
Subject: [PATCH] Remove the header line after disabling which-function-mode

Previously, the header line would stay around even when after
disabling which-function-mode, although it may be empty.  Now the
which-function-mode element is properly removed from
header-line-format, so the header line will disappear if there's
nothing else in header-line-format.

Also, previously, when we ran (which-function-mode), we would enable
which-function-mode for all buffers even if they didn't support imenu.
We didn't run the normal logic in which-func-ff-hook to disable
which-func-mode if imenu wasn't present.  Now we do run that logic, by
just calling which-func-ff-hook.  This is especially important when
the header line is enabled, because otherwise there's a very
noticeable header line added to every buffer, including e.g. *Help*
and *Buffer List*.

Also, we now check that header-line-format is a list before trying to
add to it; this makes us work properly when enabling and disabling
which-function-mode for modes which set header-line-format to a string
or symbol, such as eww.

* lisp/progmodes/which-func.el (which-func-try-to-enable): Re-add
which-func-format to the header line.
(which-func--header-line-remove): Add.
(which-func--disable): Call which-func--header-line-remove.
(which-function-mode): Call which-func-ff-hook and
which-func--header-line-remove. (bug#66283)
* test/lisp/progmodes/which-func-tests.el: Add.
---
 lisp/progmodes/which-func.el            | 39 ++++++++++-------
 test/lisp/progmodes/which-func-tests.el | 58 +++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 15 deletions(-)
 create mode 100644 test/lisp/progmodes/which-func-tests.el

diff --git a/lisp/progmodes/which-func.el b/lisp/progmodes/which-func.el
index 09d0250515f..0e04bab6ea4 100644
--- a/lisp/progmodes/which-func.el
+++ b/lisp/progmodes/which-func.el
@@ -208,21 +208,28 @@ which-func--use-mode-line
 (add-hook 'after-change-major-mode-hook #'which-func-ff-hook t)
 
 (defun which-func-try-to-enable ()
-  (unless (or (not which-function-mode)
-              (local-variable-p 'which-func-mode))
-    (setq which-func-mode (or (eq which-func-modes t)
-                              (member major-mode which-func-modes)))
-    (setq which-func--use-mode-line
-          (member which-func-display '(mode mode-and-header)))
-    (setq which-func--use-header-line
-          (member which-func-display '(header mode-and-header)))
-    (when (and which-func-mode which-func--use-header-line)
+  (when which-function-mode
+    (unless (local-variable-p 'which-func-mode)
+      (setq which-func-mode (or (eq which-func-modes t)
+                                (member major-mode which-func-modes)))
+      (setq which-func--use-mode-line
+            (member which-func-display '(mode mode-and-header)))
+      (setq which-func--use-header-line
+            (member which-func-display '(header mode-and-header))))
+    ;; We might need to re-add which-func-format to the header line,
+    ;; if which-function-mode was toggled off and on.
+    (when (and which-func-mode which-func--use-header-line
+               (listp header-line-format))
       (add-to-list 'header-line-format '("" which-func-format " ")))))
 
-(defun which-func--disable ()
-  (when (and which-func-mode which-func--use-header-line)
+(defun which-func--header-line-remove ()
+  (when (and which-func-mode which-func--use-header-line
+             (listp header-line-format))
     (setq header-line-format
-          (delete '("" which-func-format " ") header-line-format)))
+          (delete '("" which-func-format " ") header-line-format))))
+
+(defun which-func--disable ()
+  (which-func--header-line-remove)
   (setq which-func-mode nil))
 
 (defun which-func-ff-hook ()
@@ -288,9 +295,11 @@ which-function-mode
   (when which-function-mode
     ;;Turn it on.
     (setq which-func-update-timer
-          (run-with-idle-timer idle-update-delay t #'which-func-update))
-    (dolist (buf (buffer-list))
-      (with-current-buffer buf (which-func-try-to-enable)))))
+          (run-with-idle-timer idle-update-delay t #'which-func-update)))
+  (dolist (buf (buffer-list))
+    (with-current-buffer buf
+      (which-func--header-line-remove)
+      (which-func-ff-hook))))
 
 (defvar which-function-imenu-failed nil
   "Locally t in a buffer if `imenu--make-index-alist' found nothing there.")
diff --git a/test/lisp/progmodes/which-func-tests.el b/test/lisp/progmodes/which-func-tests.el
new file mode 100644
index 00000000000..73709f1c5e5
--- /dev/null
+++ b/test/lisp/progmodes/which-func-tests.el
@@ -0,0 +1,58 @@
+;;; which-func-tests.el --- tests for which-func     -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2023 Free Software Foundation, Inc.
+
+;; Author: Spencer Baugh <sbaugh@catern.com>
+
+;; This file is part of GNU Emacs.
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+(require 'ert)
+(require 'which-func)
+
+(ert-deftest which-func-tests-toggle ()
+  (let ((which-func-display 'mode-and-header) buf-code buf-not)
+    (setq buf-code (find-file-noselect "which-func-tests.el"))
+    (setq buf-not (get-buffer-create "fundamental"))
+    (with-current-buffer buf-code
+      (should-not which-func-mode) (should-not header-line-format))
+    (with-current-buffer buf-not
+      (should-not which-func-mode) (should-not header-line-format))
+    (which-function-mode 1)
+    (with-current-buffer buf-code
+      (should which-func-mode) (should header-line-format))
+    (with-current-buffer buf-not
+      (should-not which-func-mode) (should-not header-line-format))
+    (which-function-mode -1)
+    ;; which-func-mode stays set even when which-function-mode is off.
+    (with-current-buffer buf-code
+      (should which-func-mode) (should-not header-line-format))
+    (with-current-buffer buf-not
+      (should-not which-func-mode) (should-not header-line-format))
+    (kill-buffer buf-code)
+    (kill-buffer buf-not)
+    (which-function-mode 1)
+    (setq buf-code (find-file-noselect "which-func-tests.el"))
+    (setq buf-not (get-buffer-create "fundamental"))
+    (with-current-buffer buf-code
+      (should which-func-mode) (should header-line-format))
+    (with-current-buffer buf-not
+      (should-not which-func-mode) (should-not header-line-format))))
+
+(provide 'which-func-tests)
+;;; which-func-tests.el ends here
-- 
2.41.0