Extend gdb to filter registers

Extend gdb to filter registers

[Yuan Fu]

[-- Attachment #1.1: Type: text/plain, Size: 869 bytes --]

Hi,

Currently the register buffer of gdb in Emacs displays all the registers.
It would be nice if we can select some registers and ask gdb to only
display these. I added this feature in the following patch. For you
convenience I didn't modify the Emacs source but created a new file so you
can just load it and see the effect.

With the extension we can use `gdb-registers-add-to-display' to add
registers to display. And use `gdb-registers-remove-from-display' to remove
registers. Please see more details in the docstring. Change will be visible
in the next update, e.g. after "si" in gdb.

This is just a demo and there are many parts to complete, e.g., naming of
interactive commands, change defvar to defcustom, docsting wording, update
registers immediatly after `add-to-/remove-from-display*', *etc. I want to
know what do you think before going further.

Yuan

[-- Attachment #1.2: Type: text/html, Size: 1684 bytes --]

[-- Attachment #2: gdb-ext.el --]
[-- Type: text/x-emacs-lisp, Size: 3542 bytes --]

;;; -*- lexical-binding: t -*-

;;; Register filter

(defvar gdb-display-these-registers t
  "Registers that are displayed in register buffer.
Can be a list, a function or t/nil.
If a list, registers which names can be found in the list are displayed.
If a function, it is passed with a register name and should
return t to display and nil to not display.
If t/nil, display all registers / none of the registers.")

(defun gdb-registers-add-to-display (register)
  "Add REGISTER to display in register buffer.
REGISTER is a string representing the name of the register.
By default the register buffer displays all the registers.
REGISTER can also be 'all, which configures gdb to display all registers.
Also see `gdb-registers-remove-from-display'. "
  (interactive (list (completing-read "Register: " (append gdb-register-names
                                                           '("all")))))
  (if (equal register "all")
      (setq gdb-display-these-registers t)
    (if (listp gdb-display-these-registers)
        (add-to-list 'gdb-display-these-registers register)
      (setq gdb-display-these-registers (list register)))))

(defun gdb-registers-remove-from-display (register)
  "Remove REGISTER from display in register buffer.
REGISTER is a string representing the name of the register.
By default the register buffer displays all the registers.
REGISTER can also be 'all, which configures gdb to hide all registers.
Also see `gdb-registers-add-to-display'."
  (interactive (list (completing-read "Register: " (append gdb-register-names
                                                           '("all")))))
  (if (equal register "all")
      (setq gdb-display-these-registers nil)
    (if (listp gdb-display-these-registers)
        (setq gdb-display-these-registers
              (remove register gdb-display-these-registers))
      (user-error "gdb-display-these-registers is not a list, can’t remove anything from it"))))

(defun gdb-registers-handler-custom ()
  (when gdb-register-names
    (let ((register-values
           (bindat-get-field (gdb-json-partial-output) 'register-values))
          (table (make-gdb-table)))
      (dolist (register register-values)
        (let* ((register-number (bindat-get-field register 'number))
               (value (bindat-get-field register 'value))
               (register-name (nth (string-to-number register-number)
                                   gdb-register-names)))
          (when (cond ((and (listp gdb-display-these-registers)
                            gdb-display-these-registers) ; not nil
                       (message register-name)
                       (member register-name gdb-display-these-registers))
                      ((functionp gdb-display-these-registers)
                       (funcall gdb-display-these-registers register-name))
                      (t gdb-display-these-registers))
            (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"))))

;;; dynamic memory watch

Re: Extend gdb to filter registers

[Fu Yuan]

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

Hi all,

I posted this yesterday and no one seems interested. I wonder if there are problems with the post. Anything is welcome, thanks.

Yuan

> 在 2019年10月3日，下午12:40，Yuan Fu <casouri@gmail.com> 写道：
> 
> Hi,
> 
> Currently the register buffer of gdb in Emacs displays all the registers. It would be nice if we can select some registers and ask gdb to only display these. I added this feature in the following patch. For you convenience I didn't modify the Emacs source but created a new file so you can just load it and see the effect.
> 
> With the extension we can use `gdb-registers-add-to-display' to add registers to display. And use `gdb-registers-remove-from-display' to remove registers. Please see more details in the docstring. Change will be visible in the next update, e.g. after "si" in gdb.
> 
> This is just a demo and there are many parts to complete, e.g., naming of interactive commands, change defvar to defcustom, docsting wording, update registers immediatly after `add-to-/remove-from-display', etc. I want to know what do you think before going further. 
> 
> Yuan
> 
> 
> <gdb-ext.el>

[-- Attachment #2: Type: text/html, Size: 2244 bytes --]

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Fu Yuan <casouri@gmail.com>
> Date: Fri, 4 Oct 2019 12:13:39 -0400
> 
> I posted this yesterday and no one seems interested. I wonder if there are problems with the post. Anything is
> welcome, thanks.

One day without responses is definitely not long enough to decide that
no one is interested.  Please wait at least a week before pinging.

Thanks.

Re: Extend gdb to filter registers

[Stefan Monnier]

> This is just a demo and there are many parts to complete, e.g., naming of
> interactive commands, change defvar to defcustom, docsting wording, update
> registers immediatly after `add-to-/remove-from-display*', *etc. I want to
> know what do you think before going further.

I basically never do register-level debugging, so it's not something I'd
need, but it sounds like a fine feature.  I can't really comment on the
code, because it's hard to know how it would affect the existing code
without doing to work of turning your code into a patch.


        Stefan

Re: Extend gdb to filter registers

[Yuan Fu]

[-- Attachment #1.1: Type: text/plain, Size: 301 bytes --]

> Please wait at least a week before pinging.

Got it.

> I basically never do register-level debugging, so it's not something I'd
need, but it sounds like a fine feature.

Cool, here is the patch version. I try to mimic what other people do
with patches and commit, etc. I hope this look good.

Yuan

[-- Attachment #1.2: Type: text/html, Size: 1296 bytes --]

[-- Attachment #2: 0001-Add-filter-to-regiter-display-in-gdb-mi.patch --]
[-- Type: text/x-patch, Size: 4995 bytes --]

From 4add6897230037d5a68e763ea81ae526facea8f0 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Fri, 4 Oct 2019 17:34:28 -0400
Subject: [PATCH] Add filter to regiter display in gdb-mi

lisp/progmode/gdb-mi.el (gdb-display-these-registers,
  gdb-registers-add-to-display, gdb-registers-remove-from-display): new
  (gdb-registers-handler-custom): add conditional check before adding
  each registers to table
---
 lisp/progmodes/gdb-mi.el | 65 +++++++++++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 11 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..95f831d36c 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -200,6 +200,13 @@ gdb-error
 (defvar gdb-macro-info nil
   "Non-nil if GDB knows that the inferior includes preprocessor macro info.")
 (defvar gdb-register-names nil "List of register names.")
+(defvar gdb-display-these-registers t
+  "Registers that are displayed in register buffer.
+Can be a list, a function or t/nil.
+If a list, registers which names can be found in the list are displayed.
+If a function, it is passed with a register name and should
+return t to display and nil to not display.
+If t/nil, display all registers / none of the registers.")
 (defvar gdb-changed-registers nil
   "List of changed register numbers (strings).")
 (defvar gdb-buffer-fringe-width nil)
@@ -4238,6 +4245,35 @@ gdb-frame-locals-buffer
  'gdb-registers-mode
  'gdb-invalidate-registers)
 
+(defun gdb-registers-add-to-display (register)
+  "Add REGISTER to display in register buffer.
+REGISTER is a string representing the name of the register.
+By default the register buffer displays all the registers.
+REGISTER can also be 'all, which configures gdb to display all registers.
+Also see `gdb-registers-remove-from-display'. "
+  (interactive (list (completing-read "Register: " (append gdb-register-names
+                                                           '("all")))))
+  (if (equal register "all")
+      (setq gdb-display-these-registers t)
+    (if (listp gdb-display-these-registers)
+        (add-to-list 'gdb-display-these-registers register)
+      (setq gdb-display-these-registers (list register)))))
+
+(defun gdb-registers-remove-from-display (register)
+  "Remove REGISTER from display in register buffer.
+REGISTER is a string representing the name of the register.
+By default the register buffer displays all the registers.
+REGISTER can also be 'all, which configures gdb to hide all registers.
+Also see `gdb-registers-add-to-display'."
+  (interactive (list (completing-read "Register: " (append gdb-register-names
+                                                           '("all")))))
+  (if (equal register "all")
+      (setq gdb-display-these-registers nil)
+    (if (listp gdb-display-these-registers)
+        (setq gdb-display-these-registers
+              (remove register gdb-display-these-registers))
+      (user-error "gdb-display-these-registers is not a list, can’t remove anything from it"))))
+
 (defun gdb-registers-handler-custom ()
   (when gdb-register-names
     (let ((register-values
@@ -4248,17 +4284,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))))
+          (when (cond ((and (listp gdb-display-these-registers)
+                            gdb-display-these-registers) ; not nil
+                       (message register-name)
+                       (member register-name gdb-display-these-registers))
+                      ((functionp gdb-display-these-registers)
+                       (funcall gdb-display-these-registers register-name))
+                      (t gdb-display-these-registers))
+            (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"))))
-- 
2.23.0

Re: Extend gdb to filter registers

[Yuan Fu]

[-- Attachment #1.1: Type: text/plain, Size: 445 bytes --]

Please use this revised patch instead, sorry.

Yuan



On Fri, Oct 4, 2019 at 5:55 PM Yuan Fu <casouri@gmail.com> wrote:

> > Please wait at least a week before pinging.
>
> Got it.
>
> > I basically never do register-level debugging, so it's not something I'd
> need, but it sounds like a fine feature.
>
> Cool, here is the patch version. I try to mimic what other people do
> with patches and commit, etc. I hope this look good.
>
> Yuan
>
>

[-- Attachment #1.2: Type: text/html, Size: 2042 bytes --]

[-- Attachment #2: 0001-Add-filter-to-regiter-display-in-gdb-mi.patch --]
[-- Type: text/x-patch, Size: 4947 bytes --]

From 42c61fb1dbb005871c6d0896e6e85f29e0494f6d Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Fri, 4 Oct 2019 17:34:28 -0400
Subject: [PATCH] Add filter to regiter display in gdb-mi

lisp/progmode/gdb-mi.el (gdb-display-these-registers,
  gdb-registers-add-to-display, gdb-registers-remove-from-display): new
  (gdb-registers-handler-custom): add conditional check before adding
  each registers to table
---
 lisp/progmodes/gdb-mi.el | 64 +++++++++++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..5b7e3321f3 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -200,6 +200,13 @@ gdb-error
 (defvar gdb-macro-info nil
   "Non-nil if GDB knows that the inferior includes preprocessor macro info.")
 (defvar gdb-register-names nil "List of register names.")
+(defvar gdb-display-these-registers t
+  "Registers that are displayed in register buffer.
+Can be a list, a function or t/nil.
+If a list, registers which names can be found in the list are displayed.
+If a function, it is passed with a register name and should
+return t to display and nil to not display.
+If t/nil, display all registers / none of the registers.")
 (defvar gdb-changed-registers nil
   "List of changed register numbers (strings).")
 (defvar gdb-buffer-fringe-width nil)
@@ -4238,6 +4245,35 @@ gdb-frame-locals-buffer
  'gdb-registers-mode
  'gdb-invalidate-registers)
 
+(defun gdb-registers-add-to-display (register)
+  "Add REGISTER to display in register buffer.
+REGISTER is a string representing the name of the register.
+By default the register buffer displays all the registers.
+REGISTER can also be 'all, which configures gdb to display all registers.
+Also see `gdb-registers-remove-from-display'. "
+  (interactive (list (completing-read "Register: " (append gdb-register-names
+                                                           '("all")))))
+  (if (equal register "all")
+      (setq gdb-display-these-registers t)
+    (if (listp gdb-display-these-registers)
+        (add-to-list 'gdb-display-these-registers register)
+      (setq gdb-display-these-registers (list register)))))
+
+(defun gdb-registers-remove-from-display (register)
+  "Remove REGISTER from display in register buffer.
+REGISTER is a string representing the name of the register.
+By default the register buffer displays all the registers.
+REGISTER can also be 'all, which configures gdb to hide all registers.
+Also see `gdb-registers-add-to-display'."
+  (interactive (list (completing-read "Register: " (append gdb-register-names
+                                                           '("all")))))
+  (if (equal register "all")
+      (setq gdb-display-these-registers nil)
+    (if (listp gdb-display-these-registers)
+        (setq gdb-display-these-registers
+              (remove register gdb-display-these-registers))
+      (user-error "gdb-display-these-registers is not a list, can’t remove anything from it"))))
+
 (defun gdb-registers-handler-custom ()
   (when gdb-register-names
     (let ((register-values
@@ -4248,17 +4284,23 @@ 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))))
+          (when (cond ((and (listp gdb-display-these-registers)
+                            gdb-display-these-registers) ; not nil
+                       (member register-name gdb-display-these-registers))
+                      ((functionp gdb-display-these-registers)
+                       (funcall gdb-display-these-registers register-name))
+                      (t gdb-display-these-registers))
+            (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"))))
-- 
2.23.0

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Fri, 4 Oct 2019 23:15:31 -0400
> Cc: Emacs developers <emacs-devel@gnu.org>
> 
> Please use this revised patch instead, sorry.

Thanks, this is large enough to need a copyright assignment before we
can accept the changes.  Would you like to start your legal paperwork
rolling?  If so, I will send you the form to fill.

Re: Extend gdb to filter registers

[Yuan Fu]

> Thanks, this is large enough to need a copyright assignment before we
> can accept the changes.  Would you like to start your legal paperwork
> rolling?  If so, I will send you the form to fill.

Yes, please send it to me.

Yuan

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sat, 5 Oct 2019 09:15:22 -0400
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>  emacs-devel@gnu.org
> 
> > Thanks, this is large enough to need a copyright assignment before we
> > can accept the changes.  Would you like to start your legal paperwork
> > rolling?  If so, I will send you the form to fill.
> 
> Yes, please send it to me.

Than ks, form sent off-list.

Re: Extend gdb to filter registers

[Yuan Fu]

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

Here are some changes I want to make, I'll prepare some patch files
for you to look at hopefully soon enough:

* make gdb-mi support register filter in register buffer (the first patch)

* make gdb-mi support multiple memory display, so that I can monitor
  different memory locations at the same time

* make gdb-mi support expressions as memory location, e.g., $rsp, x+1.
  This mimics the display command of gdb, e.g. display/5gx $rsp.
  Currently, gdb-mi does supports expressions for setting memory
  monitors but translates it to a _fixed_ value; and the memory
  monitor doesn't reflect the chagne of the expression's value. For
  example, it does't follow $rsp and update address as $rsp chganges

* make gdb-mi support multiple gdb sessions at the same time.

Some questions:

* Is there any test that I can check my changes again to?

* Is the last one (multiple gdb sessions) neccessary? Have there been
  requests for it?
  

[-- Attachment #2: Type: text/html, Size: 2891 bytes --]

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sat, 5 Oct 2019 13:51:05 -0400
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>  emacs-devel@gnu.org
> 
> * make gdb-mi support multiple memory display, so that I can monitor
>   different memory locations at the same time
> 
> * make gdb-mi support expressions as memory location, e.g., $rsp, x+1.
>   This mimics the display command of gdb, e.g. display/5gx $rsp.
>   Currently, gdb-mi does supports expressions for setting memory
>   monitors but translates it to a _fixed_ value; and the memory
>   monitor doesn't reflect the chagne of the expression's value. For
>   example, it does't follow $rsp and update address as $rsp chganges
> 
> * make gdb-mi support multiple gdb sessions at the same time.

Sound useful features to me, thanks.

> * Is there any test that I can check my changes again to?

Unfortunately, no.  You will get bonus points for starting one.

> * Is the last one (multiple gdb sessions) neccessary? Have there been
>   requests for it?

It sounds a bit obscure, and I don't think I;ve ever heard a request
for that.  So we could leave this out for now.

Thanks.

Re: Extend gdb to filter registers

[Yuan Fu]

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

Thanks for your kind reply, Eli. Here are some more questions:

* If I found a problem in existing code that is unrelated to my new
  feature but is quite trivial to fix (2~3 lines), should I create a
  new branch and send it as an separate patch to bug tracker, or
  include it in my new feature?

* IIUC, I send the patch to the bug tracker and people will review it,
  is that correct?

* If the patch is a fix, should I first create a bug report and reply
  to that report?

I'm still not quite sure how everything works, hope I'm not too
garrulous.

Attached is the patch for the bug fix. Changes are:

1. gdb-mi uses a buffer to store and parse output from gdb process
   and it is cleared after each complete conversation, however
   the cleanup function is not protected against call to `error'
   in handlers who process the output's content and do various things to it.

2. gdb-mi uses `error' for error messages from gdb, I think it should
   use `user-error' instead because the gdb-mi code is running well,
   the error is from gdb and are normal errors like "symbol not
   found in current context". We shouldn't drop a user into a backtrace
   for errors like this
   
Yuan


[-- Attachment #2: unwind-protect.patch --]
[-- Type: application/octet-stream, Size: 2321 bytes --]

From 1bbfaa19081c34167722b0806550696f219e261b Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sat, 5 Oct 2019 23:29:17 -0400
Subject: [PATCH 1/2] Add unwind-protect to error handling function

lisp/progmodes/gdb-mi.el (gdb-done-or-error): add unwind-protect
---
 lisp/progmodes/gdb-mi.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..042c49c737 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -2644,13 +2644,13 @@ gdb-done-or-error
     ;; We are done concatenating to the output sink.  Restore it to user sink:
     (setq gdb-output-sink 'user)
 
-    (when (and token-number is-complete)
-      (with-current-buffer
-	  (gdb-get-buffer-create 'gdb-partial-output-buffer)
-	(gdb-handle-reply (string-to-number token-number))))
-
-  (when is-complete
-    (gdb-clear-partial-output))))
+    (unwind-protect (when (and token-number is-complete)
+                      (with-current-buffer
+	                  (gdb-get-buffer-create 'gdb-partial-output-buffer)
+	                (gdb-handle-reply (string-to-number token-number))))
+      ;; protect against handler-emitted errors
+      (when is-complete
+        (gdb-clear-partial-output)))))
 
 (defun gdb-concat-output (so-far new)
   (cond
-- 
2.23.0


From d76fdd533594153e144f78d47cdb6c2968041a99 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sat, 5 Oct 2019 23:39:05 -0400
Subject: [PATCH 2/2] Change error to user-error

lisp/progmodes/gdb-mi.el
  (gdb-read-memory-custom): Change error to user-error
---
 lisp/progmodes/gdb-mi.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 042c49c737..1bc451df8e 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -3500,7 +3500,8 @@ gdb-read-memory-custom
       (progn
         (let ((gdb-memory-address gdb-memory-last-address))
           (gdb-invalidate-memory 'update)
-          (error err-msg))))))
+          ;; an error from gdb should be considered a user error
+          (user-error err-msg))))))
 
 (defvar gdb-memory-mode-map
   (let ((map (make-sparse-keymap)))
-- 
2.23.0

Re: Extend gdb to filter registers

[Michael Welsh Duggan]

Yuan Fu <casouri@gmail.com> writes:

> * Is the last one (multiple gdb sessions) neccessary? Have there been
>   requests for it?

I live by it.  I can still do multiple gdb session using M-x gud-gdb.
This is the sole blocker that prevents me from using gdb-mi.

-- 
Michael Welsh Duggan
(md5i@md5i.com)

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 6 Oct 2019 00:24:26 -0400
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>  emacs-devel@gnu.org
> 
> * If I found a problem in existing code that is unrelated to my new
>   feature but is quite trivial to fix (2~3 lines), should I create a
>   new branch and send it as an separate patch to bug tracker, or
>   include it in my new feature?

It should be a separate patch, yes.  Whether to make a separate branch
for it is entirely up to you.

> * IIUC, I send the patch to the bug tracker and people will review it,
>   is that correct?

Yes.  use "M-x report-emacs-bug RET" for that.

> * If the patch is a fix, should I first create a bug report and reply
>   to that report?

You can include the patch in the original message you send with
report-emacs-bug, and that will create a bug for you.

> I'm still not quite sure how everything works, hope I'm not too
> garrulous.

No sweat.  CONTRIBUTE should have some answers for your questions.

> Attached is the patch for the bug fix. Changes are:

I will review it soon if no one beats me to it.

Thanks.

Re: Extend gdb to filter registers

[Yuan Fu]

Michael Welsh Duggan <mwd@md5i.com> writes:

> I live by it.  I can still do multiple gdb session using M-x gud-gdb.
> This is the sole blocker that prevents me from using gdb-mi.

Could you describe why do you debug two programs in the same time?

Yuan

Re: Extend gdb to filter registers

[Michael Welsh Duggan]

Yuan Fu <casouri@gmail.com> writes:

> Michael Welsh Duggan <mwd@md5i.com> writes:
>
>> I live by it.  I can still do multiple gdb session using M-x gud-gdb.
>> This is the sole blocker that prevents me from using gdb-mi.
>
> Could you describe why do you debug two programs in the same time?

There are two circumstances, one for which it is necessary, and the
other of which for which it is not.

The one for which it is necessary is when I am debugging both sides of a
client/server protocol, and I want to see what both the client and the
server are doing.

In the not-as-necessary but I do it all the time category, I often am
working on several programs at a time (I do a lot of infrastructure
maintenance on my team) and have to switch from debugging one to
debugging another but would prefer to not lose my prior debugging state
when doing so.

For both these I could run a separate emacs, but I am generally in the
"one long-running emacs" camp, as I know is not uncommon in this group.

-- 
Michael Welsh Duggan
(mwd@cert.org)

Re: Extend gdb to filter registers

[Yuan Fu]

Michael Welsh Duggan <mwd@cert.org> writes:

> The one for which it is necessary is when I am debugging both sides of a
> client/server protocol, and I want to see what both the client and the
> server are doing.
>
> In the not-as-necessary but I do it all the time category, I often am
> working on several programs at a time (I do a lot of infrastructure
> maintenance on my team) and have to switch from debugging one to
> debugging another but would prefer to not lose my prior debugging state
> when doing so.
>
> For both these I could run a separate emacs, but I am generally in the
> "one long-running emacs" camp, as I know is not uncommon in this group.

Thanks. That sounds reasonable. I’ll look into it once I finished other
features.

Re: Extend gdb to filter registers

[Yuan Fu]

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

If anyone is interested, please try these out:

1. register.patch

Adds register filters to register buffer. There will be [-|+] buttons
on the header line of the register buffer. Click them and chose what to
include/exclude.

2. memory.patch

Adds support for multiple memory buffers. Add support for expressions as
memory address. Call out the first memory buffer by
‘gdb-display-memory-buffer’ like before, call out additional ones by
‘gdb-display-new-memory-buffer’. To use an expression as address, click
on the address button like before and enter expressions like x, x + 1,
$rsp, etc. gdb-mi will display an error if the expression you entered is
invalid or it can’t read memory. You need to also apply the
unwind-protect patch for this to work properly.


3. unwind-protect.patch

Fixes a bug where one error could mess up the whole gdb-mi session. The
previous unwind-protect.patch doesn’t solve the bug fully and this one
should.

Any comment is welcome. Thanks.

Yuan


[-- Attachment #2: register.patch --]
[-- Type: text/x-patch, Size: 7677 bytes --]

From 5010df825853b0d393f74388371d2bb0e3d5c723 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.

* lisp/progmodes/gdb-mi.el (gdb-display-these-registers,
(gdb-registers-add-to-display, gdb-registers-remove-from-display): new
(gdb-registers-handler-custom): add condition check before adding
register to display
(gdb-registers-mode-map): add "+" and "-" keybinding
(gdb-registers-header): add "[-|+]" button
---
 lisp/progmodes/gdb-mi.el | 111 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 99 insertions(+), 12 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..6b9f448dbe 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -200,6 +200,13 @@ gdb-error
 (defvar gdb-macro-info nil
   "Non-nil if GDB knows that the inferior includes preprocessor macro info.")
 (defvar gdb-register-names nil "List of register names.")
+(defvar-local gdb-display-these-registers t
+  "Registers that are displayed in register buffer.
+Can be a list, a function or t/nil.
+If a list, registers which names can be found in the list are displayed.
+If a function, it is passed with a register name and should
+return t to display and nil to not display.
+If t/nil, display all registers / none of the registers.")
 (defvar gdb-changed-registers nil
   "List of changed register numbers (strings).")
 (defvar gdb-buffer-fringe-width nil)
@@ -4238,6 +4245,62 @@ gdb-frame-locals-buffer
  'gdb-registers-mode
  'gdb-invalidate-registers)
 
+(defun gdb-registers-add-to-display (event)
+  "Add register to display in register buffer.
+
+Register is a string representing the name of the register.
+By default the register buffer displays all the registers.
+register can also be 'all, which configures gdb to display all registers.
+Also see `gdb-registers-remove-from-display'.
+
+This function is intended to be used from clicking header line
+of register buffer, hence the EVENT."
+  (interactive "e")
+  (save-selected-window
+    ;; make sure we are in the right buffer
+    (select-window (posn-window (event-start event)))
+    (let ((register (completing-read "Register: "
+                                     (append gdb-register-names
+                                             '("all")))))
+      ;; if `gdb-display-these-registers' is a function,
+      ;; just override it with list
+      (if (equal register "all")
+          (setq gdb-display-these-registers t)
+        (if (listp gdb-display-these-registers)
+            (add-to-list 'gdb-display-these-registers register)
+          (setq gdb-display-these-registers (list register))))
+      ;; update register buffer
+      (gdb-invalidate-registers 'update))))
+
+(defun gdb-registers-remove-from-display (event)
+  "Remove register from display in register buffer.
+
+Register is a string representing the name of the register.
+By default the register buffer displays all the registers.
+REGISTER can also be 'all, which configures gdb to hide all registers.
+also see `gdb-registers-add-to-display'.
+
+This function is intended to be used from clicking header line
+of register buffer, hence the EVENT."
+  (interactive "e")
+  (save-selected-window
+    ;; make sure we are in the right buffer
+    (select-window (posn-window (event-start event)))
+    (let ((register (completing-read "Register: "
+                                     (append gdb-register-names
+                                             '("all")))))
+      (if (equal register "all")
+          (setq gdb-display-these-registers nil)
+        (cond ((listp gdb-display-these-registers)
+               (setq gdb-display-these-registers
+                     (remove register gdb-display-these-registers)))
+              ((eq gdb-display-these-registers t)
+               (setq gdb-display-these-registers
+                     (remove register gdb-register-names)))
+              (t (user-error "`gdb-display-these-registers' is not a list, can’t remove anything from it"))))
+      ;; update register buffer
+      (gdb-invalidate-registers 'update))))
+
 (defun gdb-registers-handler-custom ()
   (when gdb-register-names
     (let ((register-values
@@ -4248,17 +4311,22 @@ 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))))
+          (when (cond ((listp gdb-display-these-registers) ; consider nil as empty list
+                       (member register-name gdb-display-these-registers))
+                      ((functionp gdb-display-these-registers)
+                       (funcall gdb-display-these-registers register-name))
+                      (t gdb-display-these-registers))
+            (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 +4355,8 @@ 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)
     map))
 
 (defvar gdb-registers-header
@@ -4296,7 +4366,24 @@ gdb-registers-header
                           mode-line-inactive)
    " "
    (gdb-propertize-header "Registers" gdb-registers-buffer
-			  nil nil mode-line)))
+			  nil nil mode-line)
+   " ["
+   (propertize "-"
+               'face font-lock-warning-face
+               'help-echo "mouse-1: remove register from display filter"
+               'mouse-face 'mode-line-highlight
+               'local-map (gdb-make-header-line-mouse-map
+                           'mouse-1
+                           #'gdb-registers-remove-from-display))
+   "|"
+   (propertize "+"
+               'face font-lock-warning-face
+               'help-echo "mouse-1: add register to display filter"
+               'mouse-face 'mode-line-highlight
+               'local-map (gdb-make-header-line-mouse-map
+                           'mouse-1
+                           #'gdb-registers-add-to-display))
+   "]"))
 
 (define-derived-mode gdb-registers-mode gdb-parent-mode "Registers"
   "Major mode for gdb registers."
-- 
2.23.0


[-- Attachment #3: memory.patch --]
[-- Type: text/x-patch, Size: 13243 bytes --]

From f6db7853075e048affc245f07e7a4c6153e71237 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sat, 5 Oct 2019 13:53:24 -0400
Subject: [PATCH 1/5] Allow multiple memory buffers to co-exist
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

lisp/progmodes/gdb-mi.el (gdb-memory-address, gdb-memory-next-page,
  gdb-memory-prev-page): change to buffer local.
  (gdb-update-buffer-name, gdb-get-buffer-create): allow
    ‘rename-buffer’ to generate unique new buffer names
  (gdb-get-buffer-create): Add an option to force create new buffer
  (gdb-display-new-memory-buffer): new
---
 lisp/progmodes/gdb-mi.el | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..8235c7d2ee 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -105,12 +105,12 @@ tool-bar-map
 (defvar speedbar-initial-expansion-list-name)
 (defvar speedbar-frame)
 
-(defvar	gdb-memory-address "main")
-(defvar	gdb-memory-last-address nil
+(defvar-local gdb-memory-address "main")
+(defvar-local gdb-memory-last-address nil
   "Last successfully accessed memory address.")
-(defvar	gdb-memory-next-page nil
+(defvar-local gdb-memory-next-page nil
   "Address of next memory page for program memory buffer.")
-(defvar	gdb-memory-prev-page nil
+(defvar-local gdb-memory-prev-page nil
   "Address of previous memory page for program memory buffer.")
 
 (defvar gdb-thread-number nil
@@ -1421,7 +1421,8 @@ gdb-update-buffer-name
 it in `gdb-buffer-rules'."
   (let ((f (gdb-rules-name-maker (assoc gdb-buffer-type
                                         gdb-buffer-rules))))
-    (when f (rename-buffer (funcall f)))))
+    ;; allow multiple memory buffers to co-exist
+    (when f (rename-buffer (funcall f) t))))
 
 (defun gdb-current-buffer-rules ()
   "Get `gdb-buffer-rules' entry for current buffer type."
@@ -1462,7 +1463,7 @@ gdb-get-buffer
                        (equal gdb-thread-number thread)))
           (throw 'found buffer))))))
 
-(defun gdb-get-buffer-create (buffer-type &optional thread)
+(defun gdb-get-buffer-create (buffer-type &optional thread new)
   "Create a new GDB buffer of the type specified by BUFFER-TYPE.
 The buffer-type should be one of the cars in `gdb-buffer-rules'.
 
@@ -1473,8 +1474,10 @@ gdb-get-buffer-create
 
 If buffer has trigger associated with it in `gdb-buffer-rules',
 this trigger is subscribed to `gdb-buf-publisher' and called with
-'update argument."
-  (or (gdb-get-buffer buffer-type thread)
+'update argument.
+
+If NEW is non-nil, create a new buffer regardless."
+  (or (and (not new) (gdb-get-buffer buffer-type thread))
       (let ((rules (assoc buffer-type gdb-buffer-rules))
             (new (generate-new-buffer "limbo")))
 	(with-current-buffer new
@@ -1487,7 +1490,8 @@ gdb-get-buffer-create
 	    (set (make-local-variable 'gud-minor-mode)
 		 (buffer-local-value 'gud-minor-mode gud-comint-buffer))
 	    (set (make-local-variable 'tool-bar-map) gud-tool-bar-map)
-            (rename-buffer (funcall (gdb-rules-name-maker rules)))
+            ;; allow multiple memory buffers to co-exist
+            (rename-buffer (funcall (gdb-rules-name-maker rules)) t)
 	    (when trigger
               (gdb-add-subscriber gdb-buf-publisher
                                   (cons (current-buffer)
@@ -3786,6 +3790,11 @@ gdb-display-memory-buffer
   (interactive)
   (gdb-display-buffer (gdb-get-buffer-create 'gdb-memory-buffer thread)))
 
+(defun gdb-display-new-memory-buffer (&optional thread)
+  "Display GDB memory contents in a new gdb buffer."
+  (interactive)
+  (gdb-display-buffer (gdb-get-buffer-create 'gdb-memory-buffer thread t)))
+
 (defun gdb-frame-memory-buffer ()
   "Display memory contents in another frame."
   (interactive)
-- 
2.23.0


From 9166f87de72153c71a92c162a1311c31771051ae Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sat, 5 Oct 2019 22:42:07 -0400
Subject: [PATCH 2/5] Enhance support for expressions as memory address
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before the memory buffer evaluates the expression as address
and use the fixed result in each stop. This change stores the
expression itself and reevaluates it in each stop for an address.
Then displays the value of the memory at that address.

lisp/progmodes/gdb-mi.el (gdb-memory-address-expression): new
  (gdb-memory-address): change default value, add docstring
  (def-gdb-trigger-and-handler gdb-invalidate-memory,
  gdb-memory-set-address): replace ’gdb-memory-address’ with
    ’gdb-memory-address-expression’
  (gdb-memory-header): Add display for ’gdb-memory-address-expression’,
    move the mouse event from address to expression
---
 lisp/progmodes/gdb-mi.el | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 8235c7d2ee..0f89422156 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -105,7 +105,11 @@ tool-bar-map
 (defvar speedbar-initial-expansion-list-name)
 (defvar speedbar-frame)
 
-(defvar-local gdb-memory-address "main")
+(defvar-local gdb-memory-address-expression "main"
+  "This expression is passed to gdb.
+Possible value: main, $rsp, x+3.")
+(defvar-local gdb-memory-address nil
+  "Address of memory display.")
 (defvar-local gdb-memory-last-address nil
   "Last successfully accessed memory address.")
 (defvar-local gdb-memory-next-page nil
@@ -3448,7 +3452,7 @@ gdb-memory-unit
 (def-gdb-trigger-and-handler
   gdb-invalidate-memory
   (format "-data-read-memory %s %s %d %d %d"
-          gdb-memory-address
+          (gdb-mi-quote gdb-memory-address-expression)
           gdb-memory-format
           gdb-memory-unit
           gdb-memory-rows
@@ -3538,7 +3542,7 @@ gdb-memory-set-address
   "Set the start memory address."
   (interactive)
   (let ((arg (read-from-minibuffer "Memory address: ")))
-    (setq gdb-memory-address arg))
+    (setq gdb-memory-address-expression arg))
   (gdb-invalidate-memory 'update))
 
 (defmacro def-gdb-set-positive-number (name variable echo-string &optional doc)
@@ -3721,7 +3725,15 @@ gdb-memory-font-lock-keywords
 (defvar gdb-memory-header
   '(:eval
     (concat
-     "Start address["
+     "Start address "
+     (propertize gdb-memory-address-expression
+                 'face font-lock-warning-face
+                 'help-echo "mouse-1: set start address"
+                 'mouse-face 'mode-line-highlight
+                 'local-map (gdb-make-header-line-mouse-map
+                             'mouse-1
+                             #'gdb-memory-set-address-event))
+     " ["
      (propertize "-"
                  'face font-lock-warning-face
                  'help-echo "mouse-1: decrement address"
@@ -3739,12 +3751,7 @@ gdb-memory-header
                              #'gdb-memory-show-next-page))
      "]: "
      (propertize gdb-memory-address
-                 'face font-lock-warning-face
-                 'help-echo "mouse-1: set start address"
-                 'mouse-face 'mode-line-highlight
-                 'local-map (gdb-make-header-line-mouse-map
-                             'mouse-1
-                             #'gdb-memory-set-address-event))
+                 'face font-lock-warning-face)
      "  Rows: "
      (propertize (number-to-string gdb-memory-rows)
                  'face font-lock-warning-face
-- 
2.23.0


From 8166dafcfed0d2d4259232f90d64b1a12a10f840 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Mon, 7 Oct 2019 20:36:23 -0400
Subject: [PATCH 3/5] Fix memory buffer code in gdb-mi
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/progmodes/gdb-mi.el (gdb-read-memory-custom):
Break infinite loop. Change ’error’ to ’user-error’
---
 lisp/progmodes/gdb-mi.el | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 0f89422156..18a33f4933 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -3505,10 +3505,12 @@ gdb-read-memory-custom
                                             gdb-memory-format)))))
             (newline)))
       ;; Show last page instead of empty buffer when out of bounds
-      (progn
-        (let ((gdb-memory-address gdb-memory-last-address))
+      (when gdb-memory-last-address
+        (let ((gdb-memory-address-expression gdb-memory-last-address))
+          ;; avoid infinite loop
+          (setq gdb-memory-last-address nil)
           (gdb-invalidate-memory 'update)
-          (error err-msg))))))
+          (user-error "Error when retrieving memory: %s Displaying last successful page" err-msg))))))
 
 (defvar gdb-memory-mode-map
   (let ((map (make-sparse-keymap)))
-- 
2.23.0


From e4918f6ac82c4e2d1b4a9b860b4520953851f46d Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Mon, 7 Oct 2019 20:52:15 -0400
Subject: [PATCH 4/5] Protect against nil memory address in gdb-mi

* lisp/progmodes/gdb-mi.el (gdb-memory-header):
Protect against nil value
---
 lisp/progmodes/gdb-mi.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 18a33f4933..65abf9d174 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -3728,7 +3728,7 @@ gdb-memory-header
   '(:eval
     (concat
      "Start address "
-     (propertize gdb-memory-address-expression
+     (propertize (or gdb-memory-address-expression "N/A")
                  'face font-lock-warning-face
                  'help-echo "mouse-1: set start address"
                  'mouse-face 'mode-line-highlight
@@ -3752,7 +3752,7 @@ gdb-memory-header
                              'mouse-1
                              #'gdb-memory-show-next-page))
      "]: "
-     (propertize gdb-memory-address
+     (propertize (or gdb-memory-address "N/A")
                  'face font-lock-warning-face)
      "  Rows: "
      (propertize (number-to-string gdb-memory-rows)
-- 
2.23.0


From 479ef100475d453f3fa03d8110263a7c04ab54f5 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Mon, 7 Oct 2019 21:17:01 -0400
Subject: [PATCH 5/5] =?UTF-8?q?Display=20warning=20when=20address=20expres?=
 =?UTF-8?q?sion=20and=20address=20doesn=E2=80=99t=20match?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/progmodes/gdb-mi.el (gdb--memory-display-warning): new
(gdb-read-memory-custom, gdb-memory-header): Add warning
---
 lisp/progmodes/gdb-mi.el | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 65abf9d174..7b3b05a633 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -116,6 +116,12 @@ gdb-memory-next-page
   "Address of next memory page for program memory buffer.")
 (defvar-local gdb-memory-prev-page nil
   "Address of previous memory page for program memory buffer.")
+(defvar-local gdb--memory-display-warning nil
+  "Display warning on memory header if t.
+
+When error occurs when retrieving memory, gdb-mi displays the last
+successful page. In that case the expression might not match the
+memory displayed.")
 
 (defvar gdb-thread-number nil
   "Main current thread.
@@ -3492,6 +3498,9 @@ gdb-read-memory-custom
          (err-msg (bindat-get-field res 'msg)))
     (if (not err-msg)
         (let ((memory (bindat-get-field res 'memory)))
+          (when gdb-memory-last-address
+            ;; nil means last retrieve emits error or just started the session
+            (setq gdb--memory-display-warning nil))
           (setq gdb-memory-address (bindat-get-field res 'addr))
           (setq gdb-memory-next-page (bindat-get-field res 'next-page))
           (setq gdb-memory-prev-page (bindat-get-field res 'prev-page))
@@ -3508,7 +3517,8 @@ gdb-read-memory-custom
       (when gdb-memory-last-address
         (let ((gdb-memory-address-expression gdb-memory-last-address))
           ;; avoid infinite loop
-          (setq gdb-memory-last-address nil)
+          (setq gdb-memory-last-address nil
+                gdb--memory-display-warning t)
           (gdb-invalidate-memory 'update)
           (user-error "Error when retrieving memory: %s Displaying last successful page" err-msg))))))
 
@@ -3735,6 +3745,9 @@ gdb-memory-header
                  'local-map (gdb-make-header-line-mouse-map
                              'mouse-1
                              #'gdb-memory-set-address-event))
+     (if gdb--memory-display-warning
+         (propertize " !" 'face '(:inherit error :weight bold))
+       "")
      " ["
      (propertize "-"
                  'face font-lock-warning-face
-- 
2.23.0


[-- Attachment #4: unwind-protect.patch --]
[-- Type: text/x-patch, Size: 1190 bytes --]

From 9d7dce80836fa39fbd9a75261a2b66c8f8dfb552 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Mon, 7 Oct 2019 18:47:54 -0400
Subject: [PATCH] Protect against errors in handlers in gdb-mi
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/progmodes/gdb-mi.el (gdb-handle-reply):
Handle error in ’handler-function’ so the cleanup code after it
runs safely.
---
 lisp/progmodes/gdb-mi.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..5baf4e1690 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -324,7 +324,10 @@ gdb-handle-reply
 by the reception of this reply."
   (let ((handler-function (gdb-get-handler-function token-number)))
     (when handler-function
-      (funcall handler-function)
+      (condition-case err
+          ;; protect against errors in handler-function
+          (funcall handler-function)
+        (error (message (error-message-string err))))
       (gdb-delete-handler token-number))))
 
 (defun gdb-remove-all-pending-triggers ()
-- 
2.23.0

Re: Extend gdb to filter registers

[Stefan Monnier]

I just took a quick look at your code.  Looks pretty good.
See below the few nitpicks I found.


        Stefan


> diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
> index 60852e4ad6..5b7e3321f3 100644
> --- a/lisp/progmodes/gdb-mi.el
> +++ b/lisp/progmodes/gdb-mi.el
> @@ -200,6 +200,13 @@ gdb-error
>  (defvar gdb-macro-info nil
>    "Non-nil if GDB knows that the inferior includes preprocessor macro info.")
>  (defvar gdb-register-names nil "List of register names.")
> +(defvar gdb-display-these-registers t
> +  "Registers that are displayed in register buffer.
> +Can be a list, a function or t/nil.
> +If a list, registers which names can be found in the list are displayed.
> +If a function, it is passed with a register name and should
> +return t to display and nil to not display.
> +If t/nil, display all registers / none of the registers.")

"which" => "whose"
Note: nil is a list, so the nil case is already covered by the list case.
For the function case, please clarify what the argument will look like
(a symbol or a string?).

Is this expected to be set by the user via `setq`?
If so, maybe it should be a `defcustom`?

> +(defun gdb-registers-add-to-display (register)
> +  "Add REGISTER to display in register buffer.
> +REGISTER is a string representing the name of the register.
> +By default the register buffer displays all the registers.
> +REGISTER can also be 'all, which configures gdb to display all registers.
> +Also see `gdb-registers-remove-from-display'. "
> +  (interactive (list (completing-read "Register: " (append gdb-register-names
> +                                                           '("all")))))
> +  (if (equal register "all")
> +      (setq gdb-display-these-registers t)
> +    (if (listp gdb-display-these-registers)
> +        (add-to-list 'gdb-display-these-registers register)
> +      (setq gdb-display-these-registers (list register)))))

If gdb-display-these-registers is t this will not "add" but remove all
but `register` from the display, which might surprise the user.
Along the same lines, if gdb-display-these-registers is a function this
will either silently throw away the function (if `listp` returns nil) or
build a "broken" list (if `listp` return non-nil, which is very much
possible).

> +(defun gdb-registers-remove-from-display (register)
> +  "Remove REGISTER from display in register buffer.
> +REGISTER is a string representing the name of the register.
> +By default the register buffer displays all the registers.
> +REGISTER can also be 'all, which configures gdb to hide all registers.
> +Also see `gdb-registers-add-to-display'."
> +  (interactive (list (completing-read "Register: " (append gdb-register-names
> +                                                           '("all")))))
> +  (if (equal register "all")
> +      (setq gdb-display-these-registers nil)
> +    (if (listp gdb-display-these-registers)
> +        (setq gdb-display-these-registers
> +              (remove register gdb-display-these-registers))
> +      (user-error "gdb-display-these-registers is not a list, can’t remove anything from it"))))

This may similarly misbehave if gdb-display-these-registers is
a function represented as a list (tho most likely in that case
`register` won't be found in the list, so it will just silently do
nothing).

> +          (when (cond ((and (listp gdb-display-these-registers)
> +                            gdb-display-these-registers) ; not nil
> +                       (member register-name gdb-display-these-registers))

`consp` is a more efficient to test "list and non-nil".
But here, we don't need to avoid the nil case, since (member X nil) will
return nil anyway.


        Stefan

Re: Extend gdb to filter registers

[Yuan Fu]

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

> "which" => "whose"

Fixed

> Is this expected to be set by the user via `setq`?
> If so, maybe it should be a `defcustom`?

Yeah, I made new variables customizable.

> If gdb-display-these-registers is t this will not "add" but remove all
> but `register` from the display, which might surprise the user.
> Along the same lines, if gdb-display-these-registers is a function this
> will either silently throw away the function (if `listp` returns nil) or
> build a "broken" list (if `listp` return non-nil, which is very much
> possible).
> ...
> This may similarly misbehave if gdb-display-these-registers is
> a function represented as a list (tho most likely in that case
> `register` won't be found in the list, so it will just silently do
> nothing).

I think I’ve found a good solution, please see the new patch. I replaced
function with regexp, which is easier to use and should suffice 99% use
cases since all we filtering here are just a few dozen registers.

Quoting the commit message:

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.

WDYT?

Yuan


[-- Attachment #2: 0001-Add-register-filters-to-gdb-mi.patch --]
[-- Type: text/x-patch, Size: 9133 bytes --]

From 11a3557d42cfad8a976e3512839e8927f94f6e5b 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
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
(gdb-registers-handler-custom): condition check before adding each
register
(gdb-registers-mode-map): add new keys
(gdb-registers-header): add 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..dce6d76d1a 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 "27.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 in `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.23.0

Re: Extend gdb to filter registers

[Yuan Fu]

Btw I just realized that I used wrong quotes in the commit message, I’ve
fixed that now.

Yuan

Re: Extend gdb to filter registers

[Yuan Fu]

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

Here is another patch that allows a user to save and restore window
configurations. It seems to work for me and I welcome you to try it out.

Yuan


[-- Attachment #2: 0001-Add-window-configuration-save-restore-feature-for-gd.patch --]
[-- Type: text/x-patch, Size: 7264 bytes --]

From a88ede26b81a1372a74696314fe6c7ed288f730e Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Mon, 14 Oct 2019 21:11:43 -0400
Subject: [PATCH] Add window configuration save/restore feature for gdb-mi
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Now you can save a gdb window configuration to a file with
‘gdb-store-window-configuration’ and restore it from a file
with ‘gdb-restore-window-configuration’.

* lisp/progmodes/gdb-mi.el (require): add pcase
(gdb-buffer-p, gdb-function-buffer-p, gdb--buffer-type,
gdb--inhibit-window-dedicated, gdb-store-window-configuration,
gdb-restore-window-configuration): new
---
 lisp/progmodes/gdb-mi.el | 126 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..4cb09da181 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -92,6 +92,7 @@
 (require 'json)
 (require 'bindat)
 (require 'cl-lib)
+(require 'pcase)
 
 (declare-function speedbar-change-initial-expansion-list
                   "speedbar" (new-default))
@@ -4609,6 +4610,131 @@ gdb-setup-windows
                              nil win5))
     (select-window win0)))
 
+(defun gdb-buffer-p (buffer)
+  "Return t if BUFFER is gdb-related."
+  (with-current-buffer buffer
+    (eq gud-minor-mode 'gdbmi)))
+
+(defun gdb-function-buffer-p (buffer)
+  "Return t if BUFFER is a gdb function buffer.
+
+E.g., locals buffer, registers buffer, but don't include the main
+command buffer (the one in where you type gdb commands) or source
+buffers."
+  (with-current-buffer buffer
+    (when (or (derived-mode-p 'gdb-parent-mode)
+              (derived-mode-p 'gdb-inferior-io-mode)) t)))
+
+(defun gdb--buffer-type (buffer)
+  "Return the buffer type of BUFFER or nil.
+
+Buffer type is like `gdb-registers-type', `gdb-stack-buffer'.
+This symbol can be passed to `gdb-get-buffer-create'.
+
+Return nil if BUFFER isn't a gdb function buffer."
+  (with-current-buffer buffer
+    (cl-loop for rule in gdb-buffer-rules
+             for mode-name = (gdb-rules-buffer-mode rule)
+             for type = (car rule)
+             if (eq mode-name major-mode)
+             return type
+             finally return nil)))
+
+(defmacro gdb--inhibit-window-dedicated (&rest body)
+  "Run BODY with window-dedicated temporarily disabled."
+  ;; switch-to-buffer-in-dedicated-window
+  ;; doesn't work in non-interactive case
+  `(let ((window-dedicated (window-dedicated-p)))
+     (when window-dedicated
+       (set-window-dedicated-p nil nil))
+     ,@body
+     (when window-dedicated
+       (set-window-dedicated-p nil t))))
+
+(defun gdb-store-window-configuration (file)
+  "Save current window configuration to FILE.
+
+You can later restore this configuration from that file by
+`gdb-restore-window-configuration'."
+  (interactive "F Save window configuration to file: ")
+  ;; we replace the buffer in each window with a placeholder, set
+  ;; window parameter as the buffer type (register, breakpoint, etc),
+  ;; and store window configuration
+  (save-window-excursion
+    (let ((placeholder (get-buffer-create " *gdb-placeholder*"))
+          (window-persistent-parameters
+           (cons '(gdb-buffer-type . writable) window-persistent-parameters))
+          window-config
+          old-window-dedidated-value)
+      (dolist (win (window-list nil 'no-minibuffer))
+        (select-window win)
+        (setq old-window-dedidated-value (window-dedicated-p))
+        (when (gdb-buffer-p (current-buffer))
+          (set-window-parameter
+           nil 'gdb-buffer-type
+           (cond ((gdb-function-buffer-p (current-buffer))
+                  ;; we save this gdb-buffer-type symbol so
+                  ;; we can later pass it to `gdb-get-buffer-create'
+                  ;; one example: `gdb-registers-buffer'
+                  (or (gdb--buffer-type (current-buffer))
+                      (error "Unrecognized gdb buffer mode: %s" major-mode)))
+                 ;; command buffer
+                 ((derived-mode-p 'gud-mode) 'command)
+                 ((member (selected-window) gdb-source-window) 'source)))
+          (gdb--inhibit-window-dedicated
+           (set-window-buffer nil placeholder))))
+      ;; save the window configuration to FILE
+      (setq window-config (window-state-get nil t))
+      (let ((buffer (find-file file)))
+        (with-current-buffer buffer
+          (erase-buffer)
+          (prin1 window-config (current-buffer))
+          (save-buffer))
+        (kill-buffer buffer))
+      (kill-buffer placeholder))))
+
+(defun gdb-restore-window-configuration (file)
+  "Restore window configuration from FILE.
+
+FILE is saved by `gdb-store-window-configuration'."
+  (interactive "F Restore window configuration from file: ")
+  ;; basically we restore window configuration and go through each
+  ;; window and restore the function buffers
+  (let* ((config-buffer (find-file-noselect file))
+         (placeholder (get-buffer-create " *gdb-placeholder*")))
+    (progn ; don't leak buffer
+      (let ((window-config (with-current-buffer config-buffer
+                             (goto-char 1)
+                             (read (current-buffer))))
+            ;; a list of existing gdb buffers
+            (existing-gdb-buffer-list (cl-remove-if-not
+                                       #'gdb-function-buffer-p
+                                       (buffer-list)))
+            buffer-type
+            (source-buffer (when gdb-source-window
+                             (window-buffer gdb-source-window))))
+        (window-state-put window-config (frame-root-window))
+        (dolist (window (window-list nil 'no-minibuffer))
+          (select-window window)
+          (setq buffer-type (window-parameter nil 'gdb-buffer-type))
+          (pcase buffer-type
+            ('source (when source-buffer
+                       (set-window-buffer nil source-buffer)
+                       (setq gdb-source-window (selected-window))))
+            ('command (switch-to-buffer gud-comint-buffer))
+            ;; locally shadow `buffer-list', it should be safe
+            (_ (let ((buffer (cl-labels ((buffer-list () existing-gdb-buffer-list))
+                               (gdb-get-buffer-create buffer-type))))
+                 (gdb--inhibit-window-dedicated
+                  (set-window-buffer nil buffer))
+                 ;; each time when we display a gdb function buffer in a window
+                 ;; we remove that buffer from `existing-gdb-buffer-list'
+                 ;; this way we avoid displaying the same buffer in two windows
+                 (setq existing-gdb-buffer-list
+                       (remove buffer existing-gdb-buffer-list)))))))
+      (kill-buffer config-buffer)
+      (kill-buffer placeholder))))
+
 (define-minor-mode gdb-many-windows
   "If nil just pop up the GUD buffer unless `gdb-show-main' is t.
 In this case it starts with two windows: one displaying the GUD
-- 
2.23.0

Re: Extend gdb to filter registers

[martin rudalics]

 > +(defmacro gdb--inhibit-window-dedicated (&rest body)

If implemented, this should IMHO be called 'with-window-undedicated',
go to window.el and accept an arbitrary window as argument.

martin

Re: Extend gdb to filter registers

[Juri Linkov]

> Here is another patch that allows a user to save and restore window
> configurations. It seems to work for me and I welcome you to try it out.

Thanks, some additional remarks:

>  (require 'cl-lib)
> +(require 'pcase)

Usually macro packages such as 'cl-lib' and 'pcase' are loaded with
(eval-when-compile (require 'pcase))

> +    (when (or (derived-mode-p 'gdb-parent-mode)
> +              (derived-mode-p 'gdb-inferior-io-mode)) t)))

Font-lock highlights the final 't' with red color and font-lock-warning-face.
And indeed this could be simplified to:

    (or (derived-mode-p 'gdb-parent-mode)
        (derived-mode-p 'gdb-inferior-io-mode))

> +        (kill-buffer buffer))
> +      (kill-buffer placeholder))))
> [...]
> +    (progn ; don't leak buffer
>        [...]
> +      (kill-buffer config-buffer)
> +      (kill-buffer placeholder))))

Usually temporary buffers are cleaned in the end of unwind-protect,
to ensure temporary buffer cleaning even in case of errors.

> +                             (goto-char 1)

(goto-char (point-min))

Re: Extend gdb to filter registers

[Stefan Monnier]

>     (or (derived-mode-p 'gdb-parent-mode)
>         (derived-mode-p 'gdb-inferior-io-mode))

Aka (derived-mode-p 'gdb-parent-mode 'gdb-inferior-io-mode)


        Stefan

Re: Extend gdb to filter registers

[Yuan Fu]

Juri Linkov <juri@linkov.net> writes:

>> Here is another patch that allows a user to save and restore window
>> configurations. It seems to work for me and I welcome you to try it out.
>
> Thanks, some additional remarks:
>
>>  (require 'cl-lib)
>> +(require 'pcase)
>
> Usually macro packages such as 'cl-lib' and 'pcase' are loaded with
> (eval-when-compile (require 'pcase))

Fixed.


>> +    (when (or (derived-mode-p 'gdb-parent-mode)
>> +              (derived-mode-p 'gdb-inferior-io-mode)) t)))

Fixed as Stefan suggested.

> Font-lock highlights the final 't' with red color and font-lock-warning-face.
> And indeed this could be simplified to:
>
>     (or (derived-mode-p 'gdb-parent-mode)
>         (derived-mode-p 'gdb-inferior-io-mode))
>
>> +        (kill-buffer buffer))
>> +      (kill-buffer placeholder))))
>> [...]
>> +    (progn ; don't leak buffer
>>        [...]
>> +      (kill-buffer config-buffer)
>> +      (kill-buffer placeholder))))
>
> Usually temporary buffers are cleaned in the end of unwind-protect,
> to ensure temporary buffer cleaning even in case of errors.

Fixed.

>> +                             (goto-char 1)
>
> (goto-char (point-min))

I’m worried that narrowing will cause problems. Is it adviced to always
use ‘point-min’?

Thanks.

Yuan

Re: Extend gdb to filter registers

[Yuan Fu]

martin rudalics <rudalics@gmx.at> writes:

>  > +(defmacro gdb--inhibit-window-dedicated (&rest body)
>
> If implemented, this should IMHO be called 'with-window-undedicated',
> go to window.el and accept an arbitrary window as argument.

Is accepting arbitrary window as argument necessary? Because there are
already ‘with-selected-window’.

Yuan

Re: Extend gdb to filter registers

[Yuan Fu]

Before anyone slap me for it, here is another fix for the macro.

    (defmacro with-window-undedicated (&rest body)
      "Run BODY in WINDOW with window-dedicated temporarily disabled. "
      (let ((window-dedicated-sym (gensym)))
        `(let ((,window-dedicated-sym (window-dedicated-p)))
           (when ,window-dedicated-sym
             (set-window-dedicated-p nil nil))
           ,@body
           (when ,window-dedicated-sym
             (set-window-dedicated-p nil t)))))

Yuan

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Wed, 16 Oct 2019 23:08:41 -0400
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>,
>  Emacs developers <emacs-devel@gnu.org>
> 
> >> +                             (goto-char 1)
> >
> > (goto-char (point-min))
> 
> I’m worried that narrowing will cause problems. Is it adviced to always
> use ‘point-min’?

If the buffer is narrowed so that point-min as greater than 1, you
cannot goto position 1; trying that will signal an error.

Re: Extend gdb to filter registers

[martin rudalics]

 >>   > +(defmacro gdb--inhibit-window-dedicated (&rest body)
 >>
 >> If implemented, this should IMHO be called 'with-window-undedicated',
 >> go to window.el and accept an arbitrary window as argument.
 >
 > Is accepting arbitrary window as argument necessary? Because there are
 > already ‘with-selected-window’.

Then you should call it 'with-selected-window-undedicated'.  'window-'
should ideally always refer to an arbitrary (live) window.

martin

Re: Extend gdb to filter registers

[Yuan Fu]

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

> If the buffer is narrowed so that point-min as greater than 1, you
> cannot goto position 1; trying that will signal an error.

Thanks, I’ve fixed it.

Here is the latest patch. I added a customizable variable that controls
the default directory in where we store/restore window config file.

By now I’ve collected 3 patches: register, memory and store/retore
window config. They all seem to work pretty well but I haven’t used them
too much since I’ve finished my bomblab...

Yuan


[-- Attachment #2: store-window.patch --]
[-- Type: text/x-patch, Size: 9116 bytes --]

From 1a39dc1cb80a6ea54edcf17c9d59362c2cc40a33 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Thu, 17 Oct 2019 17:35:48 -0400
Subject: [PATCH 1/2] Add with-selected-window-undedicated

* lisp/window.el (with-selected-window-undedicated): new
---
 lisp/window.el | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lisp/window.el b/lisp/window.el
index d93ec0add6..ba48354572 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -278,6 +278,16 @@ with-displayed-buffer-window
 	     (funcall ,vquit-function ,window ,value)
 	   ,value)))))
 
+(defmacro with-selected-window-undedicated (&rest body)
+  "Run BODY in the selected window with window-dedicated temporarily disabled."
+  (let ((window-dedicated-sym (gensym)))
+    `(let ((,window-dedicated-sym (window-dedicated-p)))
+       (when ,window-dedicated-sym
+         (set-window-dedicated-p nil nil))
+       ,@body
+       (when ,window-dedicated-sym
+         (set-window-dedicated-p nil t)))))
+
 ;; The following two functions are like `window-next-sibling' and
 ;; `window-prev-sibling' but the WINDOW argument is _not_ optional (so
 ;; they don't substitute the selected window for nil), and they return
-- 
2.23.0


From 1036eccc4bf8e585f0b1ecf9f676db0003adf0f1 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Mon, 14 Oct 2019 21:11:43 -0400
Subject: [PATCH 2/2] Add window configuration save/restore feature for gdb-mi
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Now you can save a gdb window configuration to a file with
‘gdb-store-window-configuration’ and restore it from a file
with ‘gdb-restore-window-configuration’.

* lisp/progmodes/gdb-mi.el (require): add pcase, wrap inside
‘eval-when-compile’
(gdb-store-window-directory, gdb-buffer-p, gdb-function-buffer-p,
gdb--buffer-type, gdb--inhibit-window-dedicated,
gdb-store-window-configuration, gdb-restore-window-configuration): new
---
 lisp/progmodes/gdb-mi.el | 130 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 129 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 60852e4ad6..5d220caa42 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -91,7 +91,8 @@
 (require 'gud)
 (require 'json)
 (require 'bindat)
-(require 'cl-lib)
+(eval-when-compile (require 'cl-lib))
+(eval-when-compile (require 'pcase))
 
 (declare-function speedbar-change-initial-expansion-list
                   "speedbar" (new-default))
@@ -589,6 +590,13 @@ gdb-show-main
   :group 'gdb
   :version "22.1")
 
+(defcustom gdb-store-window-directory user-emacs-directory
+  "The default directory where window configuration files are stored.
+If nil, use `default-directory'."
+  :type 'string
+  :group 'gdb
+  :version "27.1")
+
 (defvar gdbmi-debug-mode nil
   "When non-nil, print the messages sent/received from GDB/MI in *Messages*.")
 
@@ -4609,6 +4617,126 @@ gdb-setup-windows
                              nil win5))
     (select-window win0)))
 
+(defun gdb-buffer-p (buffer)
+  "Return t if BUFFER is gdb-related."
+  (with-current-buffer buffer
+    (eq gud-minor-mode 'gdbmi)))
+
+(defun gdb-function-buffer-p (buffer)
+  "Return t if BUFFER is a gdb function buffer.
+
+E.g., locals buffer, registers buffer, but don't include the main
+command buffer (the one in where you type gdb commands) or source
+buffers."
+  (with-current-buffer buffer
+    (derived-mode-p 'gdb-parent-mode 'gdb-inferior-io-mode)))
+
+(defun gdb--buffer-type (buffer)
+  "Return the buffer type of BUFFER or nil.
+
+Buffer type is like `gdb-registers-type', `gdb-stack-buffer'.
+This symbol can be passed to `gdb-get-buffer-create'.
+
+Return nil if BUFFER isn't a gdb function buffer."
+  (with-current-buffer buffer
+    (cl-loop for rule in gdb-buffer-rules
+             for mode-name = (gdb-rules-buffer-mode rule)
+             for type = (car rule)
+             if (eq mode-name major-mode)
+             return type
+             finally return nil)))
+
+(defun gdb-store-window-configuration (file)
+  "Save current window configuration to FILE.
+
+You can later restore this configuration from that file by
+`gdb-restore-window-configuration'."
+  (interactive (list (read-file-name
+                      "Save window configuration to file: "
+                      (or gdb-store-window-directory default-directory))))
+  ;; we replace the buffer in each window with a placeholder, set
+  ;; window parameter as the buffer type (register, breakpoint, etc),
+  ;; and store window configuration
+  (save-window-excursion
+    (let ((placeholder (get-buffer-create " *gdb-placeholder*"))
+          (window-persistent-parameters
+           (cons '(gdb-buffer-type . writable) window-persistent-parameters))
+          window-config)
+      (dolist (win (window-list nil 'no-minibuffer))
+        (select-window win)
+        (when (gdb-buffer-p (current-buffer))
+          (set-window-parameter
+           nil 'gdb-buffer-type
+           (cond ((gdb-function-buffer-p (current-buffer))
+                  ;; we save this gdb-buffer-type symbol so
+                  ;; we can later pass it to `gdb-get-buffer-create'
+                  ;; one example: `gdb-registers-buffer'
+                  (or (gdb--buffer-type (current-buffer))
+                      (error "Unrecognized gdb buffer mode: %s" major-mode)))
+                 ;; command buffer
+                 ((derived-mode-p 'gud-mode) 'command)
+                 ((member (selected-window) gdb-source-window) 'source)))
+          (with-selected-window-undedicated
+           (set-window-buffer nil placeholder)
+           (set-window-prev-buffers (selected-window) nil)
+           (set-window-next-buffers (selected-window) nil))))
+      ;; save the window configuration to FILE
+      (setq window-config (window-state-get nil t))
+      (let ((buffer (find-file file)))
+        (unwind-protect
+            (with-current-buffer buffer
+              (if buffer-read-only
+                  (user-error "Error: file read only")
+                (erase-buffer)
+                (prin1 window-config (current-buffer))
+                (save-buffer)))
+          (kill-buffer buffer)
+          (kill-buffer placeholder))))))
+
+(defun gdb-restore-window-configuration (file)
+  "Restore window configuration from FILE.
+
+FILE is saved by `gdb-store-window-configuration'."
+  (interactive (list (read-file-name
+                      "Restore window configuration from file: "
+                      (or gdb-store-window-directory default-directory))))
+  ;; basically we restore window configuration and go through each
+  ;; window and restore the function buffers
+  (let* ((config-buffer (find-file-noselect file))
+         (placeholder (get-buffer-create " *gdb-placeholder*")))
+    (unwind-protect ; don't leak buffer
+        (let ((window-config (with-current-buffer config-buffer
+                               (goto-char (point-min))
+                               (read (current-buffer))))
+              ;; a list of existing gdb buffers
+              (existing-gdb-buffer-list (cl-remove-if-not
+                                         #'gdb-function-buffer-p
+                                         (buffer-list)))
+              buffer-type
+              (source-buffer (when gdb-source-window
+                               (window-buffer gdb-source-window))))
+          (window-state-put window-config (frame-root-window))
+          (dolist (window (window-list nil 'no-minibuffer))
+            (select-window window)
+            (setq buffer-type (window-parameter nil 'gdb-buffer-type))
+            (pcase buffer-type
+              ('source (when source-buffer
+                         (set-window-buffer nil source-buffer)
+                         (setq gdb-source-window (selected-window))))
+              ('command (switch-to-buffer gud-comint-buffer))
+              ;; locally shadow `buffer-list', it should be safe
+              (_ (let ((buffer (cl-labels ((buffer-list () existing-gdb-buffer-list))
+                                 (gdb-get-buffer-create buffer-type))))
+                   (with-selected-window-undedicated
+                    (set-window-buffer nil buffer))
+                   ;; each time when we display a gdb function buffer in a window
+                   ;; we remove that buffer from `existing-gdb-buffer-list'
+                   ;; this way we avoid displaying the same buffer in two windows
+                   (setq existing-gdb-buffer-list
+                         (remove buffer existing-gdb-buffer-list)))))))
+      (kill-buffer config-buffer)
+      (kill-buffer placeholder))))
+
 (define-minor-mode gdb-many-windows
   "If nil just pop up the GUD buffer unless `gdb-show-main' is t.
 In this case it starts with two windows: one displaying the GUD
-- 
2.23.0

Re: Extend gdb to filter registers

[martin rudalics]

+(defmacro with-selected-window-undedicated (&rest body)
+  "Run BODY in the selected window with window-dedicated temporarily disabled."

Since I see no clear evidence for such behavior I would write

(defmacro with-selected-window-undedicated (&rest body)
   "Run BODY with the selected window temporarily undedicated."

instead.

martin

Re: Extend gdb to filter registers

[Yuan Fu]

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


> (defmacro with-selected-window-undedicated (&rest body)
>  "Run BODY with the selected window temporarily undedicated.”

Cool, I’ve fixed the docstring.

Yuan

[-- Attachment #2: Type: text/html, Size: 1687 bytes --]

Re: Extend gdb to filter registers

[Yuan Fu]

[-- Attachment #1.1: Type: text/plain, Size: 343 bytes --]

I'm back working on gdb. This time I fixed a issue where gdb opens more
than one window to display source code. By the design of gdb-mi, it should
only have one source window.

P.S. Will it be easier to view my changes if I create a branch under
scratch? Do I need membership of emacs on savannah to create a branch?

Yuan

Here is the patch:

[-- Attachment #1.2: Type: text/html, Size: 936 bytes --]

[-- Attachment #2: gdb-source-window.patch --]
[-- Type: text/x-patch, Size: 1725 bytes --]

From d65dc05ac050df54e7ad35f49c793310814bfbe7 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Wed, 15 Jan 2020 23:06:23 -0500
Subject: [PATCH] Fix: gdb opens new window to display source file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

By design it shouldn’t, the problem is gud-display-line uses
display-buffer instead of gdb-display-source-buffer.

* lisp/gud.el (gud-display-line): add gdb-display-source-buffer,
remove (setq gdb-source-window) at the bottom because it is taken care
of in gdb-display-source-buffer
---
 lisp/progmodes/gud.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/gud.el b/lisp/progmodes/gud.el
index d5fd1dce6f..86880b0b40 100644
--- a/lisp/progmodes/gud.el
+++ b/lisp/progmodes/gud.el
@@ -2829,7 +2829,9 @@ gud-display-line
 	    (gud-find-file true-file)))
 	 (window (and buffer
 		      (or (get-buffer-window buffer)
-			  (display-buffer buffer '(nil (inhibit-same-window . t))))))
+                          (gdb-display-source-buffer buffer)
+			  (setq gdb-source-window
+                                (display-buffer buffer '(nil (inhibit-same-window . t)))))))
 	 (pos))
     (when buffer
       (with-current-buffer buffer
@@ -2859,9 +2861,7 @@ gud-display-line
 	       (widen)
 	       (goto-char pos))))
       (when window
-	(set-window-point window gud-overlay-arrow-position)
-	(if (eq gud-minor-mode 'gdbmi)
-	    (setq gdb-source-window window))))))
+	(set-window-point window gud-overlay-arrow-position)))))
 
 ;; The gud-call function must do the right thing whether its invoking
 ;; keystroke is from the GUD buffer itself (via major-mode binding)
-- 
2.24.1

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Wed, 15 Jan 2020 23:25:47 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, Emacs developers <emacs-devel@gnu.org>, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, Juri Linkov <juri@linkov.net>
> 
> I'm back working on gdb. This time I fixed a issue where gdb opens more than one window to display source
> code. By the design of gdb-mi, it should only have one source window.

Thanks, can you show a use case where such a problem happens?

> P.S. Will it be easier to view my changes if I create a branch under scratch? Do I need membership of emacs
> on savannah to create a branch?

No, posting patches here is fine.

Re: Extend gdb to filter registers

[Yuan Fu]

[-- Attachment #1.1: Type: text/plain, Size: 837 bytes --]

> > I'm back working on gdb. This time I fixed a issue where gdb opens more
> than one window to display source
> > code. By the design of gdb-mi, it should only have one source window.
>
> Thanks, can you show a use case where such a problem happens?
>

Yes, basically any program with more than one source file and a large
screen (display-buffer will reuse window if there aren't enough space).

A minimal setup:

a.c:

int f1(int a) {
    return a + 1;
}

b.c:

#include "a.c"

int main() {
    int b = f1(1);
    int c = 0;
    return b + c;
}

Steps:
- M-x gdb
- b main
- r
- s (many times)

And when you step into f1 which is in a.c, gdb opens another window to
display it (again, if your screen is large enough).

BTW, I say "by design gdb uses a single source window" because
gdb-display-source-window is written that way.

Yuan

[-- Attachment #1.2: Type: text/html, Size: 2724 bytes --]

[-- Attachment #2: b.c --]
[-- Type: text/x-csrc, Size: 83 bytes --]

#include "a.c"

int main() {
    int b = f1(1);
    int c = 0;
    return b + c;
}

[-- Attachment #3: a.c --]
[-- Type: text/x-csrc, Size: 36 bytes --]

int f1(int a) {
    return a + 1;
}

Re: Extend gdb to filter registers

[Yuan Fu]

[-- Attachment #1.1: Type: text/plain, Size: 2176 bytes --]

I forgot to apply the new trick I learned to
gdb-store-window-configuration. The trick makes file writing cleaner. This
is the store-window patch with that trick applied
Now the following code is used instead of find-file and insert.

    (with-temp-buffer
      (prin1 window-config (current-buffer))
      (write-file file t))

Yuan

On Thu, Jan 16, 2020 at 7:38 PM Yuan Fu <casouri@gmail.com> wrote:

> Here is an updated patch for store-window feature. I fixed some problem of
> the last one and added some new features. With this patch, you can
>
> 1. store your window configuration to a file, and load it back in a gdb
> session, and
> 2. set a default window config file to be loaded on gdb startup.
> 3. Now gdb preserves the window configuration you had before starting gdb
>
> More about 3.: currently when gdb quits, it leaves you with two windows,
> one in gdb comint buffer and one in source buffer, regardless of what you
> had before staring gdb. The third commit in the patch makes gdb restore the
> old window configuration you had before staring gdb.
>
> I also attached my window configuration file as default-rearrange.
>
> Yuan
>
>
> On Thu, Jan 16, 2020 at 10:04 AM Yuan Fu <casouri@gmail.com> wrote:
>
>>
>> > I'm back working on gdb. This time I fixed a issue where gdb opens more
>>> than one window to display source
>>> > code. By the design of gdb-mi, it should only have one source window.
>>>
>>> Thanks, can you show a use case where such a problem happens?
>>>
>>
>> Yes, basically any program with more than one source file and a large
>> screen (display-buffer will reuse window if there aren't enough space).
>>
>> A minimal setup:
>>
>> a.c:
>>
>> int f1(int a) {
>>     return a + 1;
>> }
>>
>> b.c:
>>
>> #include "a.c"
>>
>> int main() {
>>     int b = f1(1);
>>     int c = 0;
>>     return b + c;
>> }
>>
>> Steps:
>> - M-x gdb
>> - b main
>> - r
>> - s (many times)
>>
>> And when you step into f1 which is in a.c, gdb opens another window to
>> display it (again, if your screen is large enough).
>>
>> BTW, I say "by design gdb uses a single source window" because
>> gdb-display-source-window is written that way.
>>
>> Yuan
>>
>

[-- Attachment #1.2: Type: text/html, Size: 5749 bytes --]

[-- Attachment #2: store-window-new.patch --]
[-- Type: text/x-patch, Size: 15629 bytes --]

From 6db5201e418bc15ff51a170124a84182093c64db Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Thu, 17 Oct 2019 17:35:48 -0400
Subject: [PATCH 1/3] Add with-selected-window-undedicated

* lisp/window.el (with-selected-window-undedicated): new
---
 lisp/window.el | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lisp/window.el b/lisp/window.el
index 433486385d..98b8b21f8a 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -278,6 +278,19 @@ with-displayed-buffer-window
 	     (funcall ,vquit-function ,window ,value)
 	   ,value)))))
 
+(defmacro with-selected-window-undedicated (&rest body)
+  "Run BODY in the selected window temporarily undedicated."
+  (let ((window-dedicated-sym (gensym)))
+    `(let ((,window-dedicated-sym (window-dedicated-p)))
+       (when ,window-dedicated-sym
+         (set-window-dedicated-p nil nil))
+       ,@body
+       (when ,window-dedicated-sym
+         ;; `window-dedicated-p' returns the value set by
+         ;; `set-window-dedicated-p', which differentiates
+         ;; non-nil and t, so we cannot simply set to t
+         (set-window-dedicated-p nil ,window-dedicated-sym)))))
+
 ;; The following two functions are like `window-next-sibling' and
 ;; `window-prev-sibling' but the WINDOW argument is _not_ optional (so
 ;; they don't substitute the selected window for nil), and they return
-- 
2.24.1


From bb5e41e230a8452b41a569386131648eb2599501 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Mon, 14 Oct 2019 21:11:43 -0400
Subject: [PATCH 2/3] Add window configuration save/restore feature for gdb-mi
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Now you can save a gdb window configuration to a file with
‘gdb-store-window-configuration’ and restore it from a file
with ‘gdb-restore-window-configuration’. Set a default window
configuration by setting gdb-default-window-configuration-file.
Note that for the default window configuration to take effect,
gdb-many-windows needs to be t.

* lisp/progmodes/gdb-mi.el (require): add pcase, wrap inside
‘eval-when-compile’

(gdb-get-source-buffer): new, extracted out of gdb-restore-window
(gdb-restore-window): extract out gdb-get-source-buffer

(gdb-store-window-directory, gdb-buffer-p, gdb-function-buffer-p,
gdb--buffer-type, gdb--inhibit-window-dedicated,
gdb-store-window-configuration, gdb-restore-window-configuration): new

(gdb-default-window-configuration-file): new
(gdb-setup-windows): Add a condition branch that loads default window
config when available

(gdb-many-windows, gdb-get-source-file): add comments
---
 lisp/progmodes/gdb-mi.el | 210 ++++++++++++++++++++++++++++++++-------
 1 file changed, 176 insertions(+), 34 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index b08d487af3..cf0c8f29a9 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -91,7 +91,8 @@
 (require 'gud)
 (require 'json)
 (require 'bindat)
-(require 'cl-lib)
+(eval-when-compile (require 'cl-lib))
+(eval-when-compile (require 'pcase))
 
 (declare-function speedbar-change-initial-expansion-list
                   "speedbar" (new-default))
@@ -589,6 +590,20 @@ gdb-show-main
   :group 'gdb
   :version "22.1")
 
+(defcustom gdb-store-window-directory user-emacs-directory
+  "The default directory where window configuration files are stored.
+If nil, use `default-directory'."
+  :type 'string
+  :group 'gdb
+  :version "28.1")
+
+(defcustom gdb-default-window-configuration-file nil
+  "If non-nil, gdb loads this window configuration file on startup.
+This should be an absolute file path."
+  :type 'string
+  :group 'gdb
+  :version "28.1")
+
 (defvar gdbmi-debug-mode nil
   "When non-nil, print the messages sent/received from GDB/MI in *Messages*.")
 
@@ -4574,41 +4589,164 @@ gdb-set-window-buffer
   (set-window-buffer window (get-buffer name))
   (set-window-dedicated-p window t))
 
+(defun gdb-get-source-buffer ()
+  "Return a buffer displaying source file or nil.
+
+The source file would be the most relevant file or the main file."
+  (if gud-last-last-frame
+      (gud-find-file (car gud-last-last-frame))
+    (when gdb-main-file
+      (gud-find-file gdb-main-file))))
+
 (defun gdb-setup-windows ()
   "Layout the window pattern for option `gdb-many-windows'."
-  (gdb-get-buffer-create 'gdb-locals-buffer)
-  (gdb-get-buffer-create 'gdb-stack-buffer)
-  (gdb-get-buffer-create 'gdb-breakpoints-buffer)
-  (set-window-dedicated-p (selected-window) nil)
-  (switch-to-buffer gud-comint-buffer)
-  (delete-other-windows)
-  (let ((win0 (selected-window))
-        (win1 (split-window nil ( / ( * (window-height) 3) 4)))
-        (win2 (split-window nil ( / (window-height) 3)))
-        (win3 (split-window-right)))
-    (gdb-set-window-buffer (gdb-locals-buffer-name) nil win3)
-    (select-window win2)
-    (set-window-buffer
-     win2
-     (if gud-last-last-frame
-         (gud-find-file (car gud-last-last-frame))
-       (if gdb-main-file
-           (gud-find-file gdb-main-file)
-         ;; Put buffer list in window if we
-         ;; can't find a source file.
-         (list-buffers-noselect))))
-    (setq gdb-source-window (selected-window))
-    (let ((win4 (split-window-right)))
-      (gdb-set-window-buffer
-       (gdb-get-buffer-create 'gdb-inferior-io) nil win4))
-    (select-window win1)
-    (gdb-set-window-buffer (gdb-stack-buffer-name))
-    (let ((win5 (split-window-right)))
-      (gdb-set-window-buffer (if gdb-show-threads-by-default
-                                 (gdb-threads-buffer-name)
-                               (gdb-breakpoints-buffer-name))
-                             nil win5))
-    (select-window win0)))
+  (if gdb-default-window-configuration-file
+      (gdb-restore-window-configuration
+       gdb-default-window-configuration-file)
+    ;; default layout
+    (gdb-get-buffer-create 'gdb-locals-buffer)
+    (gdb-get-buffer-create 'gdb-stack-buffer)
+    (gdb-get-buffer-create 'gdb-breakpoints-buffer)
+    (set-window-dedicated-p (selected-window) nil)
+    (switch-to-buffer gud-comint-buffer)
+    (delete-other-windows)
+    (let ((win0 (selected-window))
+          (win1 (split-window nil ( / ( * (window-height) 3) 4)))
+          (win2 (split-window nil ( / (window-height) 3)))
+          (win3 (split-window-right)))
+      (gdb-set-window-buffer (gdb-locals-buffer-name) nil win3)
+      (select-window win2)
+      (set-window-buffer
+       win2
+       (or (gdb-get-source-buffer)
+           (list-buffers-noselect)))
+      (setq gdb-source-window (selected-window))
+      (let ((win4 (split-window-right)))
+        (gdb-set-window-buffer
+         (gdb-get-buffer-create 'gdb-inferior-io) nil win4))
+      (select-window win1)
+      (gdb-set-window-buffer (gdb-stack-buffer-name))
+      (let ((win5 (split-window-right)))
+        (gdb-set-window-buffer (if gdb-show-threads-by-default
+                                   (gdb-threads-buffer-name)
+                                 (gdb-breakpoints-buffer-name))
+                               nil win5))
+      (select-window win0))))
+
+(defun gdb-buffer-p (buffer)
+  "Return t if BUFFER is gdb-related."
+  (with-current-buffer buffer
+    (eq gud-minor-mode 'gdbmi)))
+
+(defun gdb-function-buffer-p (buffer)
+  "Return t if BUFFER is a gdb function buffer.
+
+E.g., locals buffer, registers buffer, but don't include the main
+command buffer (the one in where you type gdb commands) or source
+buffers."
+  (with-current-buffer buffer
+    (derived-mode-p 'gdb-parent-mode 'gdb-inferior-io-mode)))
+
+(defun gdb--buffer-type (buffer)
+  "Return the buffer type of BUFFER or nil.
+
+Buffer type is like `gdb-registers-type', `gdb-stack-buffer'.
+This symbol can be passed to `gdb-get-buffer-create'.
+
+Return nil if BUFFER isn't a gdb function buffer."
+  (with-current-buffer buffer
+    (cl-loop for rule in gdb-buffer-rules
+             for mode-name = (gdb-rules-buffer-mode rule)
+             for type = (car rule)
+             if (eq mode-name major-mode)
+             return type
+             finally return nil)))
+
+(defun gdb-store-window-configuration (file)
+  "Save current window configuration to FILE.
+
+You can later restore this configuration from that file by
+`gdb-restore-window-configuration'."
+  (interactive (list (read-file-name
+                      "Save window configuration to file: "
+                      (or gdb-store-window-directory default-directory))))
+  ;; we replace the buffer in each window with a placeholder, store
+  ;; the buffer type (register, breakpoint, etc) in window parameters,
+  ;; and store the window configuration
+  (save-window-excursion
+    (let ((placeholder (get-buffer-create " *gdb-placeholder*"))
+          (window-persistent-parameters
+           (cons '(gdb-buffer-type . writable) window-persistent-parameters))
+          window-config)
+      (unwind-protect
+          (dolist (win (window-list nil 'no-minibuffer))
+            (select-window win)
+            (when (gdb-buffer-p (current-buffer))
+              (set-window-parameter
+               nil 'gdb-buffer-type
+               (cond ((gdb-function-buffer-p (current-buffer))
+                      ;; 1) if a user arranged the window configuration
+                      ;; herself and saves it, windows are probably not
+                      ;; dedicated 2) we use the same dedication flag as
+                      ;; in `gdb-display-buffer'
+                      (set-window-dedicated-p nil t)
+                      ;; we save this gdb-buffer-type symbol so
+                      ;; we can later pass it to `gdb-get-buffer-create'
+                      ;; one example: `gdb-registers-buffer'
+                      (or (gdb--buffer-type (current-buffer))
+                          (error "Unrecognized gdb buffer mode: %s" major-mode)))
+                     ;; command buffer
+                     ((derived-mode-p 'gud-mode) 'command)
+                     ((equal (selected-window) gdb-source-window) 'source)))
+              (with-selected-window-undedicated
+               (set-window-buffer nil placeholder)
+               (set-window-prev-buffers (selected-window) nil)
+               (set-window-next-buffers (selected-window) nil))))
+        ;; save the window configuration to FILE
+        (let ((window-config (window-state-get nil t)))
+          (with-temp-buffer
+            (prin1 window-config (current-buffer))
+            (write-file file t)))
+        (kill-buffer placeholder)))))
+
+(defun gdb-restore-window-configuration (file)
+  "Restore window configuration from FILE.
+
+FILE should be a window configuration file saved by
+`gdb-store-window-configuration'."
+  (interactive (list (read-file-name
+                      "Restore window configuration from file: "
+                      (or gdb-store-window-directory default-directory))))
+  ;; basically we restore window configuration and go through each
+  ;; window and restore the function buffers
+  (let* ((placeholder (get-buffer-create " *gdb-placeholder*")))
+    (unwind-protect ; don't leak buffer
+        (let ((window-config (with-temp-buffer
+                               (insert-file-contents file)
+                               (goto-char (point-min))
+                               ;; we need to go to point-min even we
+                               ;; are reading the whole buffer
+                               (read (current-buffer))))
+              (source-buffer (if gdb-source-window
+                                 (window-buffer gdb-source-window)
+                               (or (gdb-get-source-buffer)
+                                   ;; do the same thing as in
+                                   ;; `gdb-setup-windows'
+                                   (list-buffers-noselect))))
+              buffer-type)
+          (window-state-put window-config (frame-root-window))
+          (dolist (window (window-list nil 'no-minibuffer))
+            (with-selected-window window
+              (setq buffer-type (window-parameter nil 'gdb-buffer-type))
+              (pcase buffer-type
+                ('source (when source-buffer
+                           (set-window-buffer nil source-buffer)
+                           (setq gdb-source-window (selected-window))))
+                ('command (switch-to-buffer gud-comint-buffer))
+                (_ (let ((buffer (gdb-get-buffer-create buffer-type)))
+                     (with-selected-window-undedicated
+                      (set-window-buffer nil buffer))))))))
+      (kill-buffer placeholder))))
 
 (define-minor-mode gdb-many-windows
   "If nil just pop up the GUD buffer unless `gdb-show-main' is t.
@@ -4627,6 +4765,9 @@ gdb-many-windows
 (defun gdb-restore-windows ()
   "Restore the basic arrangement of windows used by gdb.
 This arrangement depends on the value of option `gdb-many-windows'."
+  ;; this function is used when the user messed up window
+  ;; configuration and want to "reset to default". the function that
+  ;; sets up window configuration on start up is `gdb-get-source-file'
   (interactive)
   (switch-to-buffer gud-comint-buffer) ;Select the right window and frame.
   (delete-other-windows)
@@ -4678,6 +4819,7 @@ gdb-reset
 (defun gdb-get-source-file ()
   "Find the source file where the program starts and display it with related
 buffers, if required."
+  ;; this function is called only once on startup
   (goto-char (point-min))
   (if (re-search-forward gdb-source-file-regexp nil t)
       (setq gdb-main-file (read (match-string 1))))
-- 
2.24.1


From c7ac67ec759d16e1f5d212b6fadf93d4ec853ace Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Thu, 16 Jan 2020 18:52:17 -0500
Subject: [PATCH 3/3] Restore window configuration when gdb quits

Make gdb preserve the window configuration that the user had before
starting gdb.

* lisp/progmodes/gdb-mi.el (gdb--window-configuration-before): new
(gdb): save configuration before start
(gdb-reset): restore window configuration
---
 lisp/progmodes/gdb-mi.el | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index cf0c8f29a9..582ccf9562 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -243,6 +243,9 @@ gdb-output-sink
 	       disposition of output generated by commands that
 	       gdb mode sends to gdb on its own behalf.")
 
+(defvar gdb--window-configuration-before nil
+  "Stores the window configuration before starting gdb.")
+
 (defcustom gdb-discard-unordered-replies t
   "Non-nil means discard any out-of-order GDB replies.
 This protects against lost GDB replies, assuming that GDB always
@@ -762,6 +765,10 @@ gdb
     (gdb-restore-windows)
     (error
      "Multiple debugging requires restarting in text command mode"))
+
+  ;; save window configuration before starting gdb so we can restore
+  ;; it after gdb quits
+  (setq gdb--window-configuration-before (window-state-get))
   ;;
   (gud-common-init command-line nil 'gud-gdbmi-marker-filter)
 
@@ -4814,7 +4821,9 @@ gdb-reset
   (if (boundp 'speedbar-frame) (speedbar-timer-fn))
   (setq gud-running nil)
   (setq gdb-active-process nil)
-  (remove-hook 'after-save-hook 'gdb-create-define-alist t))
+  (remove-hook 'after-save-hook 'gdb-create-define-alist t)
+  ;; recover window configuration
+  (window-state-put gdb--window-configuration-before))
 
 (defun gdb-get-source-file ()
   "Find the source file where the program starts and display it with related
-- 
2.24.1

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Thu, 16 Jan 2020 10:04:00 -0500
> Cc: martin rudalics <rudalics@gmx.at>, Emacs developers <emacs-devel@gnu.org>, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, Juri Linkov <juri@linkov.net>
> 
> a.c:
> 
> int f1(int a) {
>     return a + 1;
> }
> 
> b.c:
> 
> #include "a.c"
> 
> int main() {
>     int b = f1(1);
>     int c = 0;
>     return b + c;
> }
> 
> Steps:
> - M-x gdb
> - b main
> - r
> - s (many times)
> 
> And when you step into f1 which is in a.c, gdb opens another window to display it (again, if your screen is
> large enough).

I cannot reproduce this even if I maximize my Emacs frame before
invoking "M-x gdb".  I always get the source in the same window.  I
used your example.  My screen is 1920x1080 pixels, if that's
important.

So I wonder what other factors could be at work here.

Re: Extend gdb to filter registers

[Yuan Fu]

> I cannot reproduce this even if I maximize my Emacs frame before
> invoking "M-x gdb".  I always get the source in the same window.  I
> used your example.  My screen is 1920x1080 pixels, if that's
> important.
> 
> So I wonder what other factors could be at work here.

Since the buffer is displayed by display-buffer, any factor that affects display-buffer would affect this. I have a lower split-width-threshold, that makes it easier to split a new window. You can try to delete the I/O window in the default 6-window gdb layout. So source buffer has room to split to right. I just tried and this works on emacs -q.

Yuan

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sat, 18 Jan 2020 08:32:16 -0500
> Cc: martin rudalics <rudalics@gmx.at>,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  juri@linkov.net
> 
> > So I wonder what other factors could be at work here.
> 
> Since the buffer is displayed by display-buffer, any factor that affects display-buffer would affect this. I have a lower split-width-threshold, that makes it easier to split a new window. You can try to delete the I/O window in the default 6-window gdb layout.

You didn't say anything about the 6-window layout in your recipe.  Was
the recipe incomplete?  By default, "M-x gdb" uses just 2 windows.

Re: Extend gdb to filter registers

[Yuan Fu]

> You didn't say anything about the 6-window layout in your recipe.  Was
> the recipe incomplete?  By default, "M-x gdb" uses just 2 windows.

Sorry, yes you need either set gdb-show-main or gdb-many-windows to t, so gdb displays the source file. I think gdb by default only shows the comint buffer, you need to set gdb-show-main to get the 2-window layout.

Recipe:
- emacs -q
- (setq gdb-many-windows t) / or (setq gdb-show-main t)
- maximize frame
- M-x gdb
- make room for source window. Make the source window span the whole width of Emacs frame works for both layout.
- step until gdb steps into a function in another file

Yuan

Re: Extend gdb to filter registers

[John Yates]

Eli,

I use gdb-many-windows without any additional customizations.
My screen at work is 1920x1200 and I never encounter this problem.
By contrast my screen at home is 3840x2160 and it happens every time.

/john

On Sat, Jan 18, 2020 at 6:16 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Yuan Fu <casouri@gmail.com>
> > Date: Thu, 16 Jan 2020 10:04:00 -0500
> > Cc: martin rudalics <rudalics@gmx.at>, Emacs developers <emacs-devel@gnu.org>,
> >       Stefan Monnier <monnier@iro.umontreal.ca>, Juri Linkov <juri@linkov.net>
> >
> > a.c:
> >
> > int f1(int a) {
> >     return a + 1;
> > }
> >
> > b.c:
> >
> > #include "a.c"
> >
> > int main() {
> >     int b = f1(1);
> >     int c = 0;
> >     return b + c;
> > }
> >
> > Steps:
> > - M-x gdb
> > - b main
> > - r
> > - s (many times)
> >
> > And when you step into f1 which is in a.c, gdb opens another window to display it (again, if your screen is
> > large enough).
>
> I cannot reproduce this even if I maximize my Emacs frame before
> invoking "M-x gdb".  I always get the source in the same window.  I
> used your example.  My screen is 1920x1080 pixels, if that's
> important.
>
> So I wonder what other factors could be at work here.
>


-- 
John Yates
505 Tremont St, #803
Boston, MA 02116

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sat, 18 Jan 2020 10:41:56 -0500
> Cc: martin rudalics <rudalics@gmx.at>,
>  emacs-devel <emacs-devel@gnu.org>,
>  monnier@iro.umontreal.ca,
>  juri@linkov.net
> 
> - emacs -q
> - (setq gdb-many-windows t) / or (setq gdb-show-main t)
> - maximize frame
> - M-x gdb
> - make room for source window. Make the source window span the whole width of Emacs frame works for both layout.
> - step until gdb steps into a function in another file

I still cannot reproduce this, even with this recipe, sorry.  But
maybe I don't understand what you mean by "make room for source
window"?

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: John Yates <john@yates-sheets.org>
> Date: Sat, 18 Jan 2020 11:20:46 -0500
> Cc: Yuan Fu <casouri@gmail.com>, martin rudalics <rudalics@gmx.at>, Juri Linkov <juri@linkov.net>, 
> 	Stefan Monnier <monnier@iro.umontreal.ca>, Emacs developers <emacs-devel@gnu.org>
> 
> I use gdb-many-windows without any additional customizations.
> My screen at work is 1920x1200 and I never encounter this problem.
> By contrast my screen at home is 3840x2160 and it happens every time.

Thanks.  Then maybe Martin will be able to explain what happens here,
and why, and why I cannot see it on my system.

In any case, the proposed changeset looks very large for fixing such a
small problem.

Re: Extend gdb to filter registers

[Yuan Fu]

> In any case, the proposed changeset looks very large for fixing such a
> small problem.

The fix is small — 4 deletion and 4 insertions. It is in gdb-source-window.patch. The big one your are referred to is the feature patch that allows user to store their layout to a file and later restore it. Right now if you want to use many-windows in gdb, the default 6-window layout is the only option. I also sent a window layout file that you can restore from, named `default-rearrange`.

To clarify, now I have collected 5 patches:

- unwind-protect.patch: bugfix
- gdb-source-window.patch: bugfix
- 0001-Add-register-filters-to-gdb-mi.patch: add register filters, so you can display only the ones you want
- memory.patch: allow user to use expressions as memory address in memory buffer
- store-window-new.patch: allow user to store & restore window layout.

And a window layout file `default-rearrange`.

Yuan

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sat, 18 Jan 2020 12:53:46 -0500
> Cc: rudalics@gmx.at, juri@linkov.net, emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca, John Yates <john@yates-sheets.org>
> 
> To clarify, now I have collected 5 patches:
> 
> - unwind-protect.patch: bugfix
> - gdb-source-window.patch: bugfix
> - 0001-Add-register-filters-to-gdb-mi.patch: add register filters, so you can display only the ones you want
> - memory.patch: allow user to use expressions as memory address in memory buffer
> - store-window-new.patch: allow user to store & restore window layout.
> 
> And a window layout file `default-rearrange`.

Thanks, please send each one as a separate bug report, so that their
discussions don't mix.

Re: Extend gdb to filter registers

[martin rudalics]

 > Thanks.  Then maybe Martin will be able to explain what happens here,
 > and why, and why I cannot see it on my system.

Without further customizations, the behavior depends on two factors: The
default values of 'split-width-threshold' and 'split-height-threshold'
and the current size of the frame.  'display-buffer' will try to split a
window when it's at least as large as indicated by the former and such
sizes are easily possible on large displays with maximized frames.  If
'display-buffer' cannot split a window, it will reuse one.  With a frame
already containing two windows, the reused window will be usually the
non-selected one.

The most simple approach to get a behavior where only a single source
code buffer is displayed is to add a new gdb option to customize the
'display-buffer' call that tries to display the next source buffer.
That call should (1) identify the current source code window, (2) add a
'previous-window' alist entry specifying that window and (3) specify
'display-buffer-in-previous-window' as the preferred buffer display
action.  To see how to this can be done compare the 'pop-to-buffer' call
in 'debug'.

martin

Re: Extend gdb to filter registers

[Eli Zaretskii]

> Cc: casouri@gmail.com, emacs-devel@gnu.org, monnier@iro.umontreal.ca,
>  juri@linkov.net
> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 18 Jan 2020 19:21:43 +0100
> 
>  > Thanks.  Then maybe Martin will be able to explain what happens here,
>  > and why, and why I cannot see it on my system.
> 
> Without further customizations, the behavior depends on two factors: The
> default values of 'split-width-threshold' and 'split-height-threshold'
> and the current size of the frame.  'display-buffer' will try to split a
> window when it's at least as large as indicated by the former and such
> sizes are easily possible on large displays with maximized frames.  If
> 'display-buffer' cannot split a window, it will reuse one.  With a frame
> already containing two windows, the reused window will be usually the
> non-selected one.
> 
> The most simple approach to get a behavior where only a single source
> code buffer is displayed is to add a new gdb option to customize the
> 'display-buffer' call that tries to display the next source buffer.
> That call should (1) identify the current source code window, (2) add a
> 'previous-window' alist entry specifying that window and (3) specify
> 'display-buffer-in-previous-window' as the preferred buffer display
> action.  To see how to this can be done compare the 'pop-to-buffer' call
> in 'debug'.

gdb-mi.el has gdb-display-buffer, which makes the window dedicated.
It also has some code in gdb-display-source-buffer.  Neither of those
uses display-buffer, AFAICT.  So where do the split thresholds come
into play in the scenario mentioned?

Re: Extend gdb to filter registers

[Yuan Fu]

> gdb-mi.el has gdb-display-buffer, which makes the window dedicated.
> It also has some code in gdb-display-source-buffer.  Neither of those
> uses display-buffer, AFAICT.  So where do the split thresholds come
> into play in the scenario mentioned?

The problem is in gud.el: it uses display-buffer instead of gdb-display-source-buffer. When you step, `gud-display-line` runs and calls display-buffer. My patch makes it use `gdb-display-source-buffer`.

Yuan

Re: Extend gdb to filter registers

[martin rudalics]

 > gdb-mi.el has gdb-display-buffer, which makes the window dedicated.
 > It also has some code in gdb-display-source-buffer.  Neither of those
 > uses display-buffer, AFAICT.  So where do the split thresholds come
 > into play in the scenario mentioned?

Here

(defun gdb-display-buffer (buf)
   "Show buffer BUF, and make that window dedicated."
   (let ((window (display-buffer buf)))
                  ^^^^^^^^^^^^^^^^^^
     (set-window-dedicated-p window t)
     window))

martin

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sat, 18 Jan 2020 13:36:34 -0500
> Cc: martin rudalics <rudalics@gmx.at>,
>  john@yates-sheets.org,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  juri@linkov.net
> 
> The problem is in gud.el: it uses display-buffer instead of gdb-display-source-buffer. When you step, `gud-display-line` runs and calls display-buffer. My patch makes it use `gdb-display-source-buffer`.

If the problem is in gud.el, then we must think about the other users
of that code, like "M-x gud-gdb".  gdb-display-source-buffer is
specific to gdb-mi.el.  And I see that as part of your patch you've
removed the test of gud-minor-mode being gdbmi, which is probably not
TRT.

Also, what happens if the function into which you step is already
displayed in some other window?

Re: Extend gdb to filter registers

[Eli Zaretskii]

> Cc: john@yates-sheets.org, casouri@gmail.com, emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca, juri@linkov.net
> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 18 Jan 2020 19:41:37 +0100
> 
>  > gdb-mi.el has gdb-display-buffer, which makes the window dedicated.
>  > It also has some code in gdb-display-source-buffer.  Neither of those
>  > uses display-buffer, AFAICT.  So where do the split thresholds come
>  > into play in the scenario mentioned?
> 
> Here
> 
> (defun gdb-display-buffer (buf)
>    "Show buffer BUF, and make that window dedicated."
>    (let ((window (display-buffer buf)))
>                   ^^^^^^^^^^^^^^^^^^
>      (set-window-dedicated-p window t)
>      window))

This function isn't called when gdb-mi.el displays a source file in
its window.

Re: Extend gdb to filter registers

[Yuan Fu]

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

> If the problem is in gud.el, then we must think about the other users
> of that code, like "M-x gud-gdb".  gdb-display-source-buffer is
> specific to gdb-mi.el.  And I see that as part of your patch you've
> removed the test of gud-minor-mode being gdbmi, which is probably not
> TRT.

Fixed, please see source-window.patch.

> Also, what happens if the function into which you step is already
> displayed in some other window?

Maybe we can modify `gdb-display-source-buffer` to handle that. But I’m not sure if that won’t interfere with other stuff. What do you think?

Yuan


[-- Attachment #2: source-window.patch --]
[-- Type: application/octet-stream, Size: 1132 bytes --]

From fb77ae530a8ed4eb3c9485e4bb6d60fa77dc4e3d Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sat, 18 Jan 2020 13:58:17 -0500
Subject: [PATCH] Fix gdb-mi opens new window to display source file
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

By design it shouldn’t, the problem is gud-display-line uses
display-buffer instead of gdb-display-source-buffer.

* lisp/progmodes/gud.el (gud-display-line): use
gdb-display-source-buffer when running gdb-mi
---
 lisp/progmodes/gud.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/progmodes/gud.el b/lisp/progmodes/gud.el
index d5fd1dce6f..a81b8996cf 100644
--- a/lisp/progmodes/gud.el
+++ b/lisp/progmodes/gud.el
@@ -2829,6 +2829,8 @@ gud-display-line
 	    (gud-find-file true-file)))
 	 (window (and buffer
 		      (or (get-buffer-window buffer)
+                          (when (eq gud-minor-mode 'gdbmi)
+                            (gdb-display-source-buffer buffer))
 			  (display-buffer buffer '(nil (inhibit-same-window . t))))))
 	 (pos))
     (when buffer
-- 
2.24.1

Re: Extend gdb to filter registers

[martin rudalics]

 >>   > gdb-mi.el has gdb-display-buffer, which makes the window dedicated.
 >>   > It also has some code in gdb-display-source-buffer.  Neither of those
 >>   > uses display-buffer, AFAICT.  So where do the split thresholds come
 >>   > into play in the scenario mentioned?
 >>
 >> Here
 >>
 >> (defun gdb-display-buffer (buf)
 >>     "Show buffer BUF, and make that window dedicated."
 >>     (let ((window (display-buffer buf)))
 >>                    ^^^^^^^^^^^^^^^^^^
 >>       (set-window-dedicated-p window t)
 >>       window))
 >
 > This function isn't called when gdb-mi.el displays a source file in
 > its window.

Above you said that "Neither of those uses display-buffer".  Now
'gdb-display-source-buffer' is called exclusively (within gdb-mi.el) by
'gdb-goto-breakpoint' which has

                      (window (or (gdb-display-source-buffer buffer)
                                  (display-buffer buffer))))

so if 'gdb-display-source-buffer' fails, 'display-buffer' is called.

martin

Re: Extend gdb to filter registers

[Eli Zaretskii]

> Cc: juri@linkov.net, casouri@gmail.com, emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca, john@yates-sheets.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 18 Jan 2020 22:43:46 +0100
> 
>  >>   > gdb-mi.el has gdb-display-buffer, which makes the window dedicated.
>  >>   > It also has some code in gdb-display-source-buffer.  Neither of those
>  >>   > uses display-buffer, AFAICT.  So where do the split thresholds come
>  >>   > into play in the scenario mentioned?
>  >>
>  >> Here
>  >>
>  >> (defun gdb-display-buffer (buf)
>  >>     "Show buffer BUF, and make that window dedicated."
>  >>     (let ((window (display-buffer buf)))
>  >>                    ^^^^^^^^^^^^^^^^^^
>  >>       (set-window-dedicated-p window t)
>  >>       window))
>  >
>  > This function isn't called when gdb-mi.el displays a source file in
>  > its window.
> 
> Above you said that "Neither of those uses display-buffer".  Now
> 'gdb-display-source-buffer' is called exclusively (within gdb-mi.el) by
> 'gdb-goto-breakpoint' which has
> 
>                       (window (or (gdb-display-source-buffer buffer)
>                                   (display-buffer buffer))))
> 
> so if 'gdb-display-source-buffer' fails, 'display-buffer' is called.

So you are saying that calling gdb-display-source-buffer instead of
display-buffer will not by itself help here, because display-buffer is
called under the hood anyway, and might decide to create a new window
instead of reusing an existing one?

If so, what would be the correct solution of this issue?

Re: Extend gdb to filter registers

[Yuan Fu]

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

> So you are saying that calling gdb-display-source-buffer instead of
> display-buffer will not by itself help here, because display-buffer is
> called under the hood anyway, and might decide to create a new window
> instead of reusing an existing one?

`gdb-goto-breakpoint` first calls `gdb-display-source-buffer` which will look for the right window to display. If `gdb-display-source-buffer` can’t find the right window, it will return nil. In that case `gdb-goto-breakpoint` will use `display-buffer` to show the buffer. So, if there _is_ a right window, gdb-mi will do the right thing, and `display-buffer` is not used.

> If so, what would be the correct solution of this issue?

As for the issue where gbd-mi creates new window when it doesn’t need to, my fix is to make `gud-display-line` behaves like `gdb-goto-breakpoint`: try `gdb-display-source-buffer` first (when gdb-mi is enabled), then try `display-buffer`; instead of simply using `display-buffer`.

Yuan

[-- Attachment #2: Type: text/html, Size: 4819 bytes --]

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 19 Jan 2020 12:33:44 -0500
> Cc: martin rudalics <rudalics@gmx.at>,
>  Juri Linkov <juri@linkov.net>,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  john@yates-sheets.org
> 
>  If so, what would be the correct solution of this issue?
> 
> As for the issue where gbd-mi creates new window when it doesn’t need to, my fix is to make
> `gud-display-line` behaves like `gdb-goto-breakpoint`: try `gdb-display-source-buffer` first (when gdb-mi is
> enabled), then try `display-buffer`; instead of simply using `display-buffer`.

Frankly, given Martin's input, I don't understand how will that solve
the problem, because display-buffer is still being called, and so it
could under some circumstances still decide to create a new window.

Am I missing something?

Re: Extend gdb to filter registers

[Yuan Fu]

> Frankly, given Martin's input, I don't understand how will that solve
> the problem, because display-buffer is still being called, and so it
> could under some circumstances still decide to create a new window.
> 
> Am I missing something?

The issue I’m having is that, under normal circumstances, which means there exists a source window, I expect gdb-mi to always reuse that source window (and not create new ones); however gdb-mi doesn’t do that when stepping. 

Note that in `gdb-goto-breakpoint`, `display-buffer` doesn’t run if `gdb-display-source-buffer` can find a source window.

The reason that `gdb-goto-breakpoint` does the right thing is because it tries `gdb-display-source-buffer` first, and only use `display-buffer` if `gdb-display-source-buffer` fails (can’t find a source window).

Yuan

Re: Extend gdb to filter registers

[martin rudalics]

 > So you are saying that calling gdb-display-source-buffer instead of
 > display-buffer will not by itself help here, because display-buffer is
 > called under the hood anyway, and might decide to create a new window
 > instead of reusing an existing one?

So far I've been talking only about the display functions in gdb-mi.el
and that in principle all of them can call 'display-buffer'.  But I
don't know what really happens on your system and that of Yuan Fu.  The
interactions betwen gdb-mi.el and gud.el are too complicated for me to
tell from here.

One way to find out is to remember the name of the file (or buffer) gdb
will try to trace, and instrument 'display-buffer' such that when it is
asked to display that file it will print a backtrace like the following:

   display-buffer(#<buffer window.c> (nil (inhibit-same-window . t)))
   gud-display-line("/home/martin/emacs-git/trunk/src/window.c" 5158)
   gud-display-frame()
   gdb-frame-handler()
   gdb-handle-reply(96)
   gdb-done-or-error("96" done "frame={level=\"0\",addr=\"0x00000000005b581d\",func=\"F..." t)
   gdb-done("96" "frame={level=\"0\",addr=\"0x00000000005b581d\",func=\"F..." t)
   gdbmi-bnf-incomplete-record-result("96" (gdb-done . progressive))
   #f(compiled-function () #<bytecode 0x1e0c1a7de807>)()
   gdbmi-bnf-result-and-async-record-impl()
   gdbmi-bnf-async-record()
   gdbmi-bnf-out-of-band-record()
   gdbmi-bnf-output()
   gud-gdbmi-marker-filter("96^done,frame={level=\"0\",addr=\"0x00000000005b581d\"...")
   apply(gud-gdbmi-marker-filter "96^done,frame={level=\"0\",addr=\"0x00000000005b581d\"...")
   gud-marker-filter("96^done,frame={level=\"0\",addr=\"0x00000000005b581d\"...")
   gud-filter(#<process gud-emacs> "96^done,frame={level=\"0\",addr=\"0x00000000005b581d\"...")

As you can see, 'gdb-frame-handler' calls 'gud-display-frame' which will
eventually display window.c according to the rules for 'display-buffer'.

 > If so, what would be the correct solution of this issue?

The 'display-buffer' call indicated above could be modified using the
'debug' approach I sketched in my first posting here.  Whether this
suffices to handle all of Yuan's cases is beyond my knowledge.

martin

Re: Extend gdb to filter registers

[martin rudalics]

 > As for the issue where gbd-mi creates new window when it doesn’t need
 > to, my fix is to make `gud-display-line` behaves like
 > `gdb-goto-breakpoint`: try `gdb-display-source-buffer` first (when
 > gdb-mi is enabled), then try `display-buffer`; instead of simply using
 > `display-buffer`.

It shouldn't harm to try that.  I'd still make it optional - maybe
someone wants to see the other buffers.

martin

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 19 Jan 2020 12:57:58 -0500
> Cc: rudalics@gmx.at,
>  juri@linkov.net,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  john@yates-sheets.org
> 
> > Frankly, given Martin's input, I don't understand how will that solve
> > the problem, because display-buffer is still being called, and so it
> > could under some circumstances still decide to create a new window.
> > 
> > Am I missing something?
> 
> The issue I’m having is that, under normal circumstances, which means there exists a source window, I expect gdb-mi to always reuse that source window (and not create new ones); however gdb-mi doesn’t do that when stepping. 
> 
> Note that in `gdb-goto-breakpoint`, `display-buffer` doesn’t run if `gdb-display-source-buffer` can find a source window.

But do we have any control on whether gdb-display-source-buffer will
find a source window where we want it to?  I'm not sure.

Re: Extend gdb to filter registers

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org, monnier@iro.umontreal.ca, john@yates-sheets.org,
>  Juri Linkov <juri@linkov.net>
> From: martin rudalics <rudalics@gmx.at>
> Date: Sun, 19 Jan 2020 19:30:20 +0100
> 
>  > As for the issue where gbd-mi creates new window when it doesn’t need
>  > to, my fix is to make `gud-display-line` behaves like
>  > `gdb-goto-breakpoint`: try `gdb-display-source-buffer` first (when
>  > gdb-mi is enabled), then try `display-buffer`; instead of simply using
>  > `display-buffer`.
> 
> It shouldn't harm to try that.  I'd still make it optional - maybe
> someone wants to see the other buffers.

I tend to agree with the last bit.

Re: Extend gdb to filter registers

[Yuan Fu]

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

> But do we have any control on whether gdb-display-source-buffer will
> find a source window where we want it to?  I'm not sure.

I don’t quite understand. gdb-display-source-buffer finds source window when there is one. What do you mean by “control” and “where we want it to”?

>> It shouldn't harm to try that.  I'd still make it optional - maybe
>> someone wants to see the other buffers.
> 
> I tend to agree with the last bit.

At the very least I want to fix the inconsistency in gdb-mi — goto-breakpoint uses `(or gdb-display-source-buffer display-buffer)` and gud-display-line uses `display-buffer`. We can modify the display logic to allow user customization once gdb-mi is consistent within itself.

Yuan

[-- Attachment #2: Type: text/html, Size: 2686 bytes --]

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 19 Jan 2020 14:35:49 -0500
> Cc: martin rudalics <rudalics@gmx.at>,
>  Juri Linkov <juri@linkov.net>,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  john@yates-sheets.org
> 
>  But do we have any control on whether gdb-display-source-buffer will
>  find a source window where we want it to?  I'm not sure.
> 
> I don’t quite understand. gdb-display-source-buffer finds source window when there is one. What do you
> mean by “control” and “where we want it to”?

I mean when gdb-display-source-buffer will fail to find the source
window?

>  It shouldn't harm to try that.  I'd still make it optional - maybe
>  someone wants to see the other buffers.
> 
>  I tend to agree with the last bit.
> 
> At the very least I want to fix the inconsistency in gdb-mi — goto-breakpoint uses `(or
> gdb-display-source-buffer display-buffer)` and gud-display-line uses `display-buffer`. We can modify the
> display logic to allow user customization once gdb-mi is consistent within itself.

Let's add a defcustom to control this new behavior.

Re: Extend gdb to filter registers

[Yuan Fu]

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

> I mean when gdb-display-source-buffer will fail to find the source
> window?

I dug into gud.el and gdb-mi.el to see what’s really going on. It’s not very straightforward. Let me present my understanding of the logic below. Please correct me if I’m wrong.

First, “frame” in gud is a cons of file name and line, basically at where gdb stops (file:line).

When gud needs to display the current line at where gdb stops at (“frame”), it tries to display it in a new window if the target buffer is not already shown. `gud-last-frame` is set to the frame we want to display, and we call `gud-display-frame` to display it. `gud-display-frame` calls `gud-display-line` calls `display-buffer` with `(inhibit-same-window . t)`. Then it sets `gud-last-last-frame` to the frame just displayed, and `gdb-last-frame` to nil. So `last-frame` is basically the frame to be displayed, and `last-last-frame` is the last displayed frame.

As for gdb-mi, in `gdb-display-source-buffer`, it first looks for the window displaying `gud-last-last-frame` (last displayed frame, that is). If that doesn’t exist, it looks for `gdb-source-window` and use that. Either way it sets the window found to `gdb-source-window`. And it returns nil if no last-last-frame window nor `gdb-source-window` is found. How can `gdb-source-window` be set? There are two ways, 1) when gdb-mi starts with many-windows or show-main option, it opens a source window and set that to `gdb-source-window`, 2) if `gdb-display-source-window` finds a window displaying `gud-last-last-frame`, that window will be set to `gdb-source-window`. 

Gdb-mi also uses `gud-display-frame`: when I type commands in comint buffer, the result from gdb is handled by `gdb-frame-handler`, which sets `gud-last-frame` and calls `gud-display-frame`.

So, commands like step, next, etc, uses `gud-display-frame`. Except that `gdb-goto-breakpoint` uses gdb-display-source.

So it seems the intension is to display source in new window, not using only one window as I originally thought. I’d like to simplify it, put the decision logic into one place. And then we can add customizability to it.

>> It shouldn't harm to try that.  I'd still make it optional - maybe
>> someone wants to see the other buffers.
>> 
>> I tend to agree with the last bit.
>> 
>> At the very least I want to fix the inconsistency in gdb-mi — goto-breakpoint uses `(or
>> gdb-display-source-buffer display-buffer)` and gud-display-line uses `display-buffer`. We can modify the
>> display logic to allow user customization once gdb-mi is consistent within itself.
> 
> Let's add a defcustom to control this new behavior.


Do you mean options like `only-one-window`, `switch-between-two`, `as-much-as-possible`?

Yuan


[-- Attachment #2: Type: text/html, Size: 6191 bytes --]

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sun, 19 Jan 2020 20:22:38 -0500
> Cc: martin rudalics <rudalics@gmx.at>,
>  juri@linkov.net,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  john@yates-sheets.org
> 
> So it seems the intension is to display source in new window, not using only one window as I originally
> thought. I’d like to simplify it, put the decision logic into one place. And then we can add customizability to it.

Thanks, sounds like a good plan to me.

Re: Extend gdb to filter registers

[Yuan Fu]

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

>> So it seems the intension is to display source in new window, not using only one window as I originally
>> thought. I’d like to simplify it, put the decision logic into one place. And then we can add customizability to it.
> 
> Thanks, sounds like a good plan to me.

Here is my first attempt. I make every function that needs to display a source buffer (only 2: gdb-goto-breakpoint, gud-display-line) use gdb-display-source-buffer. Previously they either use (or gdb-display-source-buffer display-buffer) or use (display-buffer). 

Gdb-mi.el used to have a variable gdb-source-window, I changed it to gdb-source-window-list. And now reusing a window is as simple as looking at this list of windows. (No need to check for last-last-frame, etc). 

I added a variable for maximum number of windows used for source buffer. Right now the simple logic is to open as much windows as possible until the max is reached, then we start to reuse windows. Creating new window uses display-buffer-pop-up-window (I use this function just for completeness, I would modify this part when adding customization, maybe let user customize action for display-buffer?)

Overall this patch doesn’t change the behavior except 1) new window/not is now determined by gdb-maximum-source-window-count instead of display-buffer 2) now gud also uses gdb’s display function, so there is no check for gdb-mi vs gud anymore

You’ll also notice that, when you quit, all the source windows are left undeleted. That’s the original behavior and I have another patch that restores the old window configuration the user have before starting gdb. So I didn’t do anything about that.

If you think this patch is fine, I’ll do these next: 1) add a straightforward customization, preferably only one variable. 2) currently gdb opens windows everywhere, I want to make it open in only one continues area and maybe balance windows. Do you think this is worth doing? Or it is suffice to let user customize the display-buffer action?

Yuan


[-- Attachment #2: simplify.patch --]
[-- Type: application/octet-stream, Size: 6214 bytes --]

From 74a9a53ddf29b2caeeea0ddf3896418d59fada48 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Tue, 21 Jan 2020 16:23:57 -0500
Subject: [PATCH] Simplify gdb-mi/gud display source buffer logic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/progmodes/gdb-mi.el (gdb-source-window): remove
(gdb-source-window-list, gdb-max-source-window-count): new
(gdb-init-1, gdb-setup-windows, gdb-restore-windows):
change gdb-source-window to gdb-source-window-list
(gdb-display-source-buffer): use new logic
(gdb-goto-breakpoint): remove display-buffer
and don’t set gdb-source-buffer anymore

* lisp/progmodes/gud.el (gud-display-line): remove display-buffer
and don’t set gdb-source-buffer anymore; remove check for gdb-mi,
so we basically always set gdb-source-window-list
---
 lisp/progmodes/gdb-mi.el | 50 +++++++++++++++++++++++++---------------
 lisp/progmodes/gud.el    |  8 ++-----
 2 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index e4233dacaf..27031e24f7 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -211,7 +211,11 @@ gdb-handler-list
 (defvar gdb-source-file-list nil
   "List of source files for the current executable.")
 (defvar gdb-first-done-or-error t)
-(defvar gdb-source-window nil)
+(defvar gdb-source-window-list nil
+  "List of windows used for displaying source files.
+Sorted in most recently visited first order.")
+(defvar gdb-max-source-window-count 2
+  "Maximum number of source windows to use.")
 (defvar gdb-inferior-status nil)
 (defvar gdb-continuation nil)
 (defvar gdb-supports-non-stop nil)
@@ -922,7 +926,7 @@ gdb-init-1
 	gdb-first-done-or-error t
 	gdb-buffer-fringe-width (car (window-fringes))
 	gdb-debug-log nil
-	gdb-source-window nil
+	gdb-source-window-list nil
 	gdb-inferior-status nil
 	gdb-continuation nil
         gdb-buf-publisher '()
@@ -2002,17 +2006,29 @@ gdb-show-stop-p
 ;; GDB frame (after up, down etc).  If no GDB frame is visible but the last
 ;; visited breakpoint is, use that window.
 (defun gdb-display-source-buffer (buffer)
-  (let* ((last-window (if gud-last-last-frame
-                          (get-buffer-window
-                           (gud-find-file (car gud-last-last-frame)))))
-	 (source-window (or last-window
-			    (if (and gdb-source-window
-				     (window-live-p gdb-source-window))
-				gdb-source-window))))
-    (when source-window
-      (setq gdb-source-window source-window)
-      (set-window-buffer source-window buffer))
-    source-window))
+  "Find a window to display BUFFER.
+Always find a window to display buffer, and return it."
+  ;; doesn't take care of setting up source window(s)
+  ;; if `buffer' is already shown, use that window
+  (or (get-buffer-window buffer)
+      (progn
+        ;; first update window list
+        (setq gdb-source-window-list
+              (cl-remove-if-not #'window-live-p
+                                gdb-source-window-list))
+        ;; create new or reuse?
+        (if (> gdb-max-source-window-count
+               (length gdb-source-window-list))
+            ;; create new window, push to list, return it
+            (car (push (display-buffer-pop-up-window buffer nil)
+                       gdb-source-window-list))
+          ;; reuse, we use the oldest window and put that to front
+          (let ((last-win (car (last gdb-source-window-list)))
+                (rest (butlast gdb-source-window-list)))
+            (set-window-buffer last-win buffer)
+            (setq gdb-source-window-list
+                  (cons last-win rest))
+            last-win)))))
 
 
 (defun gdbmi-start-with (str offset match)
@@ -3981,9 +3997,7 @@ gdb-goto-breakpoint
               (let* ((buffer (find-file-noselect
                               (if (file-exists-p file) file
                                 (cdr (assoc bptno gdb-location-alist)))))
-                     (window (or (gdb-display-source-buffer buffer)
-                                 (display-buffer buffer))))
-                (setq gdb-source-window window)
+                     (window (gdb-display-source-buffer buffer)))
                 (with-current-buffer buffer
                   (goto-char (point-min))
                   (forward-line (1- (string-to-number line)))
@@ -4597,7 +4611,7 @@ gdb-setup-windows
          ;; Put buffer list in window if we
          ;; can't find a source file.
          (list-buffers-noselect))))
-    (setq gdb-source-window (selected-window))
+    (setq gdb-source-window-list (list (selected-window)))
     (let ((win4 (split-window-right)))
       (gdb-set-window-buffer
        (gdb-get-buffer-create 'gdb-inferior-io) nil win4))
@@ -4639,7 +4653,7 @@ gdb-restore-windows
          (if gud-last-last-frame
              (gud-find-file (car gud-last-last-frame))
            (gud-find-file gdb-main-file)))
-        (setq gdb-source-window win)))))
+        (setq gdb-source-window-list (list win))))))
 
 ;; Called from `gud-sentinel' in gud.el:
 (defun gdb-reset ()
diff --git a/lisp/progmodes/gud.el b/lisp/progmodes/gud.el
index 567f452b93..ccf9026417 100644
--- a/lisp/progmodes/gud.el
+++ b/lisp/progmodes/gud.el
@@ -2826,9 +2826,7 @@ gud-display-line
 	 (buffer
 	  (with-current-buffer gud-comint-buffer
 	    (gud-find-file true-file)))
-	 (window (and buffer
-		      (or (get-buffer-window buffer)
-			  (display-buffer buffer '(nil (inhibit-same-window . t))))))
+	 (window (when buffer (gdb-display-source-buffer buffer)))
 	 (pos))
     (when buffer
       (with-current-buffer buffer
@@ -2858,9 +2856,7 @@ gud-display-line
 	       (widen)
 	       (goto-char pos))))
       (when window
-	(set-window-point window gud-overlay-arrow-position)
-	(if (eq gud-minor-mode 'gdbmi)
-	    (setq gdb-source-window window))))))
+	(set-window-point window gud-overlay-arrow-position)))))
 
 ;; The gud-call function must do the right thing whether its invoking
 ;; keystroke is from the GUD buffer itself (via major-mode binding)
-- 
2.25.0

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Tue, 21 Jan 2020 20:50:46 -0500
> Cc: martin rudalics <rudalics@gmx.at>,
>  Juri Linkov <juri@linkov.net>,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  john@yates-sheets.org
> 
> Here is my first attempt. I make every function that needs to display a source buffer (only 2: gdb-goto-breakpoint, gud-display-line) use gdb-display-source-buffer. Previously they either use (or gdb-display-source-buffer display-buffer) or use (display-buffer). 

Thanks.

I have two comments/questions to this design:

  . I think "M-x gud-gdb" should be unaffected by these changes.  That
    command exists so that users who are unhappy with the new UI
    presented in gdb-mi.el, or have use cases that are insufficiently
    well supported by gdb-mi.el, so its workings must remain as they
    were historically.  Does your patch affect only gdb-mi.el?
  . Are we sure that all the places that now use
    gdb-display-source-buffer should indeed use the same logic?  From
    the names of the functions it seems the answer is YES, but could
    there be use cases where this isn't so?

I'm also slightly worried by the fact that previously this stuff
obeyed the display-buffer customizations, whereas after these changes
it won't.  Martin, any thoughts or comments?

> I added a variable for maximum number of windows used for source buffer. Right now the simple logic is to open as much windows as possible until the max is reached, then we start to reuse windows. Creating new window uses display-buffer-pop-up-window (I use this function just for completeness, I would modify this part when adding customization, maybe let user customize action for display-buffer?)

I don't understand this part: wasn't the original motivation to cause
gdb-mi to _always_ reuse the source window?

And in any case, I'm not sure a simple max number of windows is a
reasonable criterion for this functionality.  E.g., if I needed to set
the value of this variable, I wouldn't know what value to choose.

> If you think this patch is fine, I’ll do these next: 1) add a straightforward customization, preferably only one variable. 2) currently gdb opens windows everywhere, I want to make it open in only one continues area and maybe balance windows. Do you think this is worth doing? Or it is suffice to let user customize the display-buffer action?

My personal preference, and something I was always telling users who
expressed frustration with gdb-mi window handling, is to devote a
separate frame to gdb-mi windows.  This avoids messing up the user's
windows on other frames.  If enough people here agree with that,
perhaps we should move gdb-mi towards preferring such a modus
operandi?

Re: Extend gdb to filter registers

[Yuan Fu]

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



> On Jan 24, 2020, at 2:16 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Tue, 21 Jan 2020 20:50:46 -0500
>> Cc: martin rudalics <rudalics@gmx.at>,
>> Juri Linkov <juri@linkov.net>,
>> emacs-devel@gnu.org,
>> monnier@iro.umontreal.ca,
>> john@yates-sheets.org
>> 
>> Here is my first attempt. I make every function that needs to display a source buffer (only 2: gdb-goto-breakpoint, gud-display-line) use gdb-display-source-buffer. Previously they either use (or gdb-display-source-buffer display-buffer) or use (display-buffer). 
> 
> Thanks.
> 
> I have two comments/questions to this design:
> 
>  . I think "M-x gud-gdb" should be unaffected by these changes.  That
>    command exists so that users who are unhappy with the new UI
>    presented in gdb-mi.el, or have use cases that are insufficiently
>    well supported by gdb-mi.el, so its workings must remain as they
>    were historically.  Does your patch affect only gdb-mi.el?

No, but this can be easily fixed. I’ll fix that in the next patch.

>  . Are we sure that all the places that now use
>    gdb-display-source-buffer should indeed use the same logic?  From
>    the names of the functions it seems the answer is YES, but could
>    there be use cases where this isn't so?

In gdb-mi.el, there is only one function that used gbd-display-source-buffer — gdb-goto-breakpoint. In gud.el, there is also only one function, gud-display-frame calls gud-display-line calls gdb-display-source-buffer (after patch, before patch it calls display-buffer). And gud-display-frame is used to display a file:line. Overall I think it’s save. Of course you can never be so sure by just looking at the code and reason. But there is no test, and I don’t know how to write tests for interactive programs like this. Maybe you can give me some pointers on tests, if you think that’s vitally important.

> I'm also slightly worried by the fact that previously this stuff
> obeyed the display-buffer customizations, whereas after these changes
> it won't.  Martin, any thoughts or comments?

I still haven’t had a clear idea on “how to open the new window” part. In the previous patch I simply used display-window-pop-window so the code works, but we should definitely come up with something else. Currently my naive idea is to use (display-buffer buffer gdb-display-source-buffer-action), where gdb-display-source-buffer-action can be customized by user. WDYT?

> 
>> I added a variable for maximum number of windows used for source buffer. Right now the simple logic is to open as much windows as possible until the max is reached, then we start to reuse windows. Creating new window uses display-buffer-pop-up-window (I use this function just for completeness, I would modify this part when adding customization, maybe let user customize action for display-buffer?)
> 
> I don't understand this part: wasn't the original motivation to cause
> gdb-mi to _always_ reuse the source window?

You have the choice. If I want gdb to always reuse the window, I can set the number to 1. 

> And in any case, I'm not sure a simple max number of windows is a
> reasonable criterion for this functionality.  E.g., if I needed to set
> the value of this variable, I wouldn't know what value to choose.

I guess you mean this is somewhat ad-hoc. I agree. We can change it to options like “always reuse one”, “switch between 2”, “cycle between 3” and “as many as possible”, how about that? I.e., 

(defcustom gdb-source-window-number 'always-reuse-one
  :type 'symbol
  :options '(always-reuse-one switch-between-two
                              cycle-between-three
                              as-many-as-possible)
  :group 'gdb
  "Number of source windows gdb uses.”)

> 
>> If you think this patch is fine, I’ll do these next: 1) add a straightforward customization, preferably only one variable. 2) currently gdb opens windows everywhere, I want to make it open in only one continues area and maybe balance windows. Do you think this is worth doing? Or it is suffice to let user customize the display-buffer action?
> 
> My personal preference, and something I was always telling users who
> expressed frustration with gdb-mi window handling, is to devote a
> separate frame to gdb-mi windows.  This avoids messing up the user's
> windows on other frames.  If enough people here agree with that,
> perhaps we should move gdb-mi towards preferring such a modus
> operandi?

Are you referring to the complain that gdb messes up windows after it quits? That’s not hard to fix since we have window-configurations now. I have a patch that makes gdb preserves window configuration that the user had prior to starting gdb. As for opening a new frame, that solves the problem in someway, but what if someone don’t want to open a new frame and don’t want gdb messes his windows either? If you are talking about the problem where gdb opens new source windows when I don’t want it to, I don’t see now opening gdb in a new frame help that.

Yuan


[-- Attachment #2: Type: text/html, Size: 8057 bytes --]

Re: Extend gdb to filter registers

[John Yates]

I am trying to use gdb on my massive home screen.
It is no fun and I understand Yuan Fu's motivation.

That said I think that the current windowing model
has a surprising semantic for dedicated, namely that
a dedicated window is allowed to be split.   Were
there a way to make a window unsplittable (apart
from creating it on an unsplittable frame) then
gdb-setup-windows could use it.  I believe that
net effect would be all source displayed within the
source area (though possibly in multiple windows).

/john

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: John Yates <john@yates-sheets.org>
> Date: Fri, 24 Jan 2020 17:37:48 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>, Juri Linkov <juri@linkov.net>, 
> 	emacs-devel <emacs-devel@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>
> 
> That said I think that the current windowing model
> has a surprising semantic for dedicated, namely that
> a dedicated window is allowed to be split.

Can you describe a scenario under which this splitting happens?

Thanks.

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Fri, 24 Jan 2020 15:12:37 -0500
> Cc: martin rudalics <rudalics@gmx.at>,
>  Juri Linkov <juri@linkov.net>,
>  emacs-devel <emacs-devel@gnu.org>,
>  Stefan Monnier <monnier@iro.umontreal.ca>,
>  John Yates <john@yates-sheets.org>
> 
>  I'm also slightly worried by the fact that previously this stuff
>  obeyed the display-buffer customizations, whereas after these changes
>  it won't.  Martin, any thoughts or comments?
> 
> I still haven’t had a clear idea on “how to open the new window” part. In the previous patch I simply used
> display-window-pop-window so the code works, but we should definitely come up with something else.
> Currently my naive idea is to use (display-buffer buffer gdb-display-source-buffer-action), where
> gdb-display-source-buffer-action can be customized by user. WDYT?

Sounds OK, but I'd still like to hear martin's views on this.

>  I don't understand this part: wasn't the original motivation to cause
>  gdb-mi to _always_ reuse the source window?
> 
> You have the choice. If I want gdb to always reuse the window, I can set the number to 1. 

Yes, I think we should probably start at 1 and let users customize
that if they want to.

>  My personal preference, and something I was always telling users who
>  expressed frustration with gdb-mi window handling, is to devote a
>  separate frame to gdb-mi windows.  This avoids messing up the user's
>  windows on other frames.  If enough people here agree with that,
>  perhaps we should move gdb-mi towards preferring such a modus
>  operandi?
> 
> Are you referring to the complain that gdb messes up windows after it quits?

No, the usual complaint is that gdb-mi messes up windows during the
debugging session itself.  E.g., if you had several source files
displayed in carefully configured windows on a frame, starting
"M-x gdb" will typically mess up your window configuration on that
frame.  Using a separate frame works around that.

> That’s not hard to fix since we
> have window-configurations now. I have a patch that makes gdb preserves window configuration that the user
> had prior to starting gdb.

That could be another way, but running a debugging session usually
benefits from maximizing the frame.  Will your patch undo that as well?

Re: Extend gdb to filter registers

[martin rudalics]

 > I'm also slightly worried by the fact that previously this stuff
 > obeyed the display-buffer customizations, whereas after these changes
 > it won't.  Martin, any thoughts or comments?

+            (car (push (display-buffer-pop-up-window buffer nil)

violates this part of the doc-string of 'display-buffer-pop-up-window':

   This is an action function for buffer display, see Info
   node `(elisp) Buffer Display Action Functions'.  It should be
   called only by `display-buffer' or a function directly or
   indirectly called by the latter.

martin

Re: Extend gdb to filter registers

[martin rudalics]

 > In gdb-mi.el, there is only one function that used
 > gbd-display-source-buffer — gdb-goto-breakpoint.

Here with gdb-mi.el on Debian (and without your patch) I seem to note
one difference compared with my earlier setup on Windows: When hitting a
breakpoint, window point does _not_ move to the corresponding line in
the source file.  It moves there only after typing, for example, "n".
It would be nice to fix this but I cannot even guarantee that I can
reproduce the behavior here always and that the Windows setup worked
differently.

 > I still haven’t had a clear idea on “how to open the new window”
 > part. In the previous patch I simply used display-window-pop-window so
 > the code works, but we should definitely come up with something
 > else. Currently my naive idea is to use (display-buffer buffer
 > gdb-display-source-buffer-action), where
 > gdb-display-source-buffer-action can be customized by user. WDYT?

I think that could be a good idea.

martin

Re: Extend gdb to filter registers

[martin rudalics]

 > That said I think that the current windowing model
 > has a surprising semantic for dedicated, namely that
 > a dedicated window is allowed to be split.   Were
 > there a way to make a window unsplittable (apart
 > from creating it on an unsplittable frame) then
 > gdb-setup-windows could use it.  I believe that
 > net effect would be all source displayed within the
 > source area (though possibly in multiple windows).

I once thought so as well but Stefan convinced me of the contrary, see
this comment in window.el.

                        ;; Actually, even if the window is really dedicated,
                        ;; the frame is still usable by splitting it.
                        ;; At least Emacs-22 allowed it, and it is desirable
                        ;; when displaying same-frame windows.

Think of 'split-window' as of creating a new window somewhere near the
one of its argument.  And note that you can always split a frame's root
window instead or make your dedicated window fixed size.

martin

Re: Extend gdb to filter registers

[martin rudalics]

 >> I still haven’t had a clear idea on “how to open the new window” part. In the previous patch I simply used
 >> display-window-pop-window so the code works, but we should definitely come up with something else.
 >> Currently my naive idea is to use (display-buffer buffer gdb-display-source-buffer-action), where
 >> gdb-display-source-buffer-action can be customized by user. WDYT?
 >
 > Sounds OK, but I'd still like to hear martin's views on this.

I think gdb could provide customizations via one or more customizable
actions but always let users override them via their own buffer display
customizations.

 >>   I don't understand this part: wasn't the original motivation to cause
 >>   gdb-mi to _always_ reuse the source window?
 >>
 >> You have the choice. If I want gdb to always reuse the window, I can set the number to 1.
 >
 > Yes, I think we should probably start at 1 and let users customize
 > that if they want to.

I agree that having always one source window should be the, somewhat
austere, default behavior.

 > No, the usual complaint is that gdb-mi messes up windows during the
 > debugging session itself.  E.g., if you had several source files
 > displayed in carefully configured windows on a frame, starting
 > "M-x gdb" will typically mess up your window configuration on that
 > frame.  Using a separate frame works around that.

But only if you have not customized 'reusable-frames' and
'pop-up-frames'.

 > That could be another way, but running a debugging session usually
 > benefits from maximizing the frame.

Funnily, I always unmaximize my frame when running a debugging session.
How could I otherwise see the debuggee's frame?

martin

Re: Extend gdb to filter registers

[Eli Zaretskii]

> Cc: emacs-devel <emacs-devel@gnu.org>,
>  Stefan Monnier <monnier@iro.umontreal.ca>, John Yates
>  <john@yates-sheets.org>, Juri Linkov <juri@linkov.net>
> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 25 Jan 2020 09:43:28 +0100
> 
>  > In gdb-mi.el, there is only one function that used
>  > gbd-display-source-buffer — gdb-goto-breakpoint.
> 
> Here with gdb-mi.el on Debian (and without your patch) I seem to note
> one difference compared with my earlier setup on Windows: When hitting a
> breakpoint, window point does _not_ move to the corresponding line in
> the source file.  It moves there only after typing, for example, "n".

What version of GDB do you have there?

Re: Extend gdb to filter registers

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org, monnier@iro.umontreal.ca, john@yates-sheets.org,
>  juri@linkov.net
> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 25 Jan 2020 09:58:49 +0100
> 
>  > That could be another way, but running a debugging session usually
>  > benefits from maximizing the frame.
> 
> Funnily, I always unmaximize my frame when running a debugging session.
> How could I otherwise see the debuggee's frame?

OK, not "maximizing", but making it much larger than the "normal"
frames.

Re: Extend gdb to filter registers

[Eli Zaretskii]

> Date: Sat, 25 Jan 2020 12:25:13 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: juri@linkov.net, casouri@gmail.com, john@yates-sheets.org,
>  monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> OK, not "maximizing", but making it much larger than the "normal"
> frames.

And, of course, not all programs one debugs are GUI programs, in which
case the I/O window is enough, you don't need to see any other window
on display.

Re: Extend gdb to filter registers

[martin rudalics]

 > What version of GDB do you have there?

GNU gdb (Debian 7.12-6) 7.12.0.20161007-git

martin

Re: Extend gdb to filter registers

[Eli Zaretskii]

> Cc: casouri@gmail.com, emacs-devel@gnu.org, monnier@iro.umontreal.ca,
>  john@yates-sheets.org, juri@linkov.net
> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 25 Jan 2020 13:11:59 +0100
> 
>  > What version of GDB do you have there?
> 
> GNU gdb (Debian 7.12-6) 7.12.0.20161007-git

So it could be related to GDB itself, as 7.13 is somewhat old.

Re: Extend gdb to filter registers

[John Yates]

On my 3840x2160 emacs with a small font
a maximal window is 480 characters wide.
My split-width-threshold is 160 characters.
When gdb-mi creates its 6 windows each
is nearly 240 and hence clearly more than
my split-width-threshold.  As I visit different
source files each of gdb-mi's dedicated
windows gets split sprinkling my source
windows all over the screen.

Now when I start gdb-mi I bump s-w-t to
1000 and all of my troubles vanish :-)

/john

On Sat, Jan 25, 2020 at 2:45 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: John Yates <john@yates-sheets.org>
> > Date: Fri, 24 Jan 2020 17:37:48 -0500
> > Cc: Eli Zaretskii <eliz@gnu.org>, martin rudalics <rudalics@gmx.at>, Juri Linkov <juri@linkov.net>,
> >       emacs-devel <emacs-devel@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>
> >
> > That said I think that the current windowing model
> > has a surprising semantic for dedicated, namely that
> > a dedicated window is allowed to be split.
>
> Can you describe a scenario under which this splitting happens?
>
> Thanks.



-- 
John Yates
505 Tremont St, #803
Boston, MA 02116

Re: Extend gdb to filter registers

[John Yates]

There seem to be three concept in play here:
  - dedicated windows
  - really dedicated windows
  - unsplittable frames

gdb-mi creates dedicated windows which
under some circumstances (see my most
recent reply to Eli) still get split.

Your quote from Stefan seems to be about
really dedicated windows.

My comment that you quoted was about
the possibility of display-buffer splitting a
window on an unsplittable frame.  I admit
to having no actual experience with such
a configuration.  I was merely referencing
my understanding per info's Window Frame
Parameters > Buffer Parameters node:

‘unsplittable’
     If non-‘nil’, this frame’s window is never
     split automatically.

To reiterate my previous suggestion, I think
that creating stable window configurations
would be well served by adding a new
unsplittable window property.

/john

On Sat, Jan 25, 2020 at 3:43 AM martin rudalics <rudalics@gmx.at> wrote:
>
>  > That said I think that the current windowing model
>  > has a surprising semantic for dedicated, namely that
>  > a dedicated window is allowed to be split.   Were
>  > there a way to make a window unsplittable (apart
>  > from creating it on an unsplittable frame) then
>  > gdb-setup-windows could use it.  I believe that
>  > net effect would be all source displayed within the
>  > source area (though possibly in multiple windows).
>
> I once thought so as well but Stefan convinced me of the contrary, see
> this comment in window.el.
>
>                         ;; Actually, even if the window is really dedicated,
>                         ;; the frame is still usable by splitting it.
>                         ;; At least Emacs-22 allowed it, and it is desirable
>                         ;; when displaying same-frame windows.
>
> Think of 'split-window' as of creating a new window somewhere near the
> one of its argument.  And note that you can always split a frame's root
> window instead or make your dedicated window fixed size.
>
> martin



-- 
John Yates
505 Tremont St, #803
Boston, MA 02116

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: John Yates <john@yates-sheets.org>
> Date: Sat, 25 Jan 2020 09:33:34 -0500
> Cc: Yuan Fu <casouri@gmail.com>, martin rudalics <rudalics@gmx.at>, Juri Linkov <juri@linkov.net>, 
> 	Emacs developers <emacs-devel@gnu.org>, Stefan Monnier <monnier@iro.umontreal.ca>
> 
> On my 3840x2160 emacs with a small font
> a maximal window is 480 characters wide.
> My split-width-threshold is 160 characters.
> When gdb-mi creates its 6 windows each
> is nearly 240 and hence clearly more than
> my split-width-threshold.  As I visit different
> source files each of gdb-mi's dedicated
> windows gets split sprinkling my source
> windows all over the screen.

OK, then I think the changes discussed here will solve your problem.

Thanks.

Re: Extend gdb to filter registers

[martin rudalics]

 > There seem to be three concept in play here:
 >    - dedicated windows
 >    - really dedicated windows
 >    - unsplittable frames
 >
 > gdb-mi creates dedicated windows which
 > under some circumstances (see my most
 > recent reply to Eli) still get split.
 >
 > Your quote from Stefan seems to be about
 > really dedicated windows.
 >
 > My comment that you quoted was about
 > the possibility of display-buffer splitting a
 > window on an unsplittable frame.  I admit
 > to having no actual experience with such
 > a configuration.  I was merely referencing
 > my understanding per info's Window Frame
 > Parameters > Buffer Parameters node:
 >
 > ‘unsplittable’
 >       If non-‘nil’, this frame’s window is never
 >       split automatically.

We might be miscommunicating here.  To summarize the current state of
things:

- Whether a window can be split does not depend on whether it is
   dedicated in some way or not.  Dedicatedness affects exclusively
   whether you can display another buffer in that window.

- As far as 'display-buffer' is concerned you can rule out splitting by
   (1) setting 'split-height-threshold' and 'split-width-threshold'
   accordingly (as with the trick you mentioned earlier), (2) making
   windows fixed-size, or (3) making the window's frame unsplittable.

A better alternative is to specify buffer display actions that do not
split windows or provide a suitable 'split-window-preferred-function'.

martin

Re: Extend gdb to filter registers

[John Yates]

On Sat, Jan 25, 2020 at 12:14 PM martin rudalics <rudalics@gmx.at> wrote:
>
> - Whether a window can be split does not depend on whether it is
>    dedicated in some way or not.  Dedicatedness affects exclusively
>    whether you can display another buffer in that window.

Yes, I know that.  My original point was that the semantics are
not intuitive.  A user writing some code to create an arrangement
of windows and then marking them dedicated could easily think
that those windows are thereafter stable,  that they will never be
split by display-buffer.  What is not called out in any documentation
that I have seen is that a dedicated window is still a candidate for
splitting; that some other property must protect it if one's  intent
is for it never to be split.

> A better alternative is to specify buffer display actions that do not
> split windows or provide a suitable 'split-window-preferred-function'.

I strongly disagree.  I view having to interact with display-buffer
as an ever fraught proposition.  I understand the motivation for
display-buffer's existence and the remarkable job that it does.

But when creating an arrangement of stable windows I much
prefer a declarative solution that clarifies my intent.

> (3) making the window's frame unsplittable.

I do not want to disable splitting altogether, only of certain
windows.  Thus an unsplittable frame solution is not practical

> (2) making windows fixed-size

Making a windows fixed-size seems - at best - a kludge.
It provides no way to state my intent.  Instead I achieve
that intent - namely preventing certain windows from
getting split - indirectly by specifying an unrelated property.
The dissonance here is that I actually do _not_ want my
windows (in this case gdb-mi's dedicated windows) to
be fixed-size.  I regularly resize those windows by dragging
their  borders.  Would that continue to be possible once I
make the windows fixed size?

> (1) setting 'split-height-threshold' and 'split-width-threshold'
> accordingly

Using ridiculously large splitting thresholds does allow
me to have stable windows that never get split while
continuing to be resizeable via dragging.  The unfortunate
side-effect is that it prevents display-buffer from managing
the large amount of screen space I have provided for
displaying source buffers.

An unsplittable window property sure seems appealing :-)

The semantics are straight forward.  And I find it hard to
imagine that the implementation would be very difficult.
I expect that the biggest efforts would be documentation,
NEWS, etc.

/john

Re: Extend gdb to filter registers

[Yuan Fu]

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

> 
>> I don't understand this part: wasn't the original motivation to cause
>> gdb-mi to _always_ reuse the source window?
>> 
>> You have the choice. If I want gdb to always reuse the window, I can set the number to 1. 
> 
> Yes, I think we should probably start at 1 and let users customize
> that if they want to.

Sure. So we are still using numbers (instead of symbols like `only-one-window` and `switch-between-two`, etc)?

> 
>> My personal preference, and something I was always telling users who
>> expressed frustration with gdb-mi window handling, is to devote a
>> separate frame to gdb-mi windows.  This avoids messing up the user's
>> windows on other frames.  If enough people here agree with that,
>> perhaps we should move gdb-mi towards preferring such a modus
>> operandi?
>> 
>> Are you referring to the complain that gdb messes up windows after it quits?
> 
> No, the usual complaint is that gdb-mi messes up windows during the
> debugging session itself.  E.g., if you had several source files
> displayed in carefully configured windows on a frame, starting
> "M-x gdb" will typically mess up your window configuration on that
> frame.  Using a separate frame works around that.

>> That’s not hard to fix since we
>> have window-configurations now. I have a patch that makes gdb preserves window configuration that the user
>> had prior to starting gdb.
> 
> That could be another way, but running a debugging session usually
> benefits from maximizing the frame.  Will your patch undo that as well?

No, I simply save the window configuration at gdb start up by `window-state-get` and restores it when gdb quits. If gdb doesn’t maximize the frame, it shouldn’t un-maximize the frame after it quits either. If you want to make gdb maximize the frame by default, then gdb should un-maximize the frame after it quits. What I want to say is, who makes the change, who is responsible for it.

> >> I still haven’t had a clear idea on “how to open the new window” part. In the previous patch I simply used
> >> display-window-pop-window so the code works, but we should definitely come up with something else.
> >> Currently my naive idea is to use (display-buffer buffer gdb-display-source-buffer-action), where
> >> gdb-display-source-buffer-action can be customized by user. WDYT?
> >
> > Sounds OK, but I'd still like to hear martin's views on this.
> 
> I think gdb could provide customizations via one or more customizable
> actions but always let users override them via their own buffer display
> customizations.

Do you mean user should be able to overwrite gdb-display-source-buffer-action as in (display-buffer gdb-display-source-buffer-action)? He is free to do so like any other (custom) variables. If you mean rules in `display-buffer-alist` should take precedence over gdb-display-source-buffer-action, I disagree. Because a source buffer in gdb setting is different from a normal buffer — it shouldn’t be surprising that source buffers display differently in gdb. Or, think of gdb as a special case, and special case normally take precedence over normal case.


As for John’s idea, un-splittable window property, I think it makes sense and would certainly be useful. But I suspect that the implementation is not as easy as that.


Below is the updates since last patch. Please apply this on top of the last patch. I figure this way you see the updates better. Now gud has the old behavior, `gdb-display-source-buffer` uses (display-buffer gdb-display-source-buffer-action), and I added two custom variable `gdb-max-source-window-count`(default to 1) and `gdb-display-source-buffer-action`.

Yuan


[-- Attachment #2: simplify-update1.patch --]
[-- Type: application/octet-stream, Size: 2922 bytes --]

From 0c54015a2264830ceb34f9458a4ebfbef92d7b70 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sat, 25 Jan 2020 17:27:32 -0500
Subject: [PATCH] Simplify update 1

---
 lisp/progmodes/gdb-mi.el | 19 ++++++++++++++++---
 lisp/progmodes/gud.el    |  6 +++++-
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 27031e24f7..db1f544077 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -214,8 +214,6 @@ gdb-first-done-or-error
 (defvar gdb-source-window-list nil
   "List of windows used for displaying source files.
 Sorted in most recently visited first order.")
-(defvar gdb-max-source-window-count 2
-  "Maximum number of source windows to use.")
 (defvar gdb-inferior-status nil)
 (defvar gdb-continuation nil)
 (defvar gdb-supports-non-stop nil)
@@ -593,6 +591,21 @@ gdb-show-main
   :group 'gdb
   :version "22.1")
 
+(defcustom gdb-display-source-buffer-action nil
+  "`display-buffer' action used when GDB displaying a source buffer."
+  :type 'list
+  :group 'gdb
+  :version "28.1")
+
+(defcustom gdb-max-source-window-count 1
+  "Maximum number of source windows to use.
+Until there are such number of source windows on screen, GDB
+tries to open a new window when visiting a new source file; if
+there are, GDB will start to reuse existing source windows."
+  :type 'number
+  :grou 'gdb
+  :version "28.1")
+
 (defvar gdbmi-debug-mode nil
   "When non-nil, print the messages sent/received from GDB/MI in *Messages*.")
 
@@ -2020,7 +2033,7 @@ gdb-display-source-buffer
         (if (> gdb-max-source-window-count
                (length gdb-source-window-list))
             ;; create new window, push to list, return it
-            (car (push (display-buffer-pop-up-window buffer nil)
+            (car (push (display-buffer buffer gdb-display-source-buffer-action)
                        gdb-source-window-list))
           ;; reuse, we use the oldest window and put that to front
           (let ((last-win (car (last gdb-source-window-list)))
diff --git a/lisp/progmodes/gud.el b/lisp/progmodes/gud.el
index ccf9026417..8189394c60 100644
--- a/lisp/progmodes/gud.el
+++ b/lisp/progmodes/gud.el
@@ -2826,7 +2826,11 @@ gud-display-line
 	 (buffer
 	  (with-current-buffer gud-comint-buffer
 	    (gud-find-file true-file)))
-	 (window (when buffer (gdb-display-source-buffer buffer)))
+	 (window (when buffer (if (eq gud-minor-mode 'gdb-mi)
+                                  (gdb-display-source-buffer buffer)
+                                ;; we don't change the old behavior for gud
+                                (or (get-buffer-window buffer)
+                                    (display-buffer buffer '(nil (inhibit-same-window . t)))))))
 	 (pos))
     (when buffer
       (with-current-buffer buffer
-- 
2.25.0


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

Re: Extend gdb to filter registers

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > That said I think that the current windowing model
  > has a surprising semantic for dedicated, namely that
  > a dedicated window is allowed to be split.

That does surprise me -- but with so many possible cases, can we be sure
that it isn't sometime desirable to split those windows?

It should be easy to add a global variable to control this.
Its default could tentatively be not to split dedicated windows.
If anyone is disappointed with that change, we would find out.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

RE: Extend gdb to filter registers

[Drew Adams]

>   > That said I think that the current windowing model
>   > has a surprising semantic for dedicated, namely that
>   > a dedicated window is allowed to be split.
> 
> That does surprise me -- but with so many possible cases, can we be
> sure that it isn't sometime desirable to split those windows?
> 
> It should be easy to add a global variable to control this.
> Its default could tentatively be not to split dedicated windows.
> If anyone is disappointed with that change, we would find out.

I apologize for jumping in here out of the blue,
and without even have read the (long) thread.
(I don't use gdb.)

I'd just like to say this -

I use dedicated windows for all buffers, such as
`*Messages*', that have names like `*...*', and I've
long done so.  For me, they are "special-display"
buffers.

I've never had any idea that one should not be able
to split a dedicated window.  And occasionally I've
done so.

To me, a window being dedicated is only about the
association between a window and a buffer.  It's
not about also not being able to split or delete
the window.

I believe that's what the doc says too, and that's
always been the behavior.  Is there a good reason
why that should change?

I don't want to join a discussion about the question.
I just wanted to mention my experience and raise the
question, in case it hasn't been raised explicitly
in the thread.

If I had my druthers, I think I'd prefer that the
behavior remain as it's been, and if someone needs
something like a dedicated window that also cannot
be split then we create something else for that.

I know that Emacs has already created a second kind
of dedication - weakly dedicated.  Perhaps a third
kind should be created to handle the (apparent?)
use of case of strongly dedicated + unsplittable.
Just a thought, in passing.

The alternative of having a global variable whose
current value determines whether _all_ dedicated
windows are (currently) splittable, which is what I
guess Richard suggested, seems worse to me.  But I
haven't followed the discussion, so I don't really
appreciate the need, i.e., use case.

I'll butt out now.

Re: Extend gdb to filter registers

[Yuan Fu]

I think John suggests a new window property that makes it un-splittable. And the semantic of window-dedication is left unchanged. 

Yuan

Re: Extend gdb to filter registers

[martin rudalics]

 > I do not want to disable splitting altogether, only of certain
 > windows.

You always can do that by setting the 'split-window' parameter of those
windows to 'ignore'.

 > Thus an unsplittable frame solution is not practical

Maybe you should put those windows in an atomic window then.

 >> (2) making windows fixed-size
 >
 > Making a windows fixed-size seems - at best - a kludge.
 > It provides no way to state my intent.  Instead I achieve
 > that intent - namely preventing certain windows from
 > getting split - indirectly by specifying an unrelated property.
 > The dissonance here is that I actually do _not_ want my
 > windows (in this case gdb-mi's dedicated windows) to
 > be fixed-size.  I regularly resize those windows by dragging
 > their  borders.  Would that continue to be possible once I
 > make the windows fixed size?

No.

 >> (1) setting 'split-height-threshold' and 'split-width-threshold'
 >> accordingly
 >
 > Using ridiculously large splitting thresholds does allow
 > me to have stable windows that never get split while
 > continuing to be resizeable via dragging.  The unfortunate
 > side-effect is that it prevents display-buffer from managing
 > the large amount of screen space I have provided for
 > displaying source buffers.

Right.

 > An unsplittable window property sure seems appealing :-)
 >
 > The semantics are straight forward.  And I find it hard to
 > imagine that the implementation would be very difficult.
 > I expect that the biggest efforts would be documentation,
 > NEWS, etc.

If all you want is to make a window really "unsplittable", the
'split-window' parameter mentioned above should accomplish that.  I
suppose though, that you want to make such windows unsplittable for
'display-buffer' only.  And this will cause real problems because how
should 'display-buffer-below-selected' or 'display-buffer-at-bottom'
handle such a case?

martin

Re: Extend gdb to filter registers

[martin rudalics]

 > If you mean rules in `display-buffer-alist` should
 > take precedence over gdb-display-source-buffer-action, I
 > disagree. Because a source buffer in gdb setting is different from a
 > normal buffer — it shouldn’t be surprising that source buffers display
 > differently in gdb. Or, think of gdb as a special case, and special
 > case normally take precedence over normal case.

If you want to use 'display-buffer' and do not set
'display-buffer-overriding-action', you cannot override
'display-buffer-alist'.

martin

Re: Extend gdb to filter registers

[Yuan Fu]

> On Jan 26, 2020, at 3:42 AM, martin rudalics <rudalics@gmx.at> wrote:
> 
> > If you mean rules in `display-buffer-alist` should
> > take precedence over gdb-display-source-buffer-action, I
> > disagree. Because a source buffer in gdb setting is different from a
> > normal buffer — it shouldn’t be surprising that source buffers display
> > differently in gdb. Or, think of gdb as a special case, and special
> > case normally take precedence over normal case.
> 
> If you want to use 'display-buffer' and do not set
> 'display-buffer-overriding-action', you cannot override
> 'display-buffer-alist’.
> 
> martin
> 


I has a second look at the docstring of `display-buffer`, yes `display-buffer-alist` overrides supplied ACTION argument. Then the user is always able to override our display buffer customizations.

> If all you want is to make a window really "unsplittable", the
> 'split-window' parameter mentioned above should accomplish that.  I
> suppose though, that you want to make such windows unsplittable for
> 'display-buffer' only.  And this will cause real problems because how
> should 'display-buffer-below-selected' or 'display-buffer-at-bottom'
> handle such a case?


Maybe we could provide a customization to make gdb function windows (breakpoint, io, thread, etc) un-splittable?

Yuan

Re: Extend gdb to filter registers

[martin rudalics]

 > I has a second look at the docstring of `display-buffer`, yes
 > `display-buffer-alist` overrides supplied ACTION argument. Then the
 > user is always able to override our display buffer customizations.

Right.  An application can always re-override the user by using
'display-buffer-overriding-action' but that should be a last remedy
only.

 > Maybe we could provide a customization to make gdb function windows
 > (breakpoint, io, thread, etc) un-splittable?

I think we should do two things.

(1) Provide a simple interface for users who don't need the function
windows.  The aim here is to avoid that new windows pop up wildly
whenever hitting a new breakpoint or during stepping.

(2) Provide a more sophisticated window layout showing all sorts of
function windows using side windows.  In particular, more "flat" windows
like the breakpoint, locals, stack frames and maybe the io window should
appear at the bottom and top of the frame.  The gud window would appear
on the left or right (the io window could then go to the opposite side).

The center of the frame would be reserved for the source window or
whatever the user wants to show there.  This way, 'display-buffer'
reaardless of whether it's called by gdb or anyone else would never
mangle those function windows - neither split nor delete or reuse them
arbitrarily - unless the user explicitly wants to do that.

martin

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Date: Sat, 25 Jan 2020 23:18:27 -0500
> Cc: casouri@gmail.com, juri@linkov.net, rudalics@gmx.at,
>  monnier@iro.umontreal.ca, eliz@gnu.org, emacs-devel@gnu.org
> 
>   > That said I think that the current windowing model
>   > has a surprising semantic for dedicated, namely that
>   > a dedicated window is allowed to be split.
> 
> That does surprise me -- but with so many possible cases, can we be sure
> that it isn't sometime desirable to split those windows?

FWIW, it doesn't surprise me: splitting a window still leaves the
buffer displayed in one of the two windows, so the contract promised
by "dedicated" still holds.  We just made another window where there
was none before, that's all.

So yes, I think the ability to split such windows can be desirable 9in
some use cases.

Re: Extend gdb to filter registers

[Yuan Fu]

> On Jan 26, 2020, at 11:57 AM, martin rudalics <rudalics@gmx.at> wrote:
> 
> > I has a second look at the docstring of `display-buffer`, yes
> > `display-buffer-alist` overrides supplied ACTION argument. Then the
> > user is always able to override our display buffer customizations.
> 
> Right.  An application can always re-override the user by using
> 'display-buffer-overriding-action' but that should be a last remedy
> only.
> 
> > Maybe we could provide a customization to make gdb function windows
> > (breakpoint, io, thread, etc) un-splittable?
> 
> I think we should do two things.
> 
> (1) Provide a simple interface for users who don't need the function
> windows.  The aim here is to avoid that new windows pop up wildly
> whenever hitting a new breakpoint or during stepping.

Not sure if I understand you correctly, but AFAICT setting gdb-many-windows to nil (default value) and our new variable `gdb-max-source-window-count` to 1 (default value) should achieve that. You need to set `gdb-show-main` to t (not default) to show source file tho.

> (2) Provide a more sophisticated window layout showing all sorts of
> function windows using side windows.  In particular, more "flat" windows
> like the breakpoint, locals, stack frames and maybe the io window should
> appear at the bottom and top of the frame.  The gud window would appear
> on the left or right (the io window could then go to the opposite side).
> 
> The center of the frame would be reserved for the source window or
> whatever the user wants to show there.  This way, 'display-buffer'
> reaardless of whether it's called by gdb or anyone else would never
> mangle those function windows - neither split nor delete or reuse them
> arbitrarily - unless the user explicitly wants to do that.

Currently gdb-many-windows gives a similar arrangement. And in my other patch the user can customize his layout and save it to a file, and later restore from it. So they have a lot of freedom in this aspect. Side windows are a bit more limiting. And if we add non-split property to function windows(breakpoint, io, etc), we can make sure all new source windows are created around the old source windows (instead of splitting function windows everywhere). That should work too.


Btw, I have collected a few patches and they all resides in individual branches, and it’s getting a bit confusing, especially when some of the patches are several months old. Any tricks/advice on working with a bunch of branches/patches?

BBtw, Could someone have a look at the old patches that I sent to bug tracker?

Yuan

Re: Extend gdb to filter registers

[martin rudalics]

 > Currently gdb-many-windows gives a similar arrangement. And in my
 > other patch the user can customize his layout and save it to a file,
 > and later restore from it. So they have a lot of freedom in this
 > aspect. Side windows are a bit more limiting.

In which regard?

 > And if we add non-split
 > property to function windows(breakpoint, io, etc), we can make sure
 > all new source windows are created around the old source windows
 > (instead of splitting function windows everywhere). That should work
 > too.

This can lead to strange results when 'display-buffer-at-bottom' or
'display-buffer-in-direction' are used.  Both may split internal
windows.

martin

Re: Extend gdb to filter registers

[Yuan Fu]

> On Jan 27, 2020, at 2:18 PM, martin rudalics <rudalics@gmx.at> wrote:
> 
> > Currently gdb-many-windows gives a similar arrangement. And in my
> > other patch the user can customize his layout and save it to a file,
> > and later restore from it. So they have a lot of freedom in this
> > aspect. Side windows are a bit more limiting.
> 
> In which regard?

Side window have to be on the side, so you can’t put a function window in the middle. And if a user manually layout his windows, they won’t be side windows. 

> 
> > And if we add non-split
> > property to function windows(breakpoint, io, etc), we can make sure
> > all new source windows are created around the old source windows
> > (instead of splitting function windows everywhere). That should work
> > too.
> 
> This can lead to strange results when 'display-buffer-at-bottom' or
> 'display-buffer-in-direction' are used.  Both may split internal
> windows.

That approach doesn’t work then, too bad. Is there any other way to limit the location of a to-be-created window?

Yuan

Re: Extend gdb to filter registers

[martin rudalics]

 > Side window have to be on the side, so you can’t put a function window
 > in the middle.

All this is on purpose.  It's the most simple approach to draw a divider
between a rectangular region of the frame called the main window where
users and applications can conceptually do whatever they want and a
fixed region outside that main window, laid out in a strict and
immutable fashion.  If such a division doesn't work for GDB's many
windows concept, I have no idea what to do.  Side windows were designed
to accomplish exactly tasks like this.

 > And if a user manually layout his windows, they won’t
 > be side windows.

In that case the user has to bear the consequences as with the basic
layout we already have now.

 > Is there any other way to limit the location of a to-be-created window?

The buffer display functions try to not limit locations.  Limiting
locations will sooner or later paint you into a corner where you find no
window to display your buffers.  The idea is rather to suggest which
locations can be used rather than suggesting which locations cannot be
used.

With the default settings, users can always use the recipe provided by
Eli: Put the gdb windows into a separate frame (optionally making that
frame unsplittable) and operate from there as long as you are debugging
only.

martin

Re: Extend gdb to filter registers

[Yuan Fu]

> On Jan 28, 2020, at 4:49 AM, martin rudalics <rudalics@gmx.at> wrote:
> 
> > Side window have to be on the side, so you can’t put a function window
> > in the middle.
> 
> All this is on purpose.  It's the most simple approach to draw a divider
> between a rectangular region of the frame called the main window where
> users and applications can conceptually do whatever they want and a
> fixed region outside that main window, laid out in a strict and
> immutable fashion.  If such a division doesn't work for GDB's many
> windows concept, I have no idea what to do.  Side windows were designed
> to accomplish exactly tasks like this.

Ok, that makes sense. I had a look at `display-buffer-in-side-window`, it’s a pretty big function. I guess it’s not easy to make an ordinary window into a side-window? 
> 
> > And if a user manually layout his windows, they won’t
> > be side windows.
> 
> In that case the user has to bear the consequences as with the basic
> layout we already have now.
> 
> > Is there any other way to limit the location of a to-be-created window?
> 
> The buffer display functions try to not limit locations.  Limiting
> locations will sooner or later paint you into a corner where you find no
> window to display your buffers.  The idea is rather to suggest which
> locations can be used rather than suggesting which locations cannot be
> used.
> 
> With the default settings, users can always use the recipe provided by
> Eli: Put the gdb windows into a separate frame (optionally making that
> frame unsplittable) and operate from there as long as you are debugging
> only.

These all make a lot of sense. I guess there is no easy way to simultaneously let user use arbitrary window layouts and use side window for function buffers (to prevent splitting).

I’ll let these patches sit for a while and wait for someone to review them. It seems not many people on emacs-devel use gdb-mi w/ many windows tho. What do you use?

Yuan

Re: Extend gdb to filter registers

[John Yates]

On Tue, Jan 28, 2020 at 3:01 PM Yuan Fu <casouri@gmail.com> wrote:
>
> It seems not many people on emacs-devel use gdb-mi w/ many
> windows tho. What do you use?

I do.  And learning that side windows never get split was precisely the
clue I needed to satisfy my itch.

/john

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Sat, 25 Jan 2020 17:34:13 -0500
> Cc: rudalics@gmx.at,
>  juri@linkov.net,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  john@yates-sheets.org
> 
> > No, the usual complaint is that gdb-mi messes up windows during the
> > debugging session itself.  E.g., if you had several source files
> > displayed in carefully configured windows on a frame, starting
> > "M-x gdb" will typically mess up your window configuration on that
> > frame.  Using a separate frame works around that.
> 
> >> That’s not hard to fix since we
> >> have window-configurations now. I have a patch that makes gdb preserves window configuration that the user
> >> had prior to starting gdb.
> > 
> > That could be another way, but running a debugging session usually
> > benefits from maximizing the frame.  Will your patch undo that as well?
> 
> No, I simply save the window configuration at gdb start up by `window-state-get` and restores it when gdb quits. If gdb doesn’t maximize the frame, it shouldn’t un-maximize the frame after it quits either. If you want to make gdb maximize the frame by default, then gdb should un-maximize the frame after it quits. What I want to say is, who makes the change, who is responsible for it.

But we already have gdb-restore-windows, so it sounds like users
already can restore their window configuration.  Why do we need
anything else?

Re: Extend gdb to filter registers

[Yuan Fu]

> On Jan 31, 2020, at 8:58 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Yuan Fu <casouri@gmail.com>
>> Date: Sat, 25 Jan 2020 17:34:13 -0500
>> Cc: rudalics@gmx.at,
>> juri@linkov.net,
>> emacs-devel@gnu.org,
>> monnier@iro.umontreal.ca,
>> john@yates-sheets.org
>> 
>>> No, the usual complaint is that gdb-mi messes up windows during the
>>> debugging session itself.  E.g., if you had several source files
>>> displayed in carefully configured windows on a frame, starting
>>> "M-x gdb" will typically mess up your window configuration on that
>>> frame.  Using a separate frame works around that.
>> 
>>>> That’s not hard to fix since we
>>>> have window-configurations now. I have a patch that makes gdb preserves window configuration that the user
>>>> had prior to starting gdb.
>>> 
>>> That could be another way, but running a debugging session usually
>>> benefits from maximizing the frame.  Will your patch undo that as well?
>> 
>> No, I simply save the window configuration at gdb start up by `window-state-get` and restores it when gdb quits. If gdb doesn’t maximize the frame, it shouldn’t un-maximize the frame after it quits either. If you want to make gdb maximize the frame by default, then gdb should un-maximize the frame after it quits. What I want to say is, who makes the change, who is responsible for it.
> 
> But we already have gdb-restore-windows, so it sounds like users
> already can restore their window configuration.  Why do we need
> anything else?

Hmmm, which window configuration are you referring to? gdb-restore-windows restores the window configuration you have after M-x gdb, not the one before M-x gdb. I want gdb to restore the latter after it quits.

Yuan

Re: Extend gdb to filter registers

[Eli Zaretskii]

> From: Yuan Fu <casouri@gmail.com>
> Date: Fri, 31 Jan 2020 10:25:34 -0500
> Cc: rudalics@gmx.at,
>  juri@linkov.net,
>  emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca,
>  john@yates-sheets.org
> 
> > But we already have gdb-restore-windows, so it sounds like users
> > already can restore their window configuration.  Why do we need
> > anything else?
> 
> Hmmm, which window configuration are you referring to? gdb-restore-windows restores the window configuration you have after M-x gdb, not the one before M-x gdb. I want gdb to restore the latter after it quits.

Ah, you are right.  Sorry for my confusion.