Re: edebug breaks pre,post-command-hook

[Richard Stallman]

    I think edebug should not try to reproduce the behavior of `let'
    which was already hard to implement once.

    How about just doing

    (let ((pre-command-hook pre-command-hook)
          (post-command-hook post-command-hook))
      ...)

The aim here is to turn off the outside values of pre-command-hook and
post-command-hook.  The program being debugged could have changed them
to something broken.  However, we want to put back the program's own
values for commands that evaluate something in the program's context.
And if the expression you evaluate  changes these variables, we want
those changes to remain in effect when the program resumes.

That is what the current code is designed to do.
But it doesn't restore the bindings properly in the unwind-protect.
I've written code to allow a Lisp program to do exactly what internal
unbinding of a let binding would do.

Yamaoka san, please try these patches.  They seemed to work for me.
Do they work fully for you?


*** data.c.~1.225.~     Tue Aug 26 20:46:43 2003
--- data.c      Sun Sep  7 10:52:33 2003
***************
*** 1706,1712 ****
      {
        Lisp_Object tail, elt;
  
-       variable = indirect_variable (variable);
        for (tail = buf->local_var_alist; CONSP (tail); tail = XCDR (tail))
        {
          elt = XCAR (tail);
--- 1706,1711 ----
***************
*** 1765,1770 ****
--- 1764,1804 ----
      }
    return Qnil;
  }
+ 
+ DEFUN ("variable-binding-locus", Fvariable_binding_locus, 
Svariable_binding_locus,
+        1, 1, 0,
+        doc: /* Return a value indicating where VARIABLE's current binding 
comes from.
+ If the current binding is buffer-local, the value is the current buffer.
+ If the current binding is frame-local, the value is the selected frame.
+ If the current binding is global (the default), the value is nil.  */)
+      (variable)
+      register Lisp_Object variable;
+ {
+   Lisp_Object valcontents;
+ 
+   CHECK_SYMBOL (variable);
+   variable = indirect_variable (variable);
+ 
+   /* Make sure the current binding is actually swapped in.  */
+   find_symbol_value (variable);
+ 
+   valcontents = XSYMBOL (variable)->value;
+ 
+   if (BUFFER_LOCAL_VALUEP (valcontents)
+       || SOME_BUFFER_LOCAL_VALUEP (valcontents)
+       || BUFFER_OBJFWDP (valcontents))
+     {
+       /* For a local variable, record both the symbol and which
+        buffer's or frame's value we are saving.  */
+       if (!NILP (Flocal_variable_p (variable, Qnil)))
+       return Fcurrent_buffer ();
+       else if (!BUFFER_OBJFWDP (valcontents)
+              && XBUFFER_LOCAL_VALUE (valcontents)->found_for_frame)
+       return XBUFFER_LOCAL_VALUE (valcontents)->frame;
+     }
+ 
+   return Qnil;
+ }
  
  /* Find the function at the end of a chain of symbol function indirections.  
*/
  
***************
*** 3185,3190 ****
--- 3219,3225 ----
    defsubr (&Smake_variable_frame_local);
    defsubr (&Slocal_variable_p);
    defsubr (&Slocal_variable_if_set_p);
+   defsubr (&Svariable_binding_locus);
    defsubr (&Saref);
    defsubr (&Saset);
    defsubr (&Snumber_to_string);


*** edebug.el.~3.62.~   Tue Jun  3 07:01:24 2003
--- edebug.el   Sun Sep  7 10:50:17 2003
***************
*** 2241,2248 ****
  
            ;; Save the outside value of executing macro.  (here??)
            (edebug-outside-executing-macro executing-kbd-macro)
!           (edebug-outside-pre-command-hook pre-command-hook)
!           (edebug-outside-post-command-hook post-command-hook))
        (unwind-protect
            (let (;; Don't keep reading from an executing kbd macro
                  ;; within edebug unless edebug-continue-kbd-macro is
--- 2241,2250 ----
  
            ;; Save the outside value of executing macro.  (here??)
            (edebug-outside-executing-macro executing-kbd-macro)
!           (edebug-outside-pre-command-hook
!            (edebug-var-status 'pre-command-hook))
!           (edebug-outside-post-command-hook
!            (edebug-var-status 'post-command-hook)))
        (unwind-protect
            (let (;; Don't keep reading from an executing kbd macro
                  ;; within edebug unless edebug-continue-kbd-macro is
***************
*** 2267,2276 ****
                    edebug-next-execution-mode nil)
              (edebug-enter edebug-function edebug-args edebug-body))
          ;; Reset global variables in case outside value was changed.
!         (setq executing-kbd-macro edebug-outside-executing-macro
!               pre-command-hook edebug-outside-pre-command-hook
!               post-command-hook edebug-outside-post-command-hook
!               )))
  
      (let* ((edebug-data (get edebug-function 'edebug))
           (edebug-def-mark (car edebug-data)) ; mark at def start
--- 2269,2279 ----
                    edebug-next-execution-mode nil)
              (edebug-enter edebug-function edebug-args edebug-body))
          ;; Reset global variables in case outside value was changed.
!         (setq executing-kbd-macro edebug-outside-executing-macro)
!         (edebug-restore-status
!          'post-command-hook edebug-outside-post-command-hook)
!         (edebug-restore-status
!          'pre-command-hook edebug-outside-pre-command-hook)))
  
      (let* ((edebug-data (get edebug-function 'edebug))
           (edebug-def-mark (car edebug-data)) ; mark at def start
***************
*** 2291,2296 ****
--- 2294,2323 ----
        (funcall edebug-body))
        )))
  
+ (defun edebug-var-status (var)
+   "Return a cons cell describing the status of VAR's current binding.
+ The purpose of this function is so you can properly undo
+ subsequent changes to the same binding, by passing the status
+ cons cell to `edebug-restore-status'.  The status cons cell
+ has the form (LOCUS . VALUE), where LOCUS can be a buffer
+ \(for a buffer-local binding), a frame (for a frame-local binding),
+ or nil (if the default binding is current)."
+   (cons (variable-binding-locus var)
+       (symbol-value var)))
+ 
+ (defun edebug-restore-status (var status)
+   "Reset VAR based on STATUS.
+ STATUS should be a list you got from `edebug-var-status'."
+   (let ((locus (car status))
+       (value (cdr status)))
+     (cond ((bufferp locus)
+          (if (buffer-live-p locus)
+              (with-current-buffer locus
+                (set var value))))
+         ((framep locus)
+          (modify-frame-parameters locus (list (cons var value))))
+         (t
+          (set var value)))))
  
  (defun edebug-enter-trace (edebug-body)
    (let ((edebug-stack-depth (1+ edebug-stack-depth))
***************
*** 3511,3518 ****
  
           (executing-kbd-macro edebug-outside-executing-macro)
           (defining-kbd-macro edebug-outside-defining-kbd-macro)
!          (pre-command-hook edebug-outside-pre-command-hook)
!          (post-command-hook edebug-outside-post-command-hook)
  
           ;; See edebug-display
           (overlay-arrow-position edebug-outside-o-a-p)
--- 3538,3546 ----
  
           (executing-kbd-macro edebug-outside-executing-macro)
           (defining-kbd-macro edebug-outside-defining-kbd-macro)
!          ;; Get the values out of the saved statuses.
!          (pre-command-hook (cdr edebug-outside-pre-command-hook))
!          (post-command-hook (cdr edebug-outside-post-command-hook))
  
           ;; See edebug-display
           (overlay-arrow-position edebug-outside-o-a-p)
***************
*** 3552,3564 ****
  
          edebug-outside-executing-macro executing-kbd-macro
          edebug-outside-defining-kbd-macro defining-kbd-macro
-         edebug-outside-pre-command-hook pre-command-hook
-         edebug-outside-post-command-hook post-command-hook
  
          edebug-outside-o-a-p overlay-arrow-position
          edebug-outside-o-a-s overlay-arrow-string
          edebug-outside-c-i-e-a cursor-in-echo-area
!         )))                           ; let
       ))
  
  (defvar cl-debug-env nil) ;; defined in cl; non-nil when lexical env used.
--- 3580,3597 ----
  
          edebug-outside-executing-macro executing-kbd-macro
          edebug-outside-defining-kbd-macro defining-kbd-macro
  
          edebug-outside-o-a-p overlay-arrow-position
          edebug-outside-o-a-s overlay-arrow-string
          edebug-outside-c-i-e-a cursor-in-echo-area
!         )
! 
!        ;; Restore the outside saved values; don't alter
!        ;; the outside binding loci.
!        (setcdr edebug-outside-pre-command-hook pre-command-hook)
!        (setcdr edebug-outside-post-command-hook post-command-hook)
! 
!        ))                             ; let
       ))
  
  (defvar cl-debug-env nil) ;; defined in cl; non-nil when lexical env used.
***************
*** 3676,3689 ****
      (edebug-safe-prin1-to-string (car values)))))
  
  (defun edebug-eval-last-sexp ()
!   "Evaluate sexp before point in the outside environment;
! print value in minibuffer."
    (interactive)
    (edebug-eval-expression (edebug-last-sexp)))
  
  (defun edebug-eval-print-last-sexp ()
!   "Evaluate sexp before point in the outside environment;
! print value into current buffer."
    (interactive)
    (let* ((edebug-form (edebug-last-sexp))
         (edebug-result-string
--- 3709,3721 ----
      (edebug-safe-prin1-to-string (car values)))))
  
  (defun edebug-eval-last-sexp ()
!   "Evaluate sexp before point in the outside environment; value in 
minibuffer."
    (interactive)
    (edebug-eval-expression (edebug-last-sexp)))
  
  (defun edebug-eval-print-last-sexp ()
!   "Evaluate sexp before point in the outside environment; insert the value.
! This prints the value into current buffer."
    (interactive)
    (let* ((edebug-form (edebug-last-sexp))
         (edebug-result-string
***************
*** 3698,3709 ****
  
  ;;; Edebug Minor Mode
  
! ;; Global GUD bindings for all emacs-lisp-mode buffers.
! (define-key emacs-lisp-mode-map "\C-x\C-a\C-s" 'edebug-step-mode)
! (define-key emacs-lisp-mode-map "\C-x\C-a\C-n" 'edebug-next-mode)
! (define-key emacs-lisp-mode-map "\C-x\C-a\C-c" 'edebug-go-mode)
! (define-key emacs-lisp-mode-map "\C-x\C-a\C-l" 'edebug-where)
  
  
  (defvar edebug-mode-map
    (let ((map (copy-keymap emacs-lisp-mode-map)))
--- 3730,3744 ----
  
  ;;; Edebug Minor Mode
  
! (defvar gud-inhibit-global-bindings
!   "*Non-nil means don't do global rebindings of C-x C-a subcommands.")
  
+ ;; Global GUD bindings for all emacs-lisp-mode buffers.
+ (unless gud-inhibit-global-bindings
+   (define-key emacs-lisp-mode-map "\C-x\C-a\C-s" 'edebug-step-mode)
+   (define-key emacs-lisp-mode-map "\C-x\C-a\C-n" 'edebug-next-mode)
+   (define-key emacs-lisp-mode-map "\C-x\C-a\C-c" 'edebug-go-mode)
+   (define-key emacs-lisp-mode-map "\C-x\C-a\C-l" 'edebug-where))
  
  (defvar edebug-mode-map
    (let ((map (copy-keymap emacs-lisp-mode-map)))