bug#39179: 27.0.50; [PATCH] Add filter to gdb-mi register buffer

[Yuan Fu <casouri@gmail.com>]

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



> On Jan 31, 2020, at 4:56 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Sat, 18 Jan 2020 15:51:31 -0500
>> 
>> On startup, there will be a button “[filter off]” on the header line
>> of the register buffer. Clicking on it enables the register filter,
>> changes the button to “[filter on]” and adds a “[-|+]” button next to
>> it. Click “+” to add patterns to the pattern list, click “-” to
>> remove. Register whose name matches any pattern in the list is
>> displayed. You can also use key “f” for toggle, “+” to add pattern,
>> “-” to remove pattern.
> 
> I'm not sure it's a good idea to implement this as a button on the
> header-line.  Such buttons are for frequent operations, and also have
> the disadvantage of being unavailable on TTY frames.  What are the
> chances users will need to redefine the register patters frequently
> enough to justify the button?  Wouldn't it be much easier to have a
> defcustom whose value users could interactively modify as needed?

I mainly use it to display only the registers I care about, say, all the *dx registers (rdx, edx, dx) or all the r** registers (rdx, rsi, etc). And that depends on the program you are working on. The main motivation behind this patch is that, currently the register buffer simply displays all the possible registers (153 on my machine), and tracking on some of them is very hard (scrolling back and forth, very annoying).

So this feather is a session-based quick filtering for interesting registers, I don’t think defcustom makes it better.

As for the buttons, I mimicked the buttons on memory buffer. And you don’r really need to use these buttons, instead of clicking this buttons, I just hit -/+/f key (since register buffer is a special buffer) and it’s convenient:

>> You can also use key “f” for toggle, “+” to add pattern,
>> “-” to remove pattern.


---------------------------------

> And please don't use non-ASCII characters in log messages, as these
> could cause trouble reading the Git log on less capable terminals.
> 
>> * lisp/progmodes/gdb-mi.el (gdb-registers-enable-filter,
>> gdb-registers-filter-pattern-list, gdb-header-click-event-handler,
>> gdb-registers-add-to-display, gdb-registers-remove-from-display,
>> gdb-registers-toggle-filter): new
>                                ^^^
> "New functions."
> 
>> (gdb-registers-handler-custom): condition check before adding each
>> register
>> (gdb-registers-mode-map): add new keys
>> (gdb-registers-header): add new buttons
> 
> Please start description of changes with a capitalized word, and end
> with a period -- these should be complete English sentences.
> 
>> +;; automatically local because we don't want filters persist across gdb sessions
> 
> Likewise in comments: complete sentences (here and elsewhere in the
> patch).
> 
> Thanks.

I’ve fixed these bits, here is the revised patch.

Yuan


[-- Attachment #2: register-fixed.patch --]
[-- Type: application/octet-stream, Size: 9024 bytes --]

From dcd9d640b4fa32560e40ea5e733392c4d19ff130 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sun, 6 Oct 2019 14:10:14 -0400
Subject: [PATCH] Add register filters to gdb-mi

Add filtering feature to register buffer of gdb-mi.
User can add or remove registers they want to see.

On startup, there will be a button "[filter off]" on the header line
of the register buffer. Clicking on it enables the register filter,
changes the button to "[filter on]" and adds a "[-|+]" button next to
it. Click "+" to add patterns to the pattern list, click "-" to
remove. Register whose name matches any pattern in the list is
displayed. You can also use key "f" for toggle, "+" to add pattern,
"-" to remove pattern.

* lisp/progmodes/gdb-mi.el (gdb-registers-enable-filter,
gdb-registers-filter-pattern-list, gdb-header-click-event-handler,
gdb-registers-add-to-display, gdb-registers-remove-from-display,
gdb-registers-toggle-filter): New functions.
(gdb-registers-handler-custom): Condition check before adding each
register.
(gdb-registers-mode-map): New keys.
(gdb-registers-header): New buttons.
---
 lisp/progmodes/gdb-mi.el | 137 +++++++++++++++++++++++++++++++++++----
 1 file changed, 125 insertions(+), 12 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..aafdbad2c9 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -535,6 +535,24 @@ gdb-show-threads-by-default
   :group 'gdb-buffers
   :version "23.2")
 
