bug#40268: 27.0.60; [PATCH] Unify and improve gdb-mi source window display

[Yuan Fu <casouri@gmail.com>]

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



> On Mar 31, 2020, at 5:15 AM, martin rudalics <rudalics@gmx.at> wrote:
> 
> > This one should be ok.
> 
> Thanks.  A few remarks.  Please try to ident as
> 
>        (setq gdb-source-window-list
>              (cl-remove-duplicates
>               (cl-remove-if-not
> 		(lambda (win)
> 		  (and (window-live-p win)
>                       (equal (window-frame win)
>                              (selected-frame))))
>                gdb-source-window-list)
>               :test #'equal))
> 
> so we can safely change this later and easily stay within our line
> length limits.  Also, tests like
> 
>                       (equal (window-frame win)
>                              (selected-frame))))
> 
> should use "eq" instead of "equal" and ones like
> 
>                     ((member (selected-window) gdb-source-window-list) 'source)))
> 
> "memq" instead of "member".
> 
> Similarly to the above, writing
> 
> 	 (window
> 	  (when buffer
> 	    (if (eq gud-minor-mode 'gdbmi)
>                (gdb-display-source-buffer buffer)
>              ;; Gud still has the old behavior.
>              (or (get-buffer-window buffer)
>                  (display-buffer buffer '(nil (inhibit-same-window . t)))))))
> 
> permits us to stay within the 80 columns limit.
> 
> Finally, instead of "Also, we simply the logic ..." I'd write "Also,
> simplify the logic ...".  And please provide a NEWS entry.
> 
> Thanks again for the work, Martin

Thanks for reviewing. Besides the changes you requested, I changed the last condition in the cond form in `gdb-save-window-configuration’ to a catch-all condition (to avoid weird problem when loading it back).

Yuan


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

From 41a8528515bbb9aeef6880ee3b7bc90320ab8b51 Mon Sep 17 00:00:00 2001
From: Yuan Fu <casouri@gmail.com>
Date: Sun, 29 Mar 2020 10:20:53 -0400
Subject: [PATCH] Unify and improve gdb-mi source buffer display logic

Unify the behavior of source buffer display for gdb-mi.  Before this
change, stepping and other gdb command handlers use 'gud-display-line',
and 'gdb-goto-breakpoint' uses 'gdb-display-source-buffer'.  Now whenever
gdb-mi code tries to open a source buffer, 'gdb-display-source-buffer'
is used.  Also, simplify the logic in 'gdb-display-source-buffer' and
add a feature to limit the maximum number of source windows.

* doc/emacs/building.texi (GDB User Interface Layout): Explain source
file display in GDB.
* etc/NEWS (gdb-mi): Add news about source display.
* lisp/progmodes/gdb-mi.el (gdb-source-window): Remove variable,
change to 'gdb-source-window-list'.
(gdb-source-window-list): New variable.
(gdb-display-source-buffer-action,
gdb-max-source-window-count): New custom variable.
(gdb-init-1, gdb-setup-windows, gdb-load-window-configuration,
gdb-restore-windows): Use 'gdb-source-window' rather than
'gdb-source-window-list'.
(gdb-save-window-configuration): Use 'gdb-source-window' rather than
'gdb-source-window-list'.  And consider any buffer that is not a
command or function buffer as a source buffer.
(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): If used by gdb-mi, use
'gdb-display-source-buffer' rather than 'display-buffer'.  Don't set
'gdb-source-buffer' anymore.
---
 doc/emacs/building.texi  |  8 +++++
 etc/NEWS                 |  6 ++++
 lisp/progmodes/gdb-mi.el | 75 +++++++++++++++++++++++++++++-----------
 lisp/progmodes/gud.el    | 14 ++++----
 4 files changed, 77 insertions(+), 26 deletions(-)

diff --git a/doc/emacs/building.texi b/doc/emacs/building.texi
index 8a05680c74..2c43323dc4 100644
--- a/doc/emacs/building.texi
+++ b/doc/emacs/building.texi
@@ -1022,6 +1022,14 @@ GDB User Interface Layout
 the same with the menu bar, with the @samp{GDB-Windows} and
 @samp{GDB-Frames} sub-menus of the @samp{GUD} menu.
 
+@vindex gdb-max-source-window-count
+@vindex gdb-display-source-buffer-action
+By default, GDB uses at most one window to display source file.  You
+can make it use more windows by customizing
+@code{gdb-max-source-window-count}.  You can also customize
+@code{gdb-display-source-buffer-action} to control how does GDB
+display source files.
+
   When you finish debugging, kill the GUD interaction buffer with
 @kbd{C-x k}, which will also kill all the buffers associated with the
 session.  However you need not do this if, after editing and
diff --git a/etc/NEWS b/etc/NEWS
index 4b477e5def..1260dfa30c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -213,6 +213,12 @@ will remember the window configuration before GDB started and restore
 it after GDB quits.  A toggle button is also provided under 'Gud --
 GDB-Windows'.
 
+---
+*** gdb-mi now has a better logic for displaying source buffers
+Now GDB only uses one source window to display source file by default.
+Customize 'gdb-max-source-window-count' to use more than one windows.
+Control source file display by 'gdb-display-source-buffer-action'.
+
 ** Gravatar
 
 ---
diff --git a/lisp/progmodes/gdb-mi.el b/lisp/progmodes/gdb-mi.el
index 7fb3687391..d513c7606d 100644
--- a/lisp/progmodes/gdb-mi.el
+++ b/lisp/progmodes/gdb-mi.el
@@ -224,7 +224,9 @@ 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-inferior-status nil)
 (defvar gdb-continuation nil)
 (defvar gdb-supports-non-stop nil)
@@ -645,6 +647,21 @@ gdb-default-window-configuration-file
   :group 'gdb
   :version "28.1")
 
+(defcustom gdb-display-source-buffer-action '(nil . ((inhibit-same-window . t)))
+  "`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; after
+that GDB starts to reuse existing source windows."
+  :type 'number
+  :group 'gdb
+  :version "28.1")
+
 (defvar gdbmi-debug-mode nil
   "When non-nil, print the messages sent/received from GDB/MI in *Messages*.")
 
@@ -984,7 +1001,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 '()
@@ -2069,17 +2086,36 @@ 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."
+  ;; This function doesn't take care of setting up source window(s) at startup,
+  ;; that's handled by `gdb-setup-windows' (if `gdb-many-windows' is non-nil).
+  ;; If `buffer' is already shown in a window, use that window.
+  (or (get-buffer-window buffer)
+      (progn
+        ;; First, update the window list.
+        (setq gdb-source-window-list
+              (cl-remove-duplicates
+               (cl-remove-if-not
+                (lambda (win)
+                  (and (window-live-p win)
+                       (eq (window-frame win)
+                           (selected-frame))))
+                gdb-source-window-list)))
+        ;; Should we create a new window or reuse one?
+        (if (> gdb-max-source-window-count
+               (length gdb-source-window-list))
+            ;; Create a new window, push it to window list and return it.
+            (car (push (display-buffer buffer gdb-display-source-buffer-action)
+                       gdb-source-window-list))
+          ;; Reuse a window, we use the oldest window and put that to
+          ;; the front of the window list.
+          (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)
@@ -4064,9 +4100,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)))
@@ -4715,7 +4749,7 @@ gdb-setup-windows
       (select-window win2)
       (set-window-buffer win2 (or (gdb-get-source-buffer)
                                   (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))
@@ -4791,7 +4825,8 @@ gdb-save-window-configuration
                           (error "Unrecognized gdb buffer mode: %s" major-mode)))
                      ;; Command buffer.
                      ((derived-mode-p 'gud-mode) 'command)
-                     ((equal (selected-window) gdb-source-window) 'source)))
+                     ;; Consider everything else as source buffer.
+                     (t 'source)))
               (with-window-non-dedicated nil
                 (set-window-buffer nil placeholder)
                 (set-window-prev-buffers (selected-window) nil)
@@ -4834,7 +4869,7 @@ gdb-load-window-configuration
               (pcase buffer-type
                 ('source (when source-buffer
                            (set-window-buffer nil source-buffer)
-                           (setq gdb-source-window (selected-window))))
+                           (push (selected-window) gdb-source-window-list)))
                 ('command (switch-to-buffer gud-comint-buffer))
                 (_ (let ((buffer (gdb-get-buffer-create buffer-type)))
                      (with-window-non-dedicated nil
@@ -4875,7 +4910,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..eb43e8b7e4 100644
--- a/lisp/progmodes/gud.el
+++ b/lisp/progmodes/gud.el
@@ -2826,9 +2826,13 @@ 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
+            (if (eq gud-minor-mode 'gdbmi)
+                (gdb-display-source-buffer buffer)
+              ;; Gud still has the old behavior.
+              (or (get-buffer-window buffer)
+                  (display-buffer buffer '(nil (inhibit-same-window . t)))))))
 	 (pos))
     (when buffer
       (with-current-buffer buffer
@@ -2858,9 +2862,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.1


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