* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
@ 2012-02-13 22:29 Michael Heerdegen
2012-02-14 10:53 ` martin rudalics
2012-10-04 13:17 ` martin rudalics
0 siblings, 2 replies; 9+ messages in thread
From: Michael Heerdegen @ 2012-02-13 22:29 UTC (permalink / raw)
To: 10805
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''
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
2012-02-13 22:29 bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer Michael Heerdegen
@ 2012-02-14 10:53 ` martin rudalics
2012-02-14 23:53 ` Michael Heerdegen
2012-10-04 13:17 ` martin rudalics
1 sibling, 1 reply; 9+ messages in thread
From: martin rudalics @ 2012-02-14 10:53 UTC (permalink / raw)
To: michael_heerdegen; +Cc: 10805
> 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)
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
2012-02-14 10:53 ` martin rudalics
@ 2012-02-14 23:53 ` Michael Heerdegen
2012-02-15 9:56 ` martin rudalics
0 siblings, 1 reply; 9+ messages in thread
From: Michael Heerdegen @ 2012-02-14 23:53 UTC (permalink / raw)
To: martin rudalics; +Cc: 10805
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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
2012-02-14 23:53 ` Michael Heerdegen
@ 2012-02-15 9:56 ` martin rudalics
2012-02-17 2:18 ` Michael Heerdegen
0 siblings, 1 reply; 9+ messages in thread
From: martin rudalics @ 2012-02-15 9:56 UTC (permalink / raw)
To: Michael Heerdegen; +Cc: 10805
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
2012-02-15 9:56 ` martin rudalics
@ 2012-02-17 2:18 ` Michael Heerdegen
2012-02-17 9:58 ` martin rudalics
0 siblings, 1 reply; 9+ messages in thread
From: Michael Heerdegen @ 2012-02-17 2:18 UTC (permalink / raw)
To: martin rudalics; +Cc: 10805
martin rudalics <rudalics@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
2012-02-17 2:18 ` Michael Heerdegen
@ 2012-02-17 9:58 ` martin rudalics
2012-02-17 15:12 ` Stefan Monnier
0 siblings, 1 reply; 9+ messages in thread
From: martin rudalics @ 2012-02-17 9:58 UTC (permalink / raw)
To: michael_heerdegen; +Cc: 10805
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
2012-02-17 9:58 ` martin rudalics
@ 2012-02-17 15:12 ` Stefan Monnier
2012-02-18 17:04 ` martin rudalics
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2012-02-17 15:12 UTC (permalink / raw)
To: martin rudalics; +Cc: michael_heerdegen, 10805
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
2012-02-17 15:12 ` Stefan Monnier
@ 2012-02-18 17:04 ` martin rudalics
0 siblings, 0 replies; 9+ messages in thread
From: martin rudalics @ 2012-02-18 17:04 UTC (permalink / raw)
To: Stefan Monnier; +Cc: michael_heerdegen, 10805
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer
2012-02-13 22:29 bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer Michael Heerdegen
2012-02-14 10:53 ` martin rudalics
@ 2012-10-04 13:17 ` martin rudalics
1 sibling, 0 replies; 9+ messages in thread
From: martin rudalics @ 2012-10-04 13:17 UTC (permalink / raw)
To: 10805-done; +Cc: michael_heerdegen
> 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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2012-10-04 13:17 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-13 22:29 bug#10805: 24.0.93; edebug-trace t may cause stuff being inserted into current buffer Michael Heerdegen
2012-02-14 10:53 ` martin rudalics
2012-02-14 23:53 ` Michael Heerdegen
2012-02-15 9:56 ` martin rudalics
2012-02-17 2:18 ` Michael Heerdegen
2012-02-17 9:58 ` martin rudalics
2012-02-17 15:12 ` Stefan Monnier
2012-02-18 17:04 ` martin rudalics
2012-10-04 13:17 ` martin rudalics
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).