help-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: `save-excursion' defeated by `set-buffer'


From: Uday Reddy
Subject: Re: `save-excursion' defeated by `set-buffer'
Date: Mon, 14 Mar 2011 12:22:23 +0000
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9

[This article that I sent to address@hidden on 13 Mar 2011, 12:46:00, didn't show up on the news server. However, Drew's response to it showed up on the news server. Some lossage in the ether! I am posting it as a response to Drew's message, even though it preceded it.]


Tim X writes:

> > Seems all what's neccessary to know about save-excursion already
> > is expressed in it's docstring.
> >
>
> Not sure thats true. I've seen a lot of elisp code that does the
> save-excursion, set buffer pattern, which makes me think maybe the
> documentation is inadequate.

It seems that reason for the widespread use of the
save-excursion/set-buffer pattern is actually quite simple.  Before
Emacs version 20.1, there was no other way to do it.
save-current-buffer was only introduced in 20.1.  So, there is a lot
of old code that uses the old pattern.  Now, there is also a lot of
new code that uses the old pattern because people learn their patterns
from the old code.

> > If people don't want the buffer restored alongside with point and mark, they
> > should not use this form.
> >
>
> I think it is a little more subtle than that. People may be using it
> under the expectation that point and mark will also be saved in the
> buffer accessed by set-buffer.

No, I don't actually think that.  (I notice that Andreas has
questioned this assumption too.)

If at all people are doing it consciously, they do it because they
think it is harmless.  You want to preserve the current-buffer.  But,
if you also preserve the point and mark, it seems like harmless
redundancy.  So, what is the big deal?

It takes quite a bit of insight into the code to understand that it is
actually *harmful* redundancy.  The unintentional uses of
save-excursion cover up potential bugs inside the code which don't get
noticed because of this redundant saving.

So, my test is pure and simple.  Convert all the save-excursion's that
the compiler complains about to save-current-buffer.  If your code
continues to run perfectly, you are my hero.  If your code falls over,
then you should accept that your code is buggy and it is only able to
stand up on the crutches of what you thought was "harmless
redundancy".  Then it is up to you whether you want to fix the code or
continue to live with it.  But there are many of us who won't touch
such code with a tadpole because we regard it as buggy.

> Except we know it is often used incorrectly. I think Uday's point is
> very valid here. When writing code, part of the aim is to communicate
> intentions to others who may need to maintain or update the code in the
> future. As Uday pointed out, using save-excursion followed by set-buffer
> muddies the intention and decreases clarity.

Unfortunately, it is more than a question of clarity.  It is a
question of correctness.  Let me restate my example from Friday a bit
more explicitly

(defun my-beautiful-function ()
    (save-excursion
       (set-buffer B)
       (goto-char x)
       (setq a (find-something))
       ...))

(defun find-something ()
        (....
           (set-buffer A)
           (goto-char y)
         ...))

find-something is *buggy* here.  It is moving point in buffer A though
it wasn't supposed to.  However, my-beautiful-function works fine as
long as I call it in buffer A because the "harmlessly redundant"
save-excursion at its top-level restores A's point.

Tomorrow, you decide to call my-beautiful-function from some other
buffer C, and all hell breaks loose.  You suddenly find that the point
is moving in the unrelated buffer A and you have no idea why.

So, rewrite the code as follows:

(defun my-beautiful-function ()
    (save-current-buffer
       (set-buffer B)
       (goto-char x)
       ...
       (setq a (find-something))
       ...))

(defun find-something ()
        (....
           (save-current-buffer
              (set-buffer A)
              (save-excursion
                 (goto-char y)
                 ...)))

Not only would you have mollified the compiler, but you will have much
more robust code that will continue to function when used in
unforeseen ways.

Cheers,
Uday



reply via email to

[Prev in Thread] Current Thread [Next in Thread]