Add edebug-bounce-to-previous-value

Command edebug-bounce-to-previous-value uses the previous value
observed while single-stepping or evaluating an expression to
bounce point in the outside current buffer to the buffer
position corresponding to that value.

* lisp/emacs-lisp/edebug.el (edebug-previous-value): Add
variable.
(edebug-compute-previous-result, edebug-eval-expression): Update
it.
(edebug-bounce-to-previous-value): Add command.
(edebug-mode-map): Add keybinding for the new command, replacing
the binding of "P" to edebug-view-outside.
(edebug-mode-menus): Add menu entry for the new command.
* doc/lispref/edebug.texi (Edebug Views): Add documentation.
* test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
(edebug-test-code-bounce-point): Add test code.
* test/lisp/emacs-lisp/edebug-tests.el
(edebug-tests-bounce-outside-buffer)
(edebug-tests-bounce-outside-point)
(edebug-tests-bounce-outside-mark)
(edebug-tests-bounce-record-outside-environment)
(edebug-tests-should-have-bounced-to): Add infrastructure to
test bounces.
(edebug-tests-check-keymap): Update tests to new key bindings.
(edebug-tests-bounce-point)
(edebug-tests-bounce-to-previous-value)
(edebug-tests-bounce-to-previous-non-position): Add tests.
(edebug-tests-evaluation-of-current-buffer-bug-19611): Clean up
side effects.  (Bug#79288)

5 files changed, 222 insertions, 22 deletions

diff --git a/doc/lispref/edebug.texi b/doc/lispref/edebug.texi
index f16190b85c9..97909e2bb55 100644
--- a/doc/lispref/edebug.texi
+++ b/doc/lispref/edebug.texi
@@ -677,8 +677,7 @@ effect outside of Edebug.
 
 @table @kbd
 @findex edebug-view-outside
-@item P
-@itemx v
+@item v
 Switch to viewing the outside window configuration
 (@code{edebug-view-outside}).  Type @kbd{C-x X w} to return to Edebug.
 
@@ -689,6 +688,17 @@ outside position (@code{edebug-bounce-point}), pausing for one second
 before returning to Edebug.  With a prefix argument @var{n}, pause for
 @var{n} seconds instead.
 
+@findex edebug-bounce-to-previous-value
+@item P
+Temporarily display the outside current buffer with the outside point
+corresponding to the previous value
+(@code{edebug-bounce-to-previous-value}).  The previous value is what
+Edebug has evaluated before its last stop point or what you have
+evaluated in the context outside of Edebug, for example, with
+@kbd{C-x C-e}.  This command pauses for one second before returning to
+Edebug.  With a prefix argument @var{n}, it pauses for @var{n} seconds
+instead.
+
 @findex edebug-where
 @item w
 Move point back to the current stop point in the source code buffer
@@ -713,6 +723,20 @@ source code buffer, you must use @kbd{C-x X W} from the global keymap.
 bounce to the point in the current buffer with @kbd{p}, even if
 it is not normally displayed.
 
+  You can also bounce to buffer positions other than the current point.
+Suppose you are debugging the form
+
+@example
+(make-overlay beg end)
+@end example
+
+@noindent
+and you would like to know where @code{beg} and @code{end} are located
+in the outside buffer.  Then you could either evaluate these, for
+example, with @kbd{C-x C-e}, or step over them with @kbd{n}, and
+immediately after that press @kbd{P}, to bounce to the position you have
+previously evaluated.
+
   After moving point, you may wish to jump back to the stop point.
 You can do that with @kbd{w} from a source code buffer.  You can jump
 back to the stop point in the source code buffer from any buffer using
diff --git a/etc/NEWS b/etc/NEWS
index 37d38d0d91d..c9f30dc7ef7 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2521,6 +2521,18 @@ If non-nil, FFAP always finds remote files in buffers with remote
 'default-directory'.  If nil, FFAP finds local files first for absolute
 file names in above buffers.  The default is nil.
 
+** Debugging
+
++++
+*** New command 'edebug-bounce-to-previous-value' (bound to 'P')
+This command temporarily displays the outside current buffer with the
+outside point corresponding to the previous value, where the previous
+value is what Edebug has evaluated before its last stop point or what
+the user has evaluated in the context outside of Edebug.
+
+This replaces the binding of command 'edebug-view-outside' to 'P', which
+is still available on 'v'.
+
 ---
 ** Flymake
 
diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 284e3acd959..fc349787c93 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -2617,7 +2617,11 @@ when edebug becomes active."
 
 (defvar edebug-eval-list nil) ;; List of expressions to evaluate.
 
-(defvar edebug-previous-result nil) ;; Last result returned.
+;; Last value seen while single-stepping or evaluating in the outside
+;; environment.
+(defvar edebug-previous-value nil)
+;; Last value seen while single-stepping, converted to a string.
+(defvar edebug-previous-result nil)
 
 (defun edebug--display (value offset-index arg-mode)
   ;; edebug--display-1 is too big, we should split it.  This function
@@ -3113,6 +3117,37 @@ before returning.  The default is one second."
       (sit-for arg)
       (edebug-pop-to-buffer edebug-buffer (car edebug-window-data)))))
 
