* with-current-buffer "cleanup" bug
@ 2009-11-14 15:51 Chong Yidong
2009-11-14 18:07 ` Stefan Monnier
2009-11-15 5:13 ` Stefan Monnier
0 siblings, 2 replies; 6+ messages in thread
From: Chong Yidong @ 2009-11-14 15:51 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Hi Stefan,
I just reverted your change to help-mode.el, because it led to a bug:
`C-h f describe-minor-mode', and any other help command that creates a
help buffer larger than its window size, left point at the end of the
buffer.
If you're wholesale changing `save-excursion + set-buffer' to
`with-current-buffer', this is wrong. The `with-current-buffer' macro
does not save point (or mark), so this can lead to code behavior change.
You need to fix this, or we're likely see a long slow trickle of related
bugs in the coming weeks and months.
*** help-mode.el.~1.68~ 2009-11-14 10:40:18.000000000 -0500
--- help-mode.el.~1.69.~ 2009-11-14 10:30:40.000000000 -0500
***************
*** 413,420 ****
help buffers. Variable `help-back-label' specifies the text for
that."
(interactive "b")
! (save-excursion
! (set-buffer (or buffer (current-buffer)))
(goto-char (point-min))
;; Skip the header-type info, though it might be useful to parse
;; it at some stage (e.g. "function in `library'").
--- 413,419 ----
help buffers. Variable `help-back-label' specifies the text for
that."
(interactive "b")
! (with-current-buffer (or buffer (current-buffer))
(goto-char (point-min))
;; Skip the header-type info, though it might be useful to parse
;; it at some stage (e.g. "function in `library'").
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: with-current-buffer "cleanup" bug
2009-11-14 15:51 with-current-buffer "cleanup" bug Chong Yidong
@ 2009-11-14 18:07 ` Stefan Monnier
2009-11-14 22:27 ` Chong Yidong
2009-11-15 5:13 ` Stefan Monnier
1 sibling, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2009-11-14 18:07 UTC (permalink / raw)
To: Chong Yidong; +Cc: emacs-devel
> If you're wholesale changing `save-excursion + set-buffer' to
> `with-current-buffer', this is wrong.
The change may be wrong, but the original code is wrong too. Which is
why I'm happily applying the change: when it uncovers a bug, the right
fix is almost always to use some other code because the problem can
appear even with the original code.
> The `with-current-buffer' macro
> does not save point (or mark), so this can lead to code behavior change.
> You need to fix this, or we're likely see a long slow trickle of related
> bugs in the coming weeks and months.
Yes, I'm waiting for those bugs indeed,
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: with-current-buffer "cleanup" bug
2009-11-14 18:07 ` Stefan Monnier
@ 2009-11-14 22:27 ` Chong Yidong
2009-11-15 4:48 ` Stefan Monnier
0 siblings, 1 reply; 6+ messages in thread
From: Chong Yidong @ 2009-11-14 22:27 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> The `with-current-buffer' macro does not save point (or mark), so
>> this can lead to code behavior change. You need to fix this, or
>> we're likely see a long slow trickle of related bugs in the coming
>> weeks and months.
>
> Yes, I'm waiting for those bugs indeed,
If you're not going to test your changes, how about delaying these
patches till after 23.2?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: with-current-buffer "cleanup" bug
2009-11-14 22:27 ` Chong Yidong
@ 2009-11-15 4:48 ` Stefan Monnier
2009-11-15 15:26 ` Chong Yidong
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2009-11-15 4:48 UTC (permalink / raw)
To: Chong Yidong; +Cc: emacs-devel
>>> The `with-current-buffer' macro does not save point (or mark), so
>>> this can lead to code behavior change. You need to fix this, or
>>> we're likely see a long slow trickle of related bugs in the coming
>>> weeks and months.
>> Yes, I'm waiting for those bugs indeed,
> If you're not going to test your changes, how about delaying these
> patches till after 23.2?
I don't think it's necessary. The 23.2 pretest will offer plenty of time.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: with-current-buffer "cleanup" bug
2009-11-15 4:48 ` Stefan Monnier
@ 2009-11-15 15:26 ` Chong Yidong
0 siblings, 0 replies; 6+ messages in thread
From: Chong Yidong @ 2009-11-15 15:26 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> Yes, I'm waiting for those bugs indeed,
>> If you're not going to test your changes, how about delaying these
>> patches till after 23.2?
>
> I don't think it's necessary. The 23.2 pretest will offer plenty of
> time.
At the least, please refrain from making any more of these silly
"cleanups". They have negligible upside and considerable downside.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: with-current-buffer "cleanup" bug
2009-11-14 15:51 with-current-buffer "cleanup" bug Chong Yidong
2009-11-14 18:07 ` Stefan Monnier
@ 2009-11-15 5:13 ` Stefan Monnier
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Monnier @ 2009-11-15 5:13 UTC (permalink / raw)
To: Chong Yidong; +Cc: emacs-devel
> I just reverted your change to help-mode.el, because it led to a bug:
BTW, any such revert should come with a comment explaining the problem to
avoid going through the same back&forth again.
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-11-15 15:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-14 15:51 with-current-buffer "cleanup" bug Chong Yidong
2009-11-14 18:07 ` Stefan Monnier
2009-11-14 22:27 ` Chong Yidong
2009-11-15 4:48 ` Stefan Monnier
2009-11-15 15:26 ` Chong Yidong
2009-11-15 5:13 ` Stefan Monnier
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).