GNU bug report logs - #10805
24.0.93; edebug-trace t may cause stuff being inserted into current buffer

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 10805 in the body.
You can then email your comments to 10805 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.0.93;
	edebug-trace t may cause stuff being inserted into current buffer
Date: Mon, 13 Feb 2012 23:29:39 +0100

E.g.

1. emacs -Q
2. I write this into the scratch buffer:
     (defun f (x) (if (> x 0) (* x (f (1- x))) 0))
   and do M-1 C-M-x
3. M-: (setq edebug-trace t) RET
4. M-: (f 5) RET

Then, "{ f args: (5)" is inserted into *scratch*.

The bug is related to this commented code at the end of
`edebug-pop-to-buffer':

  ;; Selecting the window does not set the buffer until command loop.
  ;;(set-buffer buffer)

If I uncomment this call to `set-buffer', the problem disappears.

Seems the one who commented this line of code wanted to test if it is
(still) needed - seems it is.


Thanks,

Michael.


In GNU Emacs 24.0.93.1 (i486-pc-linux-gnu, GTK+ Version 3.2.3)
 of 2012-02-08 on zelenka, modified by Debian
 (emacs-snapshot package, version 2:20120208-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11103901
Configured using:
 `configure '--build' 'i486-linux-gnu' '--host' 'i486-linux-gnu'
 '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib'
 '--localstatedir=/var' '--infodir=/usr/share/info'
 '--mandir=/usr/share/man' '--with-pop=yes'
 '--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.0.93/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.0.93/site-lisp:/usr/share/emacs/site-lisp'
 '--without-compress-info' '--with-crt-dir=/usr/lib/i386-linux-gnu/'
 '--with-x=yes' '--with-x-toolkit=gtk3' '--with-imagemagick=yes'
 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu'
 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2''

Message #8 received at 10805 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: michael_heerdegen <at> web.de
Cc: 10805 <at> debbugs.gnu.org
Subject: Re: bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted
	into current buffer
Date: Tue, 14 Feb 2012 11:53:48 +0100

> 1. emacs -Q
> 2. I write this into the scratch buffer:
>      (defun f (x) (if (> x 0) (* x (f (1- x))) 0))
>    and do M-1 C-M-x
> 3. M-: (setq edebug-trace t) RET
> 4. M-: (f 5) RET
>
> Then, "{ f args: (5)" is inserted into *scratch*.
>
> The bug is related to this commented code at the end of
> `edebug-pop-to-buffer':
>
>   ;; Selecting the window does not set the buffer until command loop.
>   ;;(set-buffer buffer)
>
> If I uncomment this call to `set-buffer', the problem disappears.
>
> Seems the one who commented this line of code wanted to test if it is
> (still) needed - seems it is.

More or less so, yes.  This is yet another incarnation of the "unless
WINDOW already is the selected window, its buffer becomes the current
buffer" problem of `select-window'.

We have three possibilites to fix this:

(1) Fix this in `select-window' and friends.  This should be done sooner
    or later but I'm not sure whether "now" is the right moment.

(2) Fix this by uncommenting the line above.  I suppose, however, that
    line was commented out on purpose, hence we might experience more
    surprises in edebug.

(3) Do something like the patch below.  This is the most conservative
    approach and will consequently fail to fix similar problems.  Note
    also that my experience with `edebug-trace' is zero.

martin


*** lisp/emacs-lisp/edebug.el	2012-01-19 07:21:25 +0000
--- lisp/emacs-lisp/edebug.el	2012-02-14 10:43:52 +0000
***************
*** 4129,4146 ****
  ;;	  (point) (window-start))
    (let* ((oldbuf (current-buffer))
  	 (selected-window (selected-window))
! 	 (buffer (get-buffer-create buf-name))
! 	 buf-window)
  ;;    (message "before pop-to-buffer") (sit-for 1)
      (edebug-pop-to-buffer buffer)
!     (setq truncate-lines t)
!     (setq buf-window (selected-window))
!     (goto-char (point-max))
!     (insert (apply 'edebug-format fmt args) "\n")
!     ;; Make it visible.
!     (vertical-motion (- 1 (window-height)))
!     (set-window-start buf-window (point))
!     (goto-char (point-max))
  ;;    (set-window-point buf-window (point))
  ;;    (edebug-sit-for 0)
      (bury-buffer buffer)
--- 4129,4145 ----
  ;;	  (point) (window-start))
    (let* ((oldbuf (current-buffer))
  	 (selected-window (selected-window))
! 	 (buffer (get-buffer-create buf-name)))
  ;;    (message "before pop-to-buffer") (sit-for 1)
      (edebug-pop-to-buffer buffer)
!     (with-current-buffer buffer
!       (setq truncate-lines t)
!       (goto-char (point-max))
!       (insert (apply 'edebug-format fmt args) "\n")
!       ;; Make it visible.
!       (vertical-motion (- 1 (window-height)))
!       (set-window-start nil (point))
!       (goto-char (point-max)))
  ;;    (set-window-point buf-window (point))
  ;;    (edebug-sit-for 0)
      (bury-buffer buffer)

Message #11 received at 10805 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 10805 <at> debbugs.gnu.org
Subject: Re: bug#10805: 24.0.93;
	edebug-trace t may cause stuff being inserted into current buffer
Date: Wed, 15 Feb 2012 00:53:44 +0100

Note that my experience with `edebug-trace' is also zero, I only tried
it out, so take my comments with some care.

> More or less so, yes.  This is yet another incarnation of the "unless
> WINDOW already is the selected window, its buffer becomes the current
> buffer" problem of `select-window'.

Maybe.  But please keep in mind that `edebug-trace-display' is about
_displaying_ a buffer.  We don't want the user to edit the trace buffer,
so calling `select-window' is in fact not necessary in this case.  The
former window is instantly re-selected by `edebug-trace-display'.  

Please also note (another issue, but related) that if the trace buffer
is already visible in another (visible) frame, a new window pops up in
the current frame nevertheless, which is kind of annoying if source
files are spread over multiple frames.


Thanks,

Michael.

Message #14 received at 10805 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 10805 <at> debbugs.gnu.org
Subject: Re: bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted
	into current buffer
Date: Wed, 15 Feb 2012 10:56:29 +0100

> Maybe.  But please keep in mind that `edebug-trace-display' is about
> _displaying_ a buffer.  We don't want the user to edit the trace buffer,
> so calling `select-window' is in fact not necessary in this case.  The
> former window is instantly re-selected by `edebug-trace-display'.

IIUC edebug conflates the concepts of displaying and popping to buffers.
Take `edebug-bounce-point':

  (save-excursion
    ;; If the buffer's currently displayed, avoid set-window-configuration.
    ---> Since `save-window-excursion' calls `set-window-configuration'
         we don't "avoid" anything here.
    (save-window-excursion
      (edebug-pop-to-buffer edebug-outside-buffer)
      ---> Here the `select-window' problem might strike as well.  But
           why should the window be selected?  To make `goto-char',
           `current-buffer' and `point' work?
      (goto-char edebug-outside-point)
      (message "Current buffer: %s Point: %s Mark: %s"
	       (current-buffer) (point)
	       (if (marker-buffer (edebug-mark-marker))
		   (marker-position (edebug-mark-marker)) "<not set>"))
      (edebug-sit-for arg)
      ---> Popping back to the original window as the _last_ action of
           a `save-window-excursion' doesn't make any sense.
      (edebug-pop-to-buffer edebug-buffer (car edebug-window-data)))))

This may select a window up to three times for the apparent single
purpose to display the context of a position in some buffer.

> Please also note (another issue, but related) that if the trace buffer
> is already visible in another (visible) frame, a new window pops up in
> the current frame nevertheless, which is kind of annoying if source
> files are spread over multiple frames.

Probably because `edebug-get-buffer-window' (aliased to
`get-buffer-window') is called with a nil ALL-FRAMES argument.  Try to
use another argument (you probably have to raise frames to make this
useful).

martin

Message #17 received at 10805 <at> debbugs.gnu.org (full text, mbox):

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: martin rudalics <rudalics <at> gmx.at>
Cc: 10805 <at> debbugs.gnu.org
Subject: Re: bug#10805: 24.0.93;
	edebug-trace t may cause stuff being inserted into current buffer
Date: Fri, 17 Feb 2012 03:18:24 +0100

martin rudalics <rudalics <at> gmx.at> writes:

> IIUC edebug conflates the concepts of displaying and popping to buffers.
> Take `edebug-bounce-point':
>
>   (save-excursion
>     ;; If the buffer's currently displayed, avoid set-window-configuration.
>     ---> Since `save-window-excursion' calls `set-window-configuration'
>          we don't "avoid" anything here.
>     (save-window-excursion
>       (edebug-pop-to-buffer edebug-outside-buffer)
>       ---> Here the `select-window' problem might strike as well.  But
>            why should the window be selected?  To make `goto-char',
>            `current-buffer' and `point' work?
>       (goto-char edebug-outside-point)
>       (message "Current buffer: %s Point: %s Mark: %s"
> 	       (current-buffer) (point)
> 	       (if (marker-buffer (edebug-mark-marker))
> 		   (marker-position (edebug-mark-marker)) "<not set>"))
>       (edebug-sit-for arg)
>       ---> Popping back to the original window as the _last_ action of
>            a `save-window-excursion' doesn't make any sense.
>       (edebug-pop-to-buffer edebug-buffer (car edebug-window-data)))))
>
> This may select a window up to three times for the apparent single
> purpose to display the context of a position in some buffer.

I agree, I see no reason for doing this.

> > Please also note (another issue, but related) that if the trace buffer
> > is already visible in another (visible) frame, a new window pops up in
> > the current frame nevertheless, which is kind of annoying if source
> > files are spread over multiple frames.
>
> Probably because `edebug-get-buffer-window' (aliased to
> `get-buffer-window') is called with a nil ALL-FRAMES argument.  Try to
> use another argument (you probably have to raise frames to make this
> useful).

Yes, of course, that works.  The same applies to the source code
buffers.  Maybe we could introduce a user option specifying if windows
in other (visible) frames should be considered or not.  Dunno if that
would be appropriate, I didn't use edebug much before.  Currently,
edebug never considers other frames (most of the code seems to have been
written at a time when Emacs did not yet have frames).  Some users might
find it handy if they could use source buffers spread over several
frames, and edebug would work with them.


Michael

Message #20 received at 10805 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: michael_heerdegen <at> web.de
Cc: 10805 <at> debbugs.gnu.org
Subject: Re: bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted
	into current buffer
Date: Fri, 17 Feb 2012 10:58:45 +0100

> Yes, of course, that works.  The same applies to the source code
> buffers.  Maybe we could introduce a user option specifying if windows
> in other (visible) frames should be considered or not.  Dunno if that
> would be appropriate, I didn't use edebug much before.  Currently,
> edebug never considers other frames (most of the code seems to have been
> written at a time when Emacs did not yet have frames).  Some users might
> find it handy if they could use source buffers spread over several
> frames, and edebug would work with them.

Adding an appropriate rule for these buffers to `display-buffer-alist'
should be the appropriate solution.  The problem is that it's
non-trivial to identify source code buffers in `display-buffer-alist'.
For example, in the case that the user wants to make buffer display via
edebug behave differently from via `find-file'.

martin

Message #23 received at 10805 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: martin rudalics <rudalics <at> gmx.at>
Cc: michael_heerdegen <at> web.de, 10805 <at> debbugs.gnu.org
Subject: Re: bug#10805: 24.0.93;
	edebug-trace t may cause stuff being inserted into current buffer
Date: Fri, 17 Feb 2012 10:12:59 -0500

> For example, in the case that the user wants to make buffer display via
> edebug behave differently from via `find-file'.

You can check whether the buffer is in `edebug-mode'.  This may require
some tweaks in edebug.el (e.g. I'm not sure whether edebug-mode is set
before or after displaying the buffer currently, and IIRC edebug-mode is
neither a proper major-mode nor a proper minor-mode so you'll probably
have to detect it in an ad-hoc way, or change edebug.el to use a proper
major/minor mode).


        Stefan

Message #26 received at 10805 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: michael_heerdegen <at> web.de, 10805 <at> debbugs.gnu.org
Subject: Re: bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted
	into current buffer
Date: Sat, 18 Feb 2012 18:04:36 +0100

> You can check whether the buffer is in `edebug-mode'.  This may require
> some tweaks in edebug.el (e.g. I'm not sure whether edebug-mode is set
> before or after displaying the buffer currently, and IIRC edebug-mode is
> neither a proper major-mode nor a proper minor-mode so you'll probably
> have to detect it in an ad-hoc way, or change edebug.el to use a proper
> major/minor mode).

IIUC `edebug-active' is the variable to consult in this case.  But the
plethora of declared and undocumented variables and aliased functions in
edebug.el is overwhelming.

martin

Message #31 received at 10805-done <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: 10805-done <at> debbugs.gnu.org
Cc: michael_heerdegen <at> web.de
Subject: Re: bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted
	into current buffer
Date: Thu, 04 Oct 2012 15:17:54 +0200

> 1. emacs -Q
> 2. I write this into the scratch buffer:
>      (defun f (x) (if (> x 0) (* x (f (1- x))) 0))
>    and do M-1 C-M-x
> 3. M-: (setq edebug-trace t) RET
> 4. M-: (f 5) RET
> 
> Then, "{ f args: (5)" is inserted into *scratch*.

Should be fixed with revision 110359 on trunk.  Bug closed.

Thanks, martin

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.