+(defun edebug-bounce-to-previous-value (arg)
+  "Bounce point to previous value in the outside current buffer.
+The previous value is what Edebug has evaluated before its last stop
+point or what you have evaluated in the context outside of Edebug, for
+example, by calling function `edebug-eval-expression', whatever comes
+later.
+If prefix argument ARG is supplied, sit for that many seconds before
+returning.  The default is one second."
+  (interactive "p")
+  (if (not edebug-active)
+      (error "Edebug is not active"))
+  (if (not (integer-or-marker-p edebug-previous-value))
+      (error "Previous value not a number or marker"))
+  (save-excursion
+    ;; If the buffer's currently displayed, avoid set-window-configuration.
+    (save-window-excursion
+      (let ((point-info ""))
+        (edebug-pop-to-buffer edebug-outside-buffer)
+        (cond
+         ((< edebug-previous-value (point-min))
+          (setq point-info (format " (< Point min: %s)" (point-min))))
+         ((> edebug-previous-value (point-max))
+          (setq point-info (format " (> Point max: %s)" (point-max))))
+         ((invisible-p edebug-previous-value)
+          (setq point-info (format " (invisible)"))))
+        (goto-char edebug-previous-value)
+        (message "Current buffer: %s Point: %s%s"
+                 (current-buffer) edebug-previous-value point-info)
+        (sit-for arg)
+        (edebug-pop-to-buffer edebug-buffer (car edebug-window-data))))))
+
 
 ;; Joe Wells, here is a start at your idea of adding a buffer to the internal
 ;; display list.  Still need to use this list in edebug--display.
@@ -3743,7 +3778,8 @@ Return the result of the last expression."
   (if edebug-unwrap-results
       (setq previous-value
             (edebug-unwrap* previous-value)))
-  (setq edebug-previous-result
+  (setq edebug-previous-value previous-value
+        edebug-previous-result
         (concat "Result: "
                 (edebug-safe-prin1-to-string previous-value)
                 (eval-expression-print-format previous-value))))
@@ -3785,6 +3821,8 @@ this is the prefix key.)"
             (values--store-value value)
             (concat (edebug-safe-prin1-to-string value)
                     (eval-expression-print-format value)))))
+    ;; Provide a defined previous value also in case of an error.
+    (setq edebug-previous-value (if errored nil value))
     (cond
      (errored
       (message "Error: %s" errored))
@@ -3901,9 +3939,9 @@ be installed in `emacs-lisp-mode-map'.")
 
   ;; views
   "w"       #'edebug-where
-  "v"       #'edebug-view-outside        ; maybe obsolete??
+  "v"       #'edebug-view-outside
   "p"       #'edebug-bounce-point
-  "P"       #'edebug-view-outside        ; same as v
+  "P"       #'edebug-bounce-to-previous-value
   "W"       #'edebug-toggle-save-windows
 
   ;; misc
@@ -4517,6 +4555,7 @@ It is removed when you hit any char."
     ("Views"
      ["Where am I?" edebug-where t]
      ["Bounce to Current Point" edebug-bounce-point t]
+     ["Bounce to Previous Value" edebug-bounce-to-previous-value t]
      ["View Outside Windows" edebug-view-outside t]
      ["Previous Result" edebug-previous-result t]
      ["Show Backtrace" edebug-pop-to-backtrace t]
diff --git a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
index 24981bb63cf..4e63732554f 100644
--- a/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
+++ b/test/lisp/emacs-lisp/edebug-resources/edebug-test-code.el
@@ -126,6 +126,16 @@
   !start!(with-current-buffer (get-buffer-create "*edebug-test-code-buffer*")
     !body!(format "current-buffer: %s" (current-buffer))))
 
+(defun edebug-test-code-bounce-point ()
+  !start!(with-current-buffer (get-buffer-create "*edebug-test-code-buffer*")
+    (erase-buffer)
+    (insert "123\n567\n9ab\n")
+    (narrow-to-region 5 9)
+    (goto-char 6)!goto-char!
+    (push-mark 1)!push-mark!
+    (set-mark nil)!clear-mark!
+    (+ 1)!1! (+ 6)!6! (+ 10)!10!))
+
 (defun edebug-test-code-use-destructuring-bind ()
   (let ((two 2) (three 3))
     (cl-destructuring-bind (x . y) (cons two three) (+ x!x! y!y!))))
diff --git a/test/lisp/emacs-lisp/edebug-tests.el b/test/lisp/emacs-lisp/edebug-tests.el
index 7daacea7925..4550f25f798 100644
--- a/test/lisp/emacs-lisp/edebug-tests.el
+++ b/test/lisp/emacs-lisp/edebug-tests.el
@@ -302,6 +302,29 @@ Then clear edebug-tests' saved messages."
                           edebug-tests-messages))
   (setq edebug-tests-messages ""))
 
