unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Making edebug work with Follow Mode
@ 2015-03-15 13:55 Alan Mackenzie
  2015-03-15 14:20 ` vibhavp
  2015-03-15 19:12 ` Stefan Monnier
  0 siblings, 2 replies; 4+ messages in thread
From: Alan Mackenzie @ 2015-03-15 13:55 UTC (permalink / raw)
  To: emacs-devel

Hi, Emacs.

Right now, edebug doesn't work with Follow Mode.  In particular, when
stepping through a file.el in two or three side by side Follow Mode
windows, edebug restricts its activities to just one window.  This is
undesirable.

There are two reasons for this failure.  The main one is that edebug
binds post-command-hook to nil, switching off Follow Mode's mechanism.
A subsidiary reason is the calling, from edebug--display-1 of
edebug-adjust-window; this usurps redisplay's scrolling functionality,
and is also the cause of the (to me) irritating recentring of the buffer
when point goes off the bottom of the window, regardless of the user's
setting of scroll-conservatively.

As the main fix, I have introduced two new customisable options,
edebug-OK-functions-for-post-command-hook, and (purely for symmetry)
edebug-OK-functions-for-pre-command-hook.  Any functions listed in these
variables are left on the pertinent hook by edebug.  Currently only
follow-post-command-hook is there.

As the subsidiary fix, I've commented out edebug-adjust-windows and the
call to it.  It seems redundant; redisplay is quite capable of setting
an appropriate window-start.

Here's the patch.  Comments?



diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
index 1091877..dec46a3 100644
--- a/lisp/emacs-lisp/edebug.el
+++ b/lisp/emacs-lisp/edebug.el
@@ -232,6 +232,25 @@ If the result is non-nil, then break.  Errors are ignored."
   :type 'number
   :group 'edebug)
 
+(defcustom edebug-OK-functions-for-pre-command-hook nil
+  "A list of functions edebug will leave on `pre-command-hook'
+whilst edebug is active.  All other elements of
+`pre-command-hook' are removed from the hook.
+
+Do not include in this list any functions you will be edebugging."
+  :type '(repeat function)
+  :group 'edebug)
+
+(defcustom edebug-OK-functions-for-post-command-hook
+  '(follow-post-command-hook)
+  "A list of functions edebug will leave on `post-command-hook'
+whilst edebug is active.  All other elements of
+`post-command-hook' are removed from the hook.
+
+Do not include in this list any functions you will be edebugging."
+  :type '(repeat function)
+  :group 'edebug)
+
 ;;; Form spec utilities.
 
 (defun get-edebug-spec (symbol)
@@ -2446,8 +2465,11 @@ MSG is printed after `::::} '."
                               edebug-function)
                 ))
 
-	  (setcdr edebug-window-data
-		  (edebug-adjust-window (cdr edebug-window-data)))
+	  ;; The following usurps redisplay's functionality, and
+	  ;; doesn't seem to be needed; redisplay will perform any
+	  ;; needed scrolling at the next `recursive-edit'.  2015-03.
+	  ;; (setcdr edebug-window-data
+	  ;; 	  (edebug-adjust-window (cdr edebug-window-data)))
 
 	  ;; Test if there is input, not including keyboard macros.
 	  (if (input-pending-p)
@@ -2677,11 +2699,15 @@ MSG is printed after `::::} '."
 	      (defining-kbd-macro
 		(if edebug-continue-kbd-macro defining-kbd-macro))
 
-	      ;; Disable command hooks.  This is essential when
-	      ;; a hook function is instrumented - to avoid infinite loop.
-	      ;; This may be more than we need, however.
-	      (pre-command-hook nil)
-	      (post-command-hook nil)
+	      ;; Remove from the command hooks all but allowed functions.
+	      ;; An instrumented function may not be on either of these
+	      ;; hooks.
+	      (pre-command-hook
+	       (cl-intersection pre-command-hook
+				edebug-OK-functions-for-pre-command-hook))
+	      (post-command-hook
+	       (cl-intersection post-command-hook
+				edebug-OK-functions-for-post-command-hook))
 
 	      ;; others??
 	      )
