Subject: Re: bug#49632: [PATCH] Ensure that M-x gdb populates gud-repeat-map

From: Brian Leung <[email-masked]>
Date: Tue, 27 Jul 2021 06:09:14 +0200 (CEST)

> Is there a reason why gud-set-repeat-map-property should be a macro,
> but not a function?  When it's a macro, there are problems such that
> when the user modifies the map, then the function that uses the macro
> needs to be recompiled.

When I wrote initially wrote gud-set-repeat-map-property, I couldn't figure out how to make gud-set-repeat-map-property work as a function since the repeat-map property needed to be a symbol corresponding to a keymap, and not an evaluated keymap.

I've updated the patch with a change to the repeat-post-hook function in repeat.el to accept an evaluated keymap, which permits a function version of gud-set-repeat-map-property.

> ----------------------------------------
> From: Juri Linkov <[email-masked]>
> Sent: Tue Jul 27 00:48:20 CEST 2021
> To: Brian Leung <[email-masked]>
> Cc: Lars Ingebrigtsen <[email-masked]>, <[email-masked]>
> Subject: Re: bug#49632: [PATCH] Ensure that M-x gdb populates gud-repeat-map
> 
> 
> >> There is one thing that I don't understand: is the
> >> helper function gud-set-repeat-map-property really needed?
> >>
> >> If e.g. the defvar gud-gdb-repeat-map uses such symbols as 'gud-next',
> >> could it put the 'repeat-map' property on the same symbols
> >> with reference to the variable 'map' directly in these defvars?
> >> Then only defvars will be needed, and no changes in gud-gdb, gdb.
> >
> > If we assign the repeat-map property within the defvars and avoid doing so
> > in the M-x gdb commands, then after gud.el gets loaded, the repeat-map
> > property on (say) gud-next will be 'jdb, since jdb-repeat-map is the last
> > such foo-repeat-map defvar appearing in gud.el. In that case, calling M-x
> > perldb and then running gud-next when repeat-mode is on will bring up the
> > jdb-repeat-map, which contains some commands that aren't defined for
> > perldb. So I think the gud-set-repeat-map-property macro (or something like
> > it) is necessary.
> 
> You are right.  Then I have only minor comments:
> 
> 1. I thought that you want to create two separate maps for gud-gdb and gdb.
> This is the reason why I suggested to rename gud-repeat-map to gud-gdb-repeat-map.
> But since you use only one map for gbd and gud-gdb, then the name
> gud-repeat-map is fine.
> 
> 2. Is there a reason why gud-set-repeat-map-property should be a macro,
> but not a function?  When it's a macro, there are problems such that
> when the user modifies the map, then the function that uses the macro
> needs to be recompiled.

-- 
Sent with https://mailfence.com
Secure and private email

-- 
Mailfence.com
Private and secure email
>From d12a32b225c364aa75708c766370e8f27a08995a Mon Sep 17 00:00:00 2001
From: Brian Leung <[email-masked]>
Date: Mon, 19 Jul 2021 23:41:01 -0700
Subject: [PATCH 1/2] Ensure that gud commands for M-x gdb are handled by
 repeat-mode

* lisp/progmodes/gud.el (gud-gdb-repeat-map): Rename from
gud-repeat-map, and populate at the top-level.
(gud-set-repeat-map-property): Introduce this helper function for
setting the repeat-map property.
(gud-gdb): Use the gud-set-repeat-map-property function to assign the
repeat-map property.

* lisp/progmodes/gdb-mi.el (gdb): Use the gud-set-repeat-map-property
function to assign the repeat-map property.

Because different debugging tools may not support all of the gud-foo
functions, we reassign the repeat-map property within the respective
commands, as opposed to the top level of the files, to ensure that the
repeat-map property is reassigned each time to a symbol corresponding
to the active debugging tool.

* lisp/repeat.el (repeat-post-hook): Allow the repeat-map property to
be an evaluated keymap.
---
 lisp/progmodes/gdb-mi.el |  2 ++
 lisp/progmodes/gud.el    | 33 +++++++++++++++++++++------------
 lisp/repeat.el           |  3 ++-
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 57c99abec6..ad579eb89f 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -996,6 +996,8 @@ gdb
   (define-key gud-minor-mode-map [left-margin C-mouse-3]
     'gdb-mouse-jump)
 
+  (gud-set-repeat-map-property gud-gdb-repeat-map)
+
   (setq-local gud-gdb-completion-function 'gud-gdbmi-completions)
 
   (add-hook 'completion-at-point-functions #'gud-gdb-completion-at-point
diff --git a/lisp/progmodes/gud.el b/lisp/progmodes/gud.el
index 740a6e2581..3ffae7f2e0 100644
--- a/lisp/progmodes/gud.el
+++ b/lisp/progmodes/gud.el
@@ -320,10 +320,28 @@ gud-tool-bar-map
       (tool-bar-local-item-from-menu
        (car x) (cdr x) map gud-minor-mode-map))))
 