+(defvar edebug-tests-bounce-outside-buffer nil
+  "Outside buffer observed while bouncing.")
+(defvar edebug-tests-bounce-outside-point nil
+  "Outside point observed while bouncing.")
+(defvar edebug-tests-bounce-outside-mark nil
+  "Outside mark observed while bouncing.")
+
+(defun edebug-tests-bounce-record-outside-environment (&rest _)
+  "Record outside buffer, point, and mark while bouncing."
+  (setq edebug-tests-bounce-outside-buffer (current-buffer)
+        edebug-tests-bounce-outside-point  (point)
+        edebug-tests-bounce-outside-mark   (mark)))
+
+(defun edebug-tests-should-have-bounced-to (buffer-or-name point mark message)
+  "Require that a previous bounce bounced to BUFFER-OR-NAME, POINT, and MARK.
+Ensure that the message generated by that bounce equals MESSAGE."
+  (should (equal edebug-tests-bounce-outside-buffer
+                 (get-buffer buffer-or-name)))
+  (should (equal edebug-tests-bounce-outside-point  point))
+  (should (equal edebug-tests-bounce-outside-mark   mark))
+  (should (string-match-p (concat (regexp-quote message) "$")
+                          edebug-tests-messages)))
+
 (defun edebug-tests-locate-def (def-name)
   "Search for a definition of DEF-NAME from the start of the current buffer.
 Place point at the end of DEF-NAME in the buffer."
@@ -419,9 +442,9 @@ test and possibly others should be updated."
     (verify-keybinding "\C-x\C-e" 'edebug-eval-last-sexp)
     (verify-keybinding "E" 'edebug-visit-eval-list)
     (verify-keybinding "w" 'edebug-where)
-    (verify-keybinding "v" 'edebug-view-outside) ;; maybe obsolete??
+    (verify-keybinding "v" 'edebug-view-outside)
     (verify-keybinding "p" 'edebug-bounce-point)
-    (verify-keybinding "P" 'edebug-view-outside) ;; same as v
+    (verify-keybinding "P" 'edebug-bounce-to-previous-value)
     (verify-keybinding "W" 'edebug-toggle-save-windows)
     (verify-keybinding "?" 'edebug-help)
     (verify-keybinding "d" 'edebug-pop-to-backtrace)
@@ -703,6 +726,95 @@ test and possibly others should be updated."
              edebug-tests-messages))
     "g"   (should (equal edebug-tests-@-result '(0 1))))))
 
+(ert-deftest edebug-tests-bounce-point ()
+  "Edebug can bounce point."
+  (unwind-protect
+      (cl-letf* (((symbol-function 'sit-for)
+                  #'edebug-tests-bounce-record-outside-environment))
+        (edebug-tests-with-normal-env
+         (edebug-tests-setup-@ "bounce-point" nil t)
+         (edebug-tests-run-kbd-macro
+          "@" (edebug-tests-should-be-at
+               "bounce-point" "start")
+          (goto-char (edebug-tests-get-stop-point "bounce-point" "goto-char"))
+          "h" (edebug-tests-should-be-at
+               "bounce-point" "goto-char")
+          "p" (edebug-tests-should-have-bounced-to
+               "*edebug-test-code-buffer*" 6 nil
+               "Current buffer: *edebug-test-code-buffer* Point: 6 Mark: <not set>")
+          "SPC SPC" (edebug-tests-should-be-at
+                     "bounce-point" "push-mark")
+          "p" (edebug-tests-should-have-bounced-to
+               "*edebug-test-code-buffer*" 6 1
+               "Current buffer: *edebug-test-code-buffer* Point: 6 Mark: 1")
+          "g")))
+    (when (get-buffer "*edebug-test-code-buffer*")
+      (kill-buffer "*edebug-test-code-buffer*"))))
+
+(ert-deftest edebug-tests-bounce-to-previous-value ()
+  "Edebug can bounce to previous value."
+  (unwind-protect
+      (cl-letf* (((symbol-function 'sit-for)
+                  #'edebug-tests-bounce-record-outside-environment))
+        (edebug-tests-with-normal-env
+         (edebug-tests-setup-@ "bounce-point" nil t)
+         (edebug-tests-run-kbd-macro
+          "@" (edebug-tests-should-be-at
+               "bounce-point" "start")
+          (goto-char (edebug-tests-get-stop-point "bounce-point" "clear-mark"))
+          "h" (edebug-tests-should-be-at
+               "bounce-point" "clear-mark")
+          ;; Bounce to previous values seen while single-stepping.
+          "SPC SPC" (edebug-tests-should-be-at "bounce-point" "1")
+          "P" (edebug-tests-should-have-bounced-to
+               "*edebug-test-code-buffer*" 5 nil
+               "Current buffer: *edebug-test-code-buffer* Point: 1 (< Point min: 5)")
+          "SPC SPC" (edebug-tests-should-be-at "bounce-point" "6")
+          "P" (edebug-tests-should-have-bounced-to
+               "*edebug-test-code-buffer*" 6 nil
+               "Current buffer: *edebug-test-code-buffer* Point: 6")
+          "SPC SPC" (edebug-tests-should-be-at "bounce-point" "10")
+          "P" (edebug-tests-should-have-bounced-to
+               "*edebug-test-code-buffer*" 9 nil
+               "Current buffer: *edebug-test-code-buffer* Point: 10 (> Point max: 9)")
+          ;; Bounce to previous value obtained through evaluation.
+          "e 7 RET"
+          "P" (edebug-tests-should-have-bounced-to
+               "*edebug-test-code-buffer*" 7 nil
+               "Current buffer: *edebug-test-code-buffer* Point: 7")
+          "g")))
+    (when (get-buffer "*edebug-test-code-buffer*")
+      (kill-buffer "*edebug-test-code-buffer*"))))
+
+(ert-deftest edebug-tests-bounce-to-previous-non-position ()
+  "Edebug does not bounce to previous non-position."
+  (edebug-tests-with-normal-env
+   (edebug-tests-setup-@ "fac" '(1) t)
+   (let* ((debug-on-error nil)
+          (edebug-on-error nil)
+          error-message
+          (command-error-function (lambda (&rest args)
+                                    (setq error-message (cadar args)))))
+     (edebug-tests-run-kbd-macro
+      "@" (edebug-tests-should-be-at "fac" "start")
+      ;; Bounce to previous non-position seen while single-stepping.
+      "SPC SPC SPC"
+      (edebug-tests-should-match-result-in-messages "t")
+      "P" (should (string-match-p "Previous value not a number or marker"
+                                  error-message))
+      ;; The error stopped the keyboard macro.  Start it again.
+      (should-not executing-kbd-macro)
+      (setq executing-kbd-macro t
+            error-message nil)
+      ;; Bounce to previous non-position obtained through evaluation.
+      "e nil RET"
+      "P" (should (string-match-p "Previous value not a number or marker"
+                                  error-message))
+      (should-not executing-kbd-macro)
+      (setq executing-kbd-macro t
+            error-message nil)
+      "g"))))
+
 (ert-deftest edebug-tests-step-into-function ()
   "Edebug can step into a function."
   (edebug-tests-with-normal-env
@@ -838,20 +950,23 @@ test and possibly others should be updated."
 
 (ert-deftest edebug-tests-evaluation-of-current-buffer-bug-19611 ()
   "Edebug can evaluate `current-buffer' in correct context. (Bug#19611)."
-  (edebug-tests-with-normal-env
-   (edebug-tests-setup-@ "current-buffer" nil t)
-   (edebug-tests-run-kbd-macro
-    "@"    (edebug-tests-should-be-at
-            "current-buffer" "start")
-    "SPC SPC SPC" (edebug-tests-should-be-at
-                   "current-buffer" "body")
-    "e (current-buffer) RET"
-    ;; Edebug just prints the result without "Result:"
-    (should (string-match-p
-             (regexp-quote "*edebug-test-code-buffer*")
-             edebug-tests-messages))
-    "g"    (should (equal edebug-tests-@-result
-                          "current-buffer: *edebug-test-code-buffer*")))))
+  (unwind-protect
+      (edebug-tests-with-normal-env
+       (edebug-tests-setup-@ "current-buffer" nil t)
+       (edebug-tests-run-kbd-macro
+        "@"    (edebug-tests-should-be-at
+                "current-buffer" "start")
+        "SPC SPC SPC" (edebug-tests-should-be-at
+                       "current-buffer" "body")
+        "e (current-buffer) RET"
+        ;; Edebug just prints the result without "Result:"
+        (should (string-match-p
+                 (regexp-quote "*edebug-test-code-buffer*")
+                 edebug-tests-messages))
+        "g"    (should (equal edebug-tests-@-result
+                              "current-buffer: *edebug-test-code-buffer*"))))
+    (when (get-buffer "*edebug-test-code-buffer*")
+      (kill-buffer "*edebug-test-code-buffer*"))))
 
 (ert-deftest edebug-tests-trivial-backquote ()
   "Edebug can instrument a trivial backquote expression (Bug#23651)."