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

[Tim X]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Date: Fri, 11 Mar 2011 10:52:14 -0500
>> 
>> So (save-excursion (goto-char BAR)) is pretty much a no-op.
>> 
>> But (save-excursion (set-buffer FOO) (goto-char BAR)) is either:
>> - the same as (save-excursion (goto-char BAR)), in case FOO is already 
>> current.
>> - an inefficient form of (with-current-buffer FOO (goto-char BAR)).
>> I still haven't found any code out there where this behavior is what
>> is actually wanted and expected by the programmer.
>> 
>> In more than 90% of the cases, the intended meaning is
>> (with-current-buffer FOO (goto-char BAR)) and the behavior if FOO is
>> current (to additionally preserve point) is harmless, so the warning
>> simply points out an inefficiency.
>> 
>> In the remaining cases, FOO is almost always current, but when it's not
>> the resulting behavior is a bug.  I.e. the intended code is
>> (with-current-buffer FOO (save-excursion (goto-char BAR))).
>
> Then how about changing the text of the warning to something like
>
>   Warning: `save-excursion' will not preserve point in the other buffer
>   set by `set-buffer'
>

I'm not sure it is possible to get a really concise warning message here
and the issue requires more explination than is possible in a warning
message.

My suggestion would be to add the word 'possibly' and perhaps add a
reference to a relevant section in the manual i.e.

Warning: save-excursion possibly defeated by set-buffer. See <relevant
elisp manual section>.

The above would be preferrable over longer warning messages, especially
messages that really only describe the behavior of save-excursion as
that doesn't really add anything the programmer probably didn't already
know. There are other warning which reference the manual (such as the
one concerning old style backquotes), so it seems like a consistent
approach. Adding the word possibly emphasises this is a warning and not
an error and could be legitimate.

Issuing a warning with a reference to a relevant section of the
manual would allow the programmer to see the warning, lookup that
section and get a more in-depth explination about the possible
situations and why either save-excursion is not doing what the
programmer expects or is inefficient and would be better coded using
with-current-buffer etc. They can then decide how relevant the warning
is and adapt their code appropriately. 

Tim

-- 
tcross (at) rapttech dot com dot au