bug#39181: 27.0.50; [PATCH] Allow users to store & restore gdb-mi layout

[Yuan Fu <casouri@gmail.com>]

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

Here’s the patch that addresses Štěpán and Martin’s review:

Apart from fixing all the mentioned issues, I also moved “Restore Window Layout” button from “GDB-MI” menu to “GDB-WINDOWS” menu, since it fits there better; I also mentioned that this button works for the new user-supplied default layout, too, if a user sets `gdb-default-window-configuration-file’. 

As for the layout vs configuration, I used “layout” in UI buttons, and used “window configuration (i.e. window layout)” in help echos. Hopefully the user will understand that we use “window configuration” internally but it’s (almost) the same as a window layout in gdb. This also plays well with existing UI that uses “layout”. The window layout that we save to a file in gdb is a window configuration structure with some small differences: simply loading that window configuration with `window-state-put' doesn’t give back your gdb layout. (because all the buffers (local, breakpoint, etc) are different from session to session.) `gdb-load-window-configuration` does some trick to recreate each buffers. Apart from that the layout and window configuration is the same thing.

Martin suggests to mention the layout vs window configuration in the manual, should I also document this new feature in the same section? (Let me learn texinfo first ;-)

Yuan


[-- Attachment #2.1: Type: text/html, Size: 1807 bytes --]

[-- Attachment #2.2: new-window.patch --]
[-- Type: application/octet-stream, Size: 20057 bytes --]

From 59590d8a8b21908cefc10787029a6db1ae5fdfef Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Tue, 3 Mar 2020 18:30:03 -0500
Subject: [PATCH] Add store/restore window configuration feature for gdb-mi

Add a feature that allows a user to save a gdb window
configuration (window layout) to a file with
'gdb-save-window-configuration' and load it back with
'gdb-load-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.  Add an option to make gdb preserve the window configuration
that the user had before starting gdb.  In window.el, add
'with-selected-window-non-dedicated'.
* lisp/progmodes/gdb-mi.el (require): add 'pcase' and 'cl-seq'.
(gdb--window-configuration-before): New variable.
(gdb-restore-window-configuration-after-quit): New custom variable.
(gdb-window-configuration-directory,
gdb-default-window-configuration-file): New variables.
(gdb): Save configuration on startup.
(gud-menu-map): Add "Load Layout" and "Save Layout" to menu. Add
"Restore Layout After Quit" button to menu.  Rename "Restore Window
Layout" to "Restore Default Layout", add some help echo, and move it
from "GDB-MI" menu to "GDB-WINDOWs" menu.
(gdb-toggle-restore-window-configuration): New function.
(gdb-get-source-buffer): New function, extracted out of
'gdb-restore-window'.
(gdb-setup-windows): Add a condition branch that loads default window
configuration when available.
(gdb-buffer-p, gdb-function-buffer-p, gdb--buffer-type,
gdb-save-window-configuration, gdb-load-window-configuration): New
functions.
(gdb-restore-windows): Edit docstring to mention
'gdb-default-window-configuration-file' and add comments.
(gdb-reset): Restore window configuration after quit.
(gdb-get-source-file): Add comments.
* lisp/window.el (with-window-non-dedicated): New macro.
---
 lisp/progmodes/gdb-mi.el | 279 +++++++++++++++++++++++++++++++++------
 lisp/window.el           |  18 +++
 2 files changed, 259 insertions(+), 38 deletions(-)

diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index da5a2a503a..e1a2993b4e 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -92,6 +92,8 @@
 (require 'json)
 (require 'bindat)
 (require 'cl-lib)
+(require 'cl-seq)
+(eval-when-compile (require 'pcase))
 
 (declare-function speedbar-change-initial-expansion-list
                   "speedbar" (new-default))
@@ -253,6 +255,25 @@ 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-restore-window-configuration-after-quit nil
+  "Whether to restore the window configuration the user had before gdb starts.
+
+Possible values are:
+    t -- Always restore.
+    nil -- Don't restore.
+    `if-gdb-show-main' -- Restore only if variable `gdb-show-main' is non-nil
+    `if-gdb-many-windows' -- Restore only if variable `gdb-many-windows' is non-nil."
+  :type '(choice
+          (const :tag "Always restore" t)
+          (const :tag "Don't restore" nil)
+          (const :tag "Depends on `gdb-show-main'" 'if-gdb-show-main)
+          (const :tag "Depends on `gdb-many-windows'" 'if-gdb-many-windows))
+  :group 'gdb
+  :version "28.1")
+
 (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
@@ -603,6 +624,26 @@ gdb-show-main
   :group 'gdb
   :version "22.1")
 
+(defcustom gdb-window-configuration-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 the path to the window configuration file.  If the path
+is not an absolute path, GDB treats it as a relative path and
+looks under `gdb-window-configuration-directory'.
+
+Note that this variable only takes effect when variable
+`gdb-many-windows' is t."
+  :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*.")
 
@@ -761,6 +802,12 @@ 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. Save it regardless of the value of
+  ;; `gdb-restore-window-configuration-after-quit'.
+  (setq gdb--window-configuration-before (window-state-get))
+
   ;;
   (gud-common-init command-line nil 'gud-gdbmi-marker-filter)
 
@@ -4491,6 +4538,15 @@ gdb-preempt-existing-or-display-buffer
   (define-key gud-menu-map [displays]
     `(menu-item "GDB-Windows" ,menu
 		:visible (eq gud-minor-mode 'gdbmi)))
+  (define-key menu [gdb-restore-windows]
+    '(menu-item "Restore Default Layout" gdb-restore-windows
+      :help "Restore standard layout for debug session. I.e., the layout we get when session starts."))
+  (define-key menu [load-layout] '("Load Layout" "Load GDB window configuration (i.e. window layout) from a file" . gdb-load-window-configuration))
+  (define-key menu [save-layout] '("Save Layout" "Save current GDB window configuration (i.e. window layout) to a file" . gdb-save-window-configuration))
+  (define-key menu [restore-layout-after-quit]
+    '(menu-item "Restore Layout After Quit" gdb-toggle-restore-window-configuration
+       :button (:toggle . gdb-restore-window-configuration-after-quit)
+       :help "Toggle between always restore the window configuration (i.e. window layout) after GDB quits and never restore.\n You can also change this setting in Customize to conditionally restore."))
   (define-key menu [gdb] '("Gdb" . gdb-display-gdb-buffer))
   (define-key menu [threads] '("Threads" . gdb-display-threads-buffer))
   (define-key menu [memory] '("Memory" . gdb-display-memory-buffer))
@@ -4529,9 +4585,6 @@ gdb-preempt-existing-or-display-buffer
     '(menu-item "Display Other Windows" gdb-many-windows
       :help "Toggle display of locals, stack and breakpoint information"
       :button (:toggle . gdb-many-windows)))
-  (define-key menu [gdb-restore-windows]
-    '(menu-item "Restore Window Layout" gdb-restore-windows
-      :help "Restore standard layout for debug session."))
   (define-key menu [sep1]
     '(menu-item "--"))
   (define-key menu [all-threads]
@@ -4606,41 +4659,174 @@ gdb-set-window-buffer
   (set-window-buffer window (get-buffer name))
   (set-window-dedicated-p window t))
 
+(defun gdb-toggle-restore-window-configuration ()
+  "Toggle whether to restore window configuration when GDB quit."
+  (interactive)
+  (setq gdb-restore-window-configuration-after-quit
+        (not gdb-restore-window-configuration-after-quit)))
+
+(defun gdb-get-source-buffer ()
+  "Return a buffer displaying source file or nil if we can't find one.
+
+The source file is the file that contains the source location
+where GDB stops.  There could be multiple source files during a
+debugging session, we get the most recently showed one.  If
+program hasn't start running yet, the source file is the \"main
+file\" where the GDB session starts (see `gdb-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-load-window-configuration
+       (if (file-name-absolute-p gdb-default-window-configuration-file)
+           gdb-default-window-configuration-file
+         (expand-file-name gdb-default-window-configuration-file
+                           gdb-window-configuration-directory)))
+    ;; Create default layout as before.
+    (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.
+
+Function buffers are locals buffer, registers buffer, etc, but
+not including main command buffer (the one where you type GDB
+commands) or source buffers (that display program source code)."
+  (with-current-buffer buffer
+    (derived-mode-p 'gdb-parent-mode 'gdb-inferior-io-mode)))
+
+(defun gdb--buffer-type (buffer)
+  "Return the type of BUFFER if it is a function buffer.
+
+Buffer type is like `gdb-registers-type', `gdb-stack-buffer'.
+These symbols are used by `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-save-window-configuration (file)
+  "Save current window configuration (i.e. window layout) to FILE.
+
+You can later restore this configuration from that file by
+`gdb-load-window-configuration'."
+  (interactive (list (read-file-name
+                      "Save window configuration to file: "
+                      (or gdb-window-configuration-directory default-directory))))
+  ;; We replace the buffer in each window with a placeholder, store
+  ;; the buffer type (register, breakpoint, etc) in window parameters,
+  ;; and write the window configuration to the file.
+  (save-window-excursion
+    (let ((placeholder (get-buffer-create " *gdb-placeholder*"))
+          (window-persistent-parameters
+           (cons '(gdb-buffer-type . writable) window-persistent-parameters)))
+      (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-window-non-dedicated nil
+                (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-load-window-configuration (file)
+  "Restore window configuration (i.e. window layout) from FILE.
+
+FILE should be a window configuration file saved by
+`gdb-save-window-configuration'."
+  (interactive (list (read-file-name
+                      "Restore window configuration from file: "
+                      (or gdb-window-configuration-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)
+                               ;; We need to go to point-min because
+                               ;; `read' reads from point
+                               (goto-char (point-min))
+                               (read (current-buffer))))
+              (source-buffer (or (gdb-get-source-buffer)
+                                 ;; Do the same thing as in
+                                 ;; `gdb-setup-windows' if no source
+                                 ;; buffer is found.
+                                 (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-window-non-dedicated nil
+                       (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.
@@ -4658,7 +4844,12 @@ 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 arrangement depends on the value of option
+`gdb-many-windows' and `gdb-default-window-configuration-file'."
+  ;; This function is used when the user messed up window
+  ;; configuration and wants 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)
@@ -4705,11 +4896,23 @@ 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.
+  (when (or (eq gdb-restore-window-configuration-after-quit t)
+            (and (eq gdb-restore-window-configuration-after-quit 'if-gdb-show-main)
+                 gdb-show-main)
+            (and (eq gdb-restore-window-configuration-after-quit 'if-gdb-many-windows)
+                 gdb-many-windows))
+    (when gdb--window-configuration-before
+      (window-state-put gdb--window-configuration-before)
+      ;; This way we don't accidentally restore an outdated window
+      ;; configuration.
+      (setq gdb--window-configuration-before nil))))
 
 (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))))
diff --git a/lisp/window.el b/lisp/window.el
index bd825c09e1..d774d906cd 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -278,6 +278,24 @@ with-displayed-buffer-window
 	     (funcall ,vquit-function ,window ,value)
 	   ,value)))))
 
+(defmacro with-window-non-dedicated (window &rest body)
+  "Evaluate BODY with WINDOW selected and temporarily made non-dedicated.
+
+If WINDOW is nil, use the selected window.  Return the value of the last form in BODY."
+  (declare (indent 1) (debug t))
+  (let ((window-dedicated-sym (gensym))
+        (window-sym (gensym)))
+    `(let* ((,window-sym (window-normalize-window ,window t))
+            (,window-dedicated-sym (window-dedicated-p ,window-sym)))
+       (set-window-dedicated-p ,window-sym nil)
+       (unwind-protect
+           (progn ,@body)
+         ;; `window-dedicated-p' returns the value set by
+         ;; `set-window-dedicated-p', which differentiates non-nil and
+         ;; t, so we cannot simply use t here. That's why we use
+         ;; `window-dedicated-sym'.
+         (set-window-dedicated-p ,window-sym ,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.25.1


[-- Attachment #2.3: Type: text/html, Size: 151 bytes --]