-(defvar gud-repeat-map (make-sparse-keymap)
-  "Keymap to repeat gud stepping instructions `C-x C-a C-n n n'.
+(defvar gud-gdb-repeat-map
+  (let ((map (make-sparse-keymap)))
+    (pcase-dolist (`(,key . ,cmd) '(("n" . gud-next)
+                                    ("s" . gud-step)
+                                    ("i" . gud-stepi)
+                                    ("c" . gud-cont)
+                                    ("l" . gud-refresh)
+                                    ("f" . gud-finish)
+                                    ("<" . gud-up)
+                                    (">" . gud-down)))
+      (define-key map key cmd))
+    map)
+  "Keymap to repeat `gud-gdb' stepping instructions `C-x C-a C-n n n'.
 Used in `repeat-mode'.")
 
+(defun gud-set-repeat-map-property (keymap)
+  "Set the `repeat-map' property of each command in KEYMAP to KEYMAP."
+  (map-keymap-internal (lambda (_ cmd)
+                          (put cmd 'repeat-map keymap))
+                        keymap))
+
+
 (defun gud-file-name (f)
   "Transform a relative file name to an absolute file name.
 Uses `gud-<MINOR-MODE>-directories' to find the source files."
@@ -814,16 +832,7 @@ gud-gdb
   (gud-def gud-until  "until %l" "\C-u" "Continue to current line.")
   (gud-def gud-run    "run"	 nil    "Run the program.")
 
-  (dolist (cmd '(("n" . gud-next)
-                 ("s" . gud-step)
-                 ("i" . gud-stepi)
-                 ("c" . gud-cont)
-                 ("l" . gud-refresh)
-                 ("f" . gud-finish)
-                 ("<" . gud-up)
-                 (">" . gud-down)))
-    (define-key gud-repeat-map (car cmd) (cdr cmd))
-    (put (cdr cmd) 'repeat-map 'gud-repeat-map))
+  (gud-set-repeat-map-property gud-gdb-repeat-map)
 
   (add-hook 'completion-at-point-functions #'gud-gdb-completion-at-point
             nil 'local)
diff --git a/lisp/repeat.el b/lisp/repeat.el
index cec3cb643a..aa2c3a0779 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -410,7 +410,8 @@ repeat-post-hook
                          (and (symbolp real-this-command)
                               (get real-this-command 'repeat-map)))))
         (when rep-map
-          (when (boundp rep-map)
+          (when (and (boundp rep-map)
+                     (not (keymapp rep-map)))
             (setq rep-map (symbol-value rep-map)))
           (let ((map (copy-keymap rep-map)))
 
-- 
2.32.0

>From 1bc33064c86a7dbeb0330f9aedaa23dfc35495bb Mon Sep 17 00:00:00 2001
From: Brian Leung <[email-masked]>
Date: Tue, 20 Jul 2021 00:32:34 -0700
Subject: [PATCH 2/2] Ensure that gud commands for non-GDB debuggers are
 handled by repeat-mode

* lisp/progmodes/gud.el (sdb-repeat-map): Define.
(sdb): Set repeat-mode property to the symbol corresponding to the
repeat map.
(dbx-repeat-map): Define.
(dbx): Set repeat-mode property to the symbol corresponding to the
repeat map.
(xdb-repeat-map): Define.
(xdb): Set repeat-mode property to the symbol corresponding to the
repeat map.
(perldb-repeat-map): Define.
(perldb): Set repeat-mode property to the symbol corresponding to the
repeat map.
(pdb-repeat-map): Define.
(pdb): Set repeat-mode property to the symbol corresponding to the
repeat map.
(guiler-repeat-map): Define.
(guiler): Set repeat-mode property to the symbol corresponding to the
repeat map.
(jdb-repeat-map): Define.
(jdb): Set repeat-mode property to the symbol corresponding to the
repeat map.
---
 lisp/progmodes/gud.el | 110 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 110 insertions(+)

diff --git a/lisp/progmodes/gud.el b/lisp/progmodes/gud.el
index 3ffae7f2e0..950ee356f4 100644
--- a/lisp/progmodes/gud.el
+++ b/lisp/progmodes/gud.el
@@ -1033,6 +1033,18 @@ gud-sdb-needs-tags
 
 (defvar gud-sdb-lastfile nil)
 
+(defvar sdb-repeat-map
+  (let ((map (make-sparse-keymap)))
+    (pcase-dolist (`(,key . ,cmd) '(("n" . gud-next)
+                                    ("s" . gud-step)
+                                    ("i" . gud-stepi)
+                                    ("c" . gud-cont)
+                                    ("l" . gud-refresh)))
+      (define-key map key cmd))
+    map)
+  "Keymap to repeat `sdb' stepping instructions `C-x C-a C-n n n'.
+Used in `repeat-mode'.")
+
 (defun gud-sdb-marker-filter (string)
   (setq gud-marker-acc
 	(if gud-marker-acc (concat gud-marker-acc string) string))
@@ -1103,6 +1115,8 @@ sdb
   (gud-def gud-cont   "c"    "\C-r"   "Continue with display.")
   (gud-def gud-print  "%e/"  "\C-p"   "Evaluate C expression at point.")
 
+  (gud-set-repeat-map-property sdb-repeat-map)
+
   (setq comint-prompt-regexp  "\\(^\\|\n\\)\\*")
   (setq paragraph-start comint-prompt-regexp)
   (run-hooks 'sdb-mode-hook)
@@ -1261,6 +1275,23 @@ gud-dbx-use-stopformat-p
 ;; whereby `set $stopformat=1' reportedly produces output compatible
 ;; with `gud-dbx-marker-filter', which we prefer.
 
+(defvar dbx-repeat-map
+  (let ((map (make-sparse-keymap)))
+    (pcase-dolist (`(,key . ,cmd) '(("n" . gud-next)
+                                    ("s" . gud-step)
+                                    ("i" . gud-stepi)
+                                    ("c" . gud-cont)
+                                    ("l" . gud-refresh)
+                                    ("<" . gud-up)
+                                    (">" . gud-down)))
+      (define-key map key cmd))
+    (when (or gud-mips-p
+              gud-irix-p)
+      (define-key map "f" 'gud-finish))
+    map)
+  "Keymap to repeat `dbx' stepping instructions `C-x C-a C-n n n'.
+Used in `repeat-mode'.")
+
 ;; The process filter is also somewhat
 ;; unreliable, sometimes not spotting the markers; I don't know
 ;; whether there's anything that can be done about that.]
@@ -1408,6 +1439,8 @@ dbx
   (gud-def gud-print  "print %e"  "\C-p" "Evaluate C expression at point.")
   (gud-def gud-run    "run"	     nil    "Run the program.")
 
+  (gud-set-repeat-map-property dbx-repeat-map)
+
   (setq comint-prompt-regexp  "^[^)\n]*dbx) *")
   (setq paragraph-start comint-prompt-regexp)
   (run-hooks 'dbx-mode-hook)
@@ -1419,6 +1452,21 @@ dbx
 ;; History of argument lists passed to xdb.
 (defvar gud-xdb-history nil)
 
+(defvar xdb-repeat-map
+  (let ((map (make-sparse-keymap)))
+    (pcase-dolist (`(,key . ,cmd) '(("n" . gud-next)
+                                    ("s" . gud-step)
+                                    ("i" . gud-stepi)
+                                    ("c" . gud-cont)
+                                    ("l" . gud-refresh)
+                                    ("f" . gud-finish)
+                                    ("<" . gud-up)
+                                    (">" . gud-down)))
+      (define-key map key cmd))
+    map)
+  "Keymap to repeat `xdb' stepping instructions `C-x C-a C-n n n'.
+Used in `repeat-mode'.")
+
 (defcustom gud-xdb-directories nil
   "A list of directories that xdb should search for source code.
 If nil, only source files in the program directory
@@ -1484,6 +1532,8 @@ xdb
   (gud-def gud-finish "bu\\t"      "\C-f" "Finish executing current function.")
   (gud-def gud-print  "p %e"       "\C-p" "Evaluate C expression at point.")
 
+  (gud-set-repeat-map-property xdb-repeat-map)
+
   (setq comint-prompt-regexp  "^>")
   (setq paragraph-start comint-prompt-regexp)
   (run-hooks 'xdb-mode-hook))
@@ -1494,6 +1544,17 @@ xdb
 ;; History of argument lists passed to perldb.
 (defvar gud-perldb-history nil)
 
+(defvar perldb-repeat-map
+  (let ((map (make-sparse-keymap)))
+    (pcase-dolist (`(,key . ,cmd) '(("n" . gud-next)
+                                    ("s" . gud-step)
+                                    ("c" . gud-cont)
+                                    ("l" . gud-refresh)))
+      (define-key map key cmd))
+    map)
+  "Keymap to repeat `perldb' stepping instructions `C-x C-a C-n n n'.
+Used in `repeat-mode'.")
+
 (defun gud-perldb-massage-args (_file args)
   "Convert a command line as would be typed normally to run perldb
 into one that invokes an Emacs-enabled debugging session.
@@ -1636,6 +1697,7 @@ perldb
   (gud-def gud-print  "p %e"          "\C-p" "Evaluate perl expression at point.")
   (gud-def gud-until  "c %l"          "\C-u" "Continue to current line.")
 
+  (gud-set-repeat-map-property perldb-repeat-map)
 
   (setq comint-prompt-regexp "^  DB<+[0-9]+>+ ")
   (setq paragraph-start comint-prompt-regexp)
@@ -1664,6 +1726,20 @@ gud-pdb-marker-regexp-fnname-group
 
 (defvar gud-pdb-marker-regexp-start "^> ")
 
+(defvar pdb-repeat-map
+  (let ((map (make-sparse-keymap)))
+    (pcase-dolist (`(,key . ,cmd) '(("n" . gud-next)
+                                    ("s" . gud-step)
+                                    ("c" . gud-cont)
+                                    ("l" . gud-refresh)
+                                    ("f" . gud-finish)
+                                    ("<" . gud-up)
+                                    (">" . gud-down)))
+      (define-key map key cmd))
+    map)
+  "Keymap to repeat `pdb' stepping instructions `C-x C-a C-n n n'.
+Used in `repeat-mode'.")
+
 ;; There's no guarantee that Emacs will hand the filter the entire
 ;; marker at once; it could be broken up across several strings.  We
 ;; might even receive a big chunk with several markers in it.  If we
@@ -1753,6 +1829,8 @@ pdb
   (gud-def gud-print  "p %e"         "\C-p" "Evaluate Python expression at point.")
   (gud-def gud-statement "!%e"      "\C-e" "Execute Python statement at point.")
 
+  (gud-set-repeat-map-property pdb-repeat-map)
+
   ;; (setq comint-prompt-regexp "^(.*pdb[+]?) *")
   (setq comint-prompt-regexp "^(Pdb) *")
   (setq paragraph-start comint-prompt-regexp)
@@ -1766,6 +1844,19 @@ gud-guiler-history
 
 (defvar gud-guiler-lastfile nil)
 
+(defvar guiler-repeat-map
+  (let ((map (make-sparse-keymap)))
+    (pcase-dolist (`(,key . ,cmd) '(("n" . gud-next)
+                                    ("s" . gud-step)
+                                    ("l" . gud-refresh)
+                                    ("f" . gud-finish)
+                                    ("<" . gud-up)
+                                    (">" . gud-down)))
+      (define-key map key cmd))
+    map)
+  "Keymap to repeat `guiler' stepping instructions `C-x C-a C-n n n'.
+Used in `repeat-mode'.")
+
 (defun gud-guiler-marker-filter (string)
   (setq gud-marker-acc (if gud-marker-acc (concat gud-marker-acc string) string))
 
@@ -1831,6 +1922,8 @@ guiler
   (gud-def gud-down   ",down"         ">" "Down one stack frame.")
   (gud-def gud-print  "%e"            "\C-p" "Evaluate Guile expression at point.")
 
+  (gud-set-repeat-map-property guiler-repeat-map)
+
   (setq comint-prompt-regexp "^scheme@([^>]+> ")
   (setq paragraph-start comint-prompt-regexp)
   (run-hooks 'guiler-mode-hook))
@@ -2276,6 +2369,21 @@ gud-jdb-find-source-file
 ;; Note: Reset to this value every time a prompt is seen
 (defvar gud-jdb-lowest-stack-level 999)
 
+(defvar jdb-repeat-map
+  (let ((map (make-sparse-keymap)))
+    (pcase-dolist (`(,key . ,cmd) '(("n" . gud-next)
+                                    ("s" . gud-step)
+                                    ("i" . gud-stepi)
+                                    ("c" . gud-cont)
+                                    ("f" . gud-finish)
+                                    ("<" . gud-up)
+                                    (">" . gud-down)
+                                    ("l" . gud-refresh)))
+      (define-key map key cmd))
+    map)
+  "Keymap to repeat `jdb' stepping instructions `C-x C-a C-n n n'.
+Used in `repeat-mode'.")
+
 (defun gud-jdb-find-source-using-classpath (p)
   "Find source file corresponding to fully qualified class P.
 Convert P from jdb's output, converted to a pathname
@@ -2484,6 +2592,8 @@ jdb
   (gud-def gud-print  "print %e"  "\C-p" "Print value of expression at point.")
   (gud-def gud-pstar  "dump %e"  nil "Print all object information at point.")
 
+  (gud-set-repeat-map-property jdb-repeat-map)
+
   (setq comint-prompt-regexp "^> \\|^[^ ]+\\[[0-9]+\\] ")
   (setq paragraph-start comint-prompt-regexp)
   (run-hooks 'jdb-mode-hook)
-- 
2.32.0