unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: "Drew Adams" <drew.adams@oracle.com>
To: "'Stefan Monnier'" <monnier@iro.umontreal.ca>
Cc: 7728@debbugs.gnu.org
Subject: bug#7728: 24.0.50; GDB backtrace from abort
Date: Thu, 13 Jan 2011 14:06:58 -0800	[thread overview]
Message-ID: <5BBB2AD4CF5B4B66A75E12B8D7BBD637@us.oracle.com> (raw)
In-Reply-To: <jwvtyhccxo8.fsf-monnier+emacs@gnu.org>

> I'm just telling you what's the right way to do it.

I appreciate that.  But it's not related to what's needed to fix this bug.
That's all I was trying to say.

> > Sorry, but I have never, ever suffered from "those things".
> 
> Then either you were lucky to only use the code after I fixed 
> it, or you don't know what I'm talking about.  The typical
> misuse looks like:
> (save-window-excursion (let ((b (find-file "foo")))  blabla))
> instead of (let ((b (find-file-noselect "foo")))  blabla)
> 
> I.e. call code that may modify the window-layout whereas what 
> the caller wants is something else, so he uses
> save-window-excursion to "undo" those changes.  But of course,
> with pop-up-frames and friends, in many/most cases the code may
> not only modify the window-layout but also create a new frame,
> which can't really be undone because the creation
> itself is already user-visible, and save-window-excursion 
> won't even try to undo it anyway.

It's good to have that pointed out, of course.  But no, I don't believe I've
used `s-w-e' like that.  I tend to use `find-file-noselect' much more than
`find-file' anyway.  I call `find-file' typically only from `find-file' wrapper
commands and such.  But I get your point.

I expect that both you and I have come across places now and then where we
needed to fix code because of interactions with frames.  I don't think it's
common that such source bugs stay hidden from us long, precisely because we use
frames more than most people do.

Problems such as you point out are, I imagine, more common with developers who
test only in an environment where frames are _not_ used very much.  It makes
sense to draw special attention to such gotchas in that case.
  
> > And `save-window-excursion' _has_ always been used for this kind of
> > thing in Emacs AFAIK - revisionism notwithstanding.
> 
> I didn't know that.  Can you point at some examples?

Nope, sorry.  But you can imagine, I think, that before `with-selected-frame'
existed `s-w-e' was used for similar use cases. ;-)

However, grepping for `with-selected-frame' in the Emacs sources shows very few
hits, and all of them correspond to code (e.g. function defs) that was
inexistent in older releases (e.g. Emacs 20).  So no, I don't see an example
offhand where code that used `s-w-e' might have been "upgraded" to code that
uses `with-selected-frame'.

> You know I always consider any crash as a bug in the C code, even if
> it's triggered by Elisp code.

Yes, I hope (and assume) so.  The question is whether the C fix will still allow
the same behavior as previously.  I really hope so.

So let's please just assume here that (a) the source code I mentioned is not
doing something ill-advised, verboten, off-the-wall, or silly wrt this bug, and
(b) it doesn't need to be changed for things to work properly, but rather (c)
the C code should be fixed so that such source code and its byte-compilation
continues to work as before.

I will help to diagnose things at my end if I can be of assistance, but let's
please not concentrate on having the user source code be changed to effect a
workaround.

That `save-window-excursion', like other things in Emacs Lisp, can be misused or
might otherwise not DTRT in some cases is not the question here.

For my use case it DTRT, and has been doing it, for this code, for years.  This
code needs to work not just for my own setup (with a standalone minibuffer etc.
- the case for the reported crash), but more importantly for other setups,
including more common setups.  It works well, I believe, so let's not worry here
and now about improving it.  That's all.






  reply	other threads:[~2011-01-13 22:06 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-24 16:55 bug#7728: 24.0.50; GDB backtrace from abort Drew Adams
2010-12-25  9:38 ` Eli Zaretskii
2010-12-25 10:44   ` Andreas Schwab
2010-12-25 11:12     ` Eli Zaretskii
2010-12-25 20:35   ` Stefan Monnier
2011-01-01 18:02     ` Eli Zaretskii
2011-01-09 21:18       ` Eli Zaretskii
2011-01-10 23:32         ` Drew Adams
2011-01-11 20:55       ` Stefan Monnier
2011-01-11 21:14         ` Eli Zaretskii
2011-01-11 21:44           ` Drew Adams
2011-01-12  4:11             ` Eli Zaretskii
2011-01-12  4:59               ` Drew Adams
2011-01-12 11:03                 ` Eli Zaretskii
2011-01-12 18:36                   ` Drew Adams
2011-01-12 19:52                   ` Drew Adams
2011-01-12 21:30                     ` Drew Adams
2011-01-12  7:54         ` martin rudalics
2011-01-12 15:05           ` Drew Adams
2011-01-12 15:14           ` Stefan Monnier
2011-01-12 15:59             ` martin rudalics
2011-01-12 16:22             ` Eli Zaretskii
2011-01-12 17:42               ` martin rudalics
2011-01-12 17:48                 ` Eli Zaretskii
2011-01-12 18:35                   ` martin rudalics
2011-01-12 18:36                   ` Drew Adams
2011-01-15  2:59                 ` Chong Yidong
2011-01-15 20:05                   ` martin rudalics
2011-01-13  2:53               ` Stefan Monnier
2011-01-13  7:07                 ` Drew Adams
2011-01-13 17:02                   ` Stefan Monnier
2011-01-13 17:57                     ` Drew Adams
2011-01-13 21:24                       ` Stefan Monnier
2011-01-13 22:06                         ` Drew Adams [this message]
2011-01-14  0:26                       ` Eli Zaretskii
2011-01-14  1:19                         ` Drew Adams
2011-01-14  2:40                           ` Eli Zaretskii
2011-01-14  6:46                             ` Drew Adams
2011-01-14  7:09                               ` Drew Adams
2011-01-14 20:01                         ` Sean Sieger
2011-01-14 21:06                           ` Drew Adams
2011-01-14 21:46                             ` Sean Sieger
2011-01-14 22:51                               ` Eli Zaretskii
2011-01-14 23:56                                 ` Sean Sieger
2011-01-14  2:25                       ` Stefan Monnier
2011-01-14  4:25                         ` Drew Adams
2011-01-14  8:26                     ` martin rudalics
2011-01-14  8:58                       ` Drew Adams
2011-01-14 15:30                         ` Stefan Monnier
2011-01-16 20:44                           ` Drew Adams

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5BBB2AD4CF5B4B66A75E12B8D7BBD637@us.oracle.com \
    --to=drew.adams@oracle.com \
    --cc=7728@debbugs.gnu.org \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).