bug#73675: [PATCH] Clean up tmm.el

bug#73675: [PATCH] Clean up tmm.el

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Tags: patch

Hi,

Here is a patch with some clean up in "tmm.el".

Best regards,

In GNU Emacs 31.0.50 (build 15, x86_64-unknown-openbsd7.6, X toolkit) of
 2024-10-07 built on computer
Repository revision: 8c5d69998e65d3ecf5f599bd828bf3330f4f118a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: OpenBSD computer 7.6 GENERIC.MP#344 amd64

Configured using:
 'configure CC=egcc CPPFLAGS=-I/usr/local/include
 LDFLAGS=-L/usr/local/lib MAKEINFO=gmakeinfo --prefix=/home/manuel/emacs
 --bindir=/home/manuel/bin --with-x-toolkit=lucid
 --with-toolkit-scroll-bars=no --without-cairo
 --without-compress-install'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clean-up-tmm.el.patch --]
[-- Type: text/patch, Size: 3299 bytes --]

From 2acf1e10828b760b87be3791014451cb666336c3 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Mon, 7 Oct 2024 10:52:03 +0200
Subject: [PATCH] Clean up tmm.el

* lisp/tmm.el (tmm-old-mb-map, tmm-mb-map, tmm-prompt): Remove
unused keymap variables.
(tmm-define-keys, tmm-add-prompt): Remove always true parameter.
---
 lisp/tmm.el | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/lisp/tmm.el b/lisp/tmm.el
index ed74c307009..46919a08b96 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -37,7 +37,6 @@ tmm
 
 ;;; The following will be localized, added only to pacify the compiler.
 (defvar tmm-short-cuts)
-(defvar tmm-old-mb-map nil)
 (defvar tmm-c-prompt nil)
 (defvar tmm-km-list)
 (defvar tmm-next-shortcut-digit)
@@ -82,9 +81,6 @@ tmm-mid-prompt
   :type '(choice (const :tag "No shortcuts" nil)
                  string))
 
-(defvar tmm-mb-map nil
-  "A place to store minibuffer map.")
-
 (defcustom tmm-completion-prompt
   "Press PageUp key to reach this buffer from the minibuffer.
 Alternatively, you can use Up/Down keys (or your History keys) to change
@@ -146,7 +142,7 @@ tmm-prompt
   (let ((gl-str "Menu bar")  ;; The menu bar itself is not a menu keymap
 					; so it doesn't have a name.
 	tmm-km-list out history-len tmm-table-undef tmm-c-prompt
-	tmm-old-mb-map tmm-short-cuts
+	tmm-short-cuts
 	chosen-string choice
 	(not-menu (not (keymapp menu))))
     (run-hooks 'activate-menubar-hook)
@@ -314,8 +310,7 @@ tmm-add-one-shortcut
                     str)
             (cdr elt))))))
 