@@ -2722,28 +2748,28 @@ MSG is printed after `::::} '."
 
 ;;; Display related functions
 
-(defun edebug-adjust-window (old-start)
-  ;; If pos is not visible, adjust current window to fit following context.
-  ;; (message "window: %s old-start: %s window-start: %s pos: %s"
-  ;;          (selected-window) old-start (window-start) (point)) (sit-for 5)
-  (if (not (pos-visible-in-window-p))
-      (progn
-	;; First try old-start
-	(if old-start
-	    (set-window-start (selected-window) old-start))
-	(if (not (pos-visible-in-window-p))
-	    (progn
-	;; (message "resetting window start") (sit-for 2)
-	(set-window-start
-	 (selected-window)
-	 (save-excursion
-	   (forward-line
-	    (if (< (point) (window-start)) -1	; one line before if in back
-	      (- (/ (window-height) 2)) ; center the line moving forward
-	      ))
-	   (beginning-of-line)
-	   (point)))))))
-  (window-start))
+;; (defun edebug-adjust-window (old-start)
+;;   ;; If pos is not visible, adjust current window to fit following context.
+;;   ;; (message "window: %s old-start: %s window-start: %s pos: %s"
+;;   ;;          (selected-window) old-start (window-start) (point)) (sit-for 5)
+;;   (if (not (pos-visible-in-window-p))
+;;       (progn
+;; 	;; First try old-start
+;; 	(if old-start
+;; 	    (set-window-start (selected-window) old-start))
+;; 	(if (not (pos-visible-in-window-p))
+;; 	    (progn
+;; 	;; (message "resetting window start") (sit-for 2)
+;; 	(set-window-start
+;; 	 (selected-window)
+;; 	 (save-excursion
+;; 	   (forward-line
+;; 	    (if (< (point) (window-start)) -1	; one line before if in back
+;; 	      (- (/ (window-height) 2)) ; center the line moving forward
+;; 	      ))
+;; 	   (beginning-of-line)
+;; 	   (point)))))))
+;;   (window-start))
 
 
 
-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: Making edebug work with Follow Mode
  2015-03-15 13:55 Making edebug work with Follow Mode Alan Mackenzie
@ 2015-03-15 14:20 ` vibhavp
  2015-03-15 19:12 ` Stefan Monnier
  1 sibling, 0 replies; 4+ messages in thread
From: vibhavp @ 2015-03-15 14:20 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> +(defcustom edebug-OK-functions-for-pre-command-hook
> +(defcustom edebug-OK-functions-for-post-command-hook

IMO, using a shorter name like edebug-post-command-exclude-functions and
edebug-pre-command-exclude-functions would be better.

-- 
Vibhav Pant
vibhavp@gmail.com



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Making edebug work with Follow Mode
  2015-03-15 13:55 Making edebug work with Follow Mode Alan Mackenzie
  2015-03-15 14:20 ` vibhavp
@ 2015-03-15 19:12 ` Stefan Monnier
  2015-03-16 17:20   ` Alan Mackenzie
  1 sibling, 1 reply; 4+ messages in thread
From: Stefan Monnier @ 2015-03-15 19:12 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

> There are two reasons for this failure.  The main one is that edebug
> binds post-command-hook to nil, switching off Follow Mode's mechanism.

FWIW, in my local edebug.el I removed these let-bindings of
pre/post-command-hook a long time ago and never looked back.

So I'm in favor of simply removing them altogether.

> As the subsidiary fix, I've commented out edebug-adjust-windows and the
> call to it.

Sounds OK.  If/when someone complains about a particular side-effect, we
can see how to get it back in a less heavy-handed way than
edebug-adjust-windows.


        Stefan



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Making edebug work with Follow Mode
  2015-03-15 19:12 ` Stefan Monnier
@ 2015-03-16 17:20   ` Alan Mackenzie
  0 siblings, 0 replies; 4+ messages in thread
From: Alan Mackenzie @ 2015-03-16 17:20 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Hello, Stefan.

On Sun, Mar 15, 2015 at 03:12:16PM -0400, Stefan Monnier wrote:
> > There are two reasons for this failure.  The main one is that edebug
> > binds post-command-hook to nil, switching off Follow Mode's mechanism.

> FWIW, in my local edebug.el I removed these let-bindings of
> pre/post-command-hook a long time ago and never looked back.

> So I'm in favor of simply removing them altogether.

I've now removed them altogether.

> > As the subsidiary fix, I've commented out edebug-adjust-windows and the
> > call to it.

> Sounds OK.  If/when someone complains about a particular side-effect, we
> can see how to get it back in a less heavy-handed way than
> edebug-adjust-windows.

I've also removed these completely.

It's not often that a new feature can be added simply by deleting code.
;-)

Thanks for the help.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-03-16 17:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-15 13:55 Making edebug work with Follow Mode Alan Mackenzie
2015-03-15 14:20 ` vibhavp
2015-03-15 19:12 ` Stefan Monnier
2015-03-16 17:20   ` Alan Mackenzie

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).