+;; Automatically local because we don't want filters persist across gdb sessions.
+(defcustom gdb-registers-enable-filter nil
+  "If t, enable register name filter in register buffer."
+  :local t
+  :type 'boolean
+  :group 'gdb-buffers
+  :version "27.1")
+
+(defcustom gdb-registers-filter-pattern-list nil
+  "Registers that are displayed in register buffer.
+
+Should be a list.  Registers whose name can match
+any of the regexps in the list is displayed."
+  :local t
+  :type 'list
+  :group 'gdb-buffers
+  :version "28.1")
+
 (defvar gdb-debug-log nil
   "List of commands sent to and replies received from GDB.
 Most recent commands are listed first.  This list stores only the last
@@ -4238,6 +4256,53 @@ gdb-frame-locals-buffer
  'gdb-registers-mode
  'gdb-invalidate-registers)
 
+(defun gdb-header-click-event-handler (function)
+  "Return a function that handles clicking event on gdb header buttons.
+
+This function switches to the window where the header locates and
+executes FUNCTION."
+  (lambda (event)
+    (interactive "e")
+    (save-selected-window
+      ;; Make sure we are in the right buffer.
+      (select-window (posn-window (event-start event)))
+      (funcall function))))
+
+(defun gdb-registers-add-to-display ()
+  "Add register to display in register buffer.
+
+Prompt for a register pattern.  The pattern should be a regexp
+pattern matching the name of the register(s) you want to
+display."
+  (interactive)
+  (let ((register (completing-read "Register pattern: "
+                                   gdb-register-names)))
+    (cl-pushnew register gdb-registers-filter-pattern-list)
+    ;; Update register buffer.
+    (gdb-invalidate-registers 'update)))
+
+(defun gdb-registers-remove-from-display ()
+  "Remove register from display in register buffer.
+
+Prompt for a register pattern.  The pattern should be a pattern
+you want to remove from the existing patterns."
+  (interactive)
+  (let ((register (completing-read "Register pattern: "
+                                   gdb-registers-filter-pattern-list
+                                   nil t))) ; require match
+    (setq gdb-registers-filter-pattern-list
+          (remove register gdb-registers-filter-pattern-list))
+    ;; Update register buffer.
+    (gdb-invalidate-registers 'update)))
+
+(defun gdb-registers-toggle-filter ()
+  "Toggle register filter."
+  (interactive)
+  (setq gdb-registers-enable-filter
+        (not gdb-registers-enable-filter))
+  ;; Update register buffer.
+  (gdb-invalidate-registers 'update))
+
 (defun gdb-registers-handler-custom ()
   (when gdb-register-names
     (let ((register-values
@@ -4248,17 +4313,24 @@ gdb-registers-handler-custom
                (value (bindat-get-field register 'value))
                (register-name (nth (string-to-number register-number)
                                    gdb-register-names)))
-          (gdb-table-add-row
-           table
-           (list
-            (propertize register-name
-                        'font-lock-face font-lock-variable-name-face)
-            (if (member register-number gdb-changed-registers)
-                (propertize value 'font-lock-face font-lock-warning-face)
-              value))
-           `(mouse-face highlight
-                        help-echo "mouse-2: edit value"
-                        gdb-register-name ,register-name))))
+          ;; Add register if `gdb-display-these-registers' is t;
+          ;; or any pattern that `gdb-display-these-registers' matches.
+          (when (or (not gdb-registers-enable-filter)
+                    (cl-loop for pattern in gdb-registers-filter-pattern-list
+                             if (string-match pattern register-name)
+                             return t
+                             finally return nil))
+            (gdb-table-add-row
+             table
+             (list
+              (propertize register-name
+                          'font-lock-face font-lock-variable-name-face)
+              (if (member register-number gdb-changed-registers)
+                  (propertize value 'font-lock-face font-lock-warning-face)
+                value))
+             `(mouse-face highlight
+                          help-echo "mouse-2: edit value"
+                          gdb-register-name ,register-name)))))
       (insert (gdb-table-string table " ")))
     (setq mode-name
           (gdb-current-context-mode-name "Registers"))))
@@ -4287,6 +4359,9 @@ gdb-registers-mode-map
                             (gdb-get-buffer-create
                              'gdb-locals-buffer
                              gdb-thread-number) t)))
+    (define-key map "+" #'gdb-registers-add-to-display)
+    (define-key map "-" #'gdb-registers-remove-from-display)
+    (define-key map "f" #'gdb-registers-toggle-filter)
     map))
 
 (defvar gdb-registers-header
@@ -4296,7 +4371,45 @@ gdb-registers-header
                           mode-line-inactive)
    " "
    (gdb-propertize-header "Registers" gdb-registers-buffer
-			  nil nil mode-line)))
+			  nil nil mode-line)
+
+   '(:eval (if (not gdb-registers-enable-filter)
+               (propertize " [filter off]"
+                           'face 'shadow
+                           'help-echo "mouse-1: toggle filter"
+                           'mouse-face 'mode-line-highlight
+                           'local-map (gdb-make-header-line-mouse-map
+                                       'mouse-1
+                                       (gdb-header-click-event-handler
+                                        #'gdb-registers-toggle-filter)))
+             (concat ; enable filter
+              (propertize " [filter on]"
+                          'face '(:weight bold :inherit success)
+                          'help-echo "mouse-1: toggle filter"
+                          'mouse-face 'mode-line-highlight
+                          'local-map (gdb-make-header-line-mouse-map
+                                      'mouse-1
+                                      (gdb-header-click-event-handler
+                                       #'gdb-registers-toggle-filter)))
+              " ["
+              (propertize "-"
+                          'face 'font-lock-warning-face
+                          'help-echo "mouse-1: remove register pattern from display filter"
+                          'mouse-face 'mode-line-highlight
+                          'local-map (gdb-make-header-line-mouse-map
+                                      'mouse-1
+                                      (gdb-header-click-event-handler
+                                       #'gdb-registers-remove-from-display)))
+              "|"
+              (propertize "+"
+                          'face 'font-lock-warning-face
+                          'help-echo "mouse-1: add register pattern to display filter"
+                          'mouse-face 'mode-line-highlight
+                          'local-map (gdb-make-header-line-mouse-map
+                                      'mouse-1
+                                      (gdb-header-click-event-handler
+                                       #'gdb-registers-add-to-display)))
+              "]")))))
 
 (define-derived-mode gdb-registers-mode gdb-parent-mode "Registers"
   "Major mode for gdb registers."
-- 
2.25.0


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