-;; This returns the old map.
-(defun tmm-define-keys (minibuffer)
+(defun tmm-define-keys ()
   (let ((map (make-sparse-keymap)))
     (suppress-keymap map t)
     (dolist (c tmm-short-cuts)
@@ -325,16 +320,14 @@ tmm-define-keys
         ;; downcase input to the same
         (define-key map (char-to-string (downcase c)) 'tmm-shortcut)
         (define-key map (char-to-string (upcase c)) 'tmm-shortcut)))
-    (if minibuffer
-	(progn
-          (define-key map [pageup] 'tmm-goto-completions)
-          (define-key map [prior] 'tmm-goto-completions)
-          (define-key map "\ev" 'tmm-goto-completions)
-          (define-key map "\C-n" 'next-history-element)
-          (define-key map "\C-p" 'previous-history-element)
-          (define-key map "^" 'self-insert-and-exit)))
-    (prog1 (current-local-map)
-      (use-local-map (append map (current-local-map))))))
+    (define-key map "\C-p" 'previous-history-element)
+    (define-key map "\C-n" 'next-history-element)
+    (define-key map "\ev" 'tmm-goto-completions)
+    (define-key map [prior] 'tmm-goto-completions)
+    (define-key map [pageup] 'tmm-goto-completions)
+    ;; Previous menu shortcut (see `tmm-prompt')
+    (define-key map "^" 'self-insert-and-exit)
+    (use-local-map (append map (current-local-map)))))
 
 (defun tmm-completion-delete-prompt ()
   (with-current-buffer standard-output
@@ -374,7 +367,7 @@ tmm-remove-inactive-mouse-face
 (defun tmm-add-prompt ()
   (unless tmm-c-prompt
     (error "No active menu entries"))
-  (setq tmm-old-mb-map (tmm-define-keys t))
+  (tmm-define-keys)
   (or tmm-completion-prompt
       (add-hook 'completion-setup-hook
                 #'tmm-completion-delete-prompt 'append))
-- 
2.46.1


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#73675: [PATCH] Clean up tmm.el

[Eli Zaretskii]

> Date: Mon, 07 Oct 2024 10:59:16 +0200
> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Here is a patch with some clean up in "tmm.el".

Thanks, but these are public symbols, how do we know no one out there
uses them?  I'd leave them alone, TBH.

bug#73675: [PATCH] Clean up tmm.el

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Mon, 07 Oct 2024 10:59:16 +0200
>> From:  Manuel Giraud via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> Here is a patch with some clean up in "tmm.el".
>
> Thanks, but these are public symbols, how do we know no one out there
> uses them?  I'd leave them alone, TBH.

Ok.  What about the following one?  `tmm-mb-map' is not even set
anywhere.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clean-up-tmm.el.patch --]
[-- Type: text/x-patch, Size: 1893 bytes --]

From dc27a84ef0f387566966fbbe2c9ccc41d1f639cd Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Mon, 7 Oct 2024 10:52:03 +0200
Subject: [PATCH] Clean up tmm.el

* lisp/tmm.el (tmm-mb-map): Remove unused keymap variable.
---
 lisp/tmm.el | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/lisp/tmm.el b/lisp/tmm.el
index ed74c307009..582551cfb87 100644
--- a/lisp/tmm.el
+++ b/lisp/tmm.el
@@ -82,9 +82,6 @@ tmm-mid-prompt
   :type '(choice (const :tag "No shortcuts" nil)
                  string))
 
-(defvar tmm-mb-map nil
-  "A place to store minibuffer map.")
-
 (defcustom tmm-completion-prompt
   "Press PageUp key to reach this buffer from the minibuffer.
 Alternatively, you can use Up/Down keys (or your History keys) to change
@@ -325,14 +322,14 @@ tmm-define-keys
         ;; downcase input to the same
         (define-key map (char-to-string (downcase c)) 'tmm-shortcut)
         (define-key map (char-to-string (upcase c)) 'tmm-shortcut)))
-    (if minibuffer
-	(progn
-          (define-key map [pageup] 'tmm-goto-completions)
-          (define-key map [prior] 'tmm-goto-completions)
-          (define-key map "\ev" 'tmm-goto-completions)
-          (define-key map "\C-n" 'next-history-element)
-          (define-key map "\C-p" 'previous-history-element)
-          (define-key map "^" 'self-insert-and-exit)))
+    (when minibuffer
+      (define-key map [pageup] 'tmm-goto-completions)
+      (define-key map [prior] 'tmm-goto-completions)
+      (define-key map "\ev" 'tmm-goto-completions)
+      (define-key map "\C-n" 'next-history-element)
+      (define-key map "\C-p" 'previous-history-element)
+      ;; Previous menu shortcut (see `tmm-prompt')
+      (define-key map "^" 'self-insert-and-exit))
     (prog1 (current-local-map)
       (use-local-map (append map (current-local-map))))))
 
-- 
2.46.1


[-- Attachment #3: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

bug#73675: [PATCH] Clean up tmm.el

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: 73675@debbugs.gnu.org
> Date: Mon, 07 Oct 2024 14:05:24 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, but these are public symbols, how do we know no one out there
> > uses them?  I'd leave them alone, TBH.
> 
> Ok.  What about the following one?  `tmm-mb-map' is not even set
> anywhere.

That's okay, but if you intend to work on improving and developing
tmm.el (which is always welcome), we prefer that such cleanup changes
be done as part of larger, significant changesets, not as separate
changes that modify the code without adding any new or improved
functionality.

In a nutshell, this is like fixing whitespace: we prefer to do it as
part of real changes, not separately.

So if you intend to work on tmm.el, I suggest to install this with
whatever other changes you are planning.

Thanks.

bug#73675: [PATCH] Clean up tmm.el

[Manuel Giraud via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Manuel Giraud <manuel@ledu-giraud.fr>
>> Cc: 73675@debbugs.gnu.org
>> Date: Mon, 07 Oct 2024 14:05:24 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Thanks, but these are public symbols, how do we know no one out there
>> > uses them?  I'd leave them alone, TBH.
>> 
>> Ok.  What about the following one?  `tmm-mb-map' is not even set
>> anywhere.
>
> That's okay, but if you intend to work on improving and developing
> tmm.el (which is always welcome), we prefer that such cleanup changes
> be done as part of larger, significant changesets, not as separate
> changes that modify the code without adding any new or improved
> functionality.

Ok.  I should have done this in my previous patch :-) I may still have
some work on tmm.el.  Anyway, I'm closing this one.
-- 
Manuel